Avoid many `cmt` allocations.#50418

nnethercote · 2018-05-03T12:18:25Z

cmt is a ref-counted wrapper around cmt_ The use of refcounting
keeps cmt handling simple, but a lot of cmt instances are very
short-lived, and heap-allocating the short-lived ones takes up time.

This patch changes things in the following ways.

Most of the functions that produced cmt instances now produce cmt_
instances. The Rc::new calls that occurred within those functions
now occur at their call sites (but only when necessary, which isn't
that often).
Many of the functions that took cmt arguments now take &cmt_
arguments. This includes all the methods in the Delegate trait.

As a result, the vast majority of the heap allocations are avoided. In
an extreme case, the number of calls to malloc in tuple-stress drops
from 9.9M to 7.9M, a drop of 20%. And the compile times for many runs of
coercions, deep-vector, and tuple-stress drop by 1--2%.

`cmt` is a ref-counted wrapper around `cmt_` The use of refcounting keeps `cmt` handling simple, but a lot of `cmt` instances are very short-lived, and heap-allocating the short-lived ones takes up time. This patch changes things in the following ways. - Most of the functions that produced `cmt` instances now produce `cmt_` instances. The `Rc::new` calls that occurred within those functions now occur at their call sites (but only when necessary, which isn't that often). - Many of the functions that took `cmt` arguments now take `&cmt_` arguments. This includes all the methods in the `Delegate` trait. As a result, the vast majority of the heap allocations are avoided. In an extreme case, the number of calls to malloc in tuple-stress drops from 9.9M to 7.9M, a drop of 20%. And the compile times for many runs of coercions, deep-vector, and tuple-stress drop by 1--2%.

rust-highfive · 2018-05-03T12:18:28Z

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

leonardo-m · 2018-05-03T17:07:00Z

Could a compiler pass be added to perform this optimization (escape analysis?)?

estebank · 2018-05-03T17:49:47Z

LGTM. r? @petrochenkov or @eddyb for a second pair of eyes.

eddyb · 2018-05-03T18:02:20Z

Welp, all of this ancient code will hopefully go away once MIR borrowck takes over.
But for now, nice wins! @bors r+

bors · 2018-05-03T18:02:21Z

📌 Commit 7cf142f has been approved by eddyb

nnethercote · 2018-05-03T23:39:21Z

@Manishearth: note that this will break Clippy. The fix will be easy, just updating method arguments for the changes in the Delegate trait.

Manishearth · 2018-05-03T23:49:42Z

sounds good, thanks for the heads up!

bors · 2018-05-04T10:38:31Z

⌛ Testing commit 7cf142f with merge f86297d51946264715934bafd69c6c9315f80594...

bors · 2018-05-04T12:12:01Z

💔 Test failed - status-appveyor

nnethercote · 2018-05-04T12:31:28Z

The failure is:

thread 'build_script::rename_with_link_search_path' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 5, kind: PermissionDenied, message: "Access is denied." }

Infrastructure failure? Worth a retry?

eddyb · 2018-05-04T14:23:58Z

@bors retry

kennytm · 2018-05-04T17:30:45Z

cc #48775

bors · 2018-05-05T01:27:45Z

⌛ Testing commit 7cf142f with merge a19e9ece9c3a52c20c96c3a6d337c8d193a94e6b...

bors · 2018-05-05T03:36:05Z

💔 Test failed - status-travis

nnethercote · 2018-05-05T05:00:19Z

[01:36:07] test process::tests::test_process_output_fail_to_start ... test process::tests::test_process_output_fail_to_start has been running for over 60 seconds

Which is #43283.

Can somebody please retry?

Zoxc · 2018-05-05T06:14:53Z

@bors r=eddyb

bors · 2018-05-05T06:14:53Z

💡 This pull request was already approved, no need to approve it again.

This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
There's another pull request that is currently being tested, blocking this pull request: Add armv5te-unknown-linux-musl target #50423

bors · 2018-05-05T06:14:54Z

📌 Commit 7cf142f has been approved by eddyb

bors · 2018-05-05T08:52:34Z

⌛ Testing commit 7cf142f with merge e471c20...

Avoid many `cmt` allocations. `cmt` is a ref-counted wrapper around `cmt_` The use of refcounting keeps `cmt` handling simple, but a lot of `cmt` instances are very short-lived, and heap-allocating the short-lived ones takes up time. This patch changes things in the following ways. - Most of the functions that produced `cmt` instances now produce `cmt_` instances. The `Rc::new` calls that occurred within those functions now occur at their call sites (but only when necessary, which isn't that often). - Many of the functions that took `cmt` arguments now take `&cmt_` arguments. This includes all the methods in the `Delegate` trait. As a result, the vast majority of the heap allocations are avoided. In an extreme case, the number of calls to malloc in tuple-stress drops from 9.9M to 7.9M, a drop of 20%. And the compile times for many runs of coercions, deep-vector, and tuple-stress drop by 1--2%.

bors · 2018-05-05T11:05:18Z

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing e471c20 to master...

rust-highfive assigned estebank May 3, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 3, 2018

rust-highfive assigned petrochenkov and unassigned estebank May 3, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 3, 2018

petrochenkov assigned eddyb and unassigned petrochenkov May 4, 2018

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 4, 2018

phansch mentioned this pull request May 4, 2018

Incoming clippy breakage rust-lang/rust-clippy#2725

Closed

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 4, 2018

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 5, 2018

This comment has been minimized.

Sign in to view

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 5, 2018

bors merged commit 7cf142f into rust-lang:master May 5, 2018

nnethercote deleted the cmt branch May 6, 2018 01:27

This was referenced May 6, 2018

Set opt-level to 3 #50329

Merged

Spurious access violation (exit code: 3221225477) when compiling libsyntax with MSVC #50604

Closed

Uh oh!

Conversation

nnethercote commented May 3, 2018

Uh oh!

rust-highfive commented May 3, 2018

Uh oh!

leonardo-m commented May 3, 2018

Uh oh!

estebank commented May 3, 2018

Uh oh!

eddyb commented May 3, 2018

Uh oh!

bors commented May 3, 2018

Uh oh!

nnethercote commented May 3, 2018

Uh oh!

Manishearth commented May 3, 2018

Uh oh!

bors commented May 4, 2018

Uh oh!

bors commented May 4, 2018

Uh oh!

nnethercote commented May 4, 2018

Uh oh!

eddyb commented May 4, 2018

Uh oh!

kennytm commented May 4, 2018

Uh oh!

bors commented May 5, 2018

Uh oh!

bors commented May 5, 2018

Uh oh!

This comment has been minimized.

This comment has been minimized.

nnethercote commented May 5, 2018

Uh oh!

Zoxc commented May 5, 2018

Uh oh!

bors commented May 5, 2018

Uh oh!

bors commented May 5, 2018

Uh oh!

bors commented May 5, 2018

Uh oh!

bors commented May 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants