Skip to content

Conversation

nnethercote
Copy link
Contributor

Following on from #64545, more speed and readability improvements.

r? @nikomatsakis

Optimizing for the common numbers of entries in `stalled_on` wins about
4% on `keccak` and `inflate`.
This is a big speed win for `keccak` and `inflate`.
We normally use `index` for indices to `ObligationForest::nodes`, but
this is a `Nodes::dependents` index.
The name `waiting_cache` sounds like it is related to the states
`NodeState::Waiting`, but it's not; the cache holds nodes of various
states.

This commit changes it to `active_state`.
Because the meaning of this `index` variable is quite different to all
the other `index` variables in this file.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 20, 2019
@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Collaborator

bors commented Sep 20, 2019

⌛ Trying commit aa10abb with merge 281fcd7...

bors added a commit that referenced this pull request Sep 20, 2019
Even more `ObligationForest` improvements

Following on from #64545, more speed and readability improvements.

r? @nikomatsakis
@bors
Copy link
Collaborator

bors commented Sep 20, 2019

💥 Test timed out

@nnethercote
Copy link
Contributor Author

Not sure what happened there. Let's try it the old-fashioned way:
@bors try

@bors
Copy link
Collaborator

bors commented Sep 20, 2019

⌛ Trying commit aa10abb with merge 85a4a15...

bors added a commit that referenced this pull request Sep 20, 2019
Even more `ObligationForest` improvements

Following on from #64545, more speed and readability improvements.

r? @nikomatsakis
@nnethercote
Copy link
Contributor Author

Some local measurements:

inflate-check
        avg: -10.3%     min: -14.3%     max: 0.1%
keccak-check
        avg: -7.8%      min: -12.0%     max: 0.0%
cranelift-codegen-check
        avg: -1.2%      min: -2.0%      max: 0.0%
clap-rs-check
        avg: -0.6%      min: -1.6%      max: -0.0%
serde-check
        avg: -0.5%      min: -0.8%      max: 0.1%
unify-linearly-check
        avg: -0.3%      min: -0.7%      max: 0.0% 
deeply-nested-check
        avg: -0.4%      min: -0.6%      max: 0.0% 
style-servo-check
        avg: -0.4%      min: -0.6%      max: 0.0%
wg-grammar-check
        avg: -0.3%      min: -0.5%      max: -0.0%
cargo-check
        avg: -0.2%      min: -0.3%      max: 0.0%
syn-check
        avg: -0.1%      min: -0.3%      max: 0.0% 

@bors
Copy link
Collaborator

bors commented Sep 20, 2019

☀️ Try build successful - checks-azure
Build commit: 85a4a15

@rust-timer
Copy link
Collaborator

Queued 85a4a15 with parent ea3ba36, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 85a4a15, comparison URL.

@bjorn3
Copy link
Member

bjorn3 commented Sep 22, 2019

@nnethercote Do you every take a day off of optimizing rustc? 😄

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 23, 2019

📌 Commit aa10abb has been approved by nikomatsakis

@bors 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 Sep 23, 2019
@nnethercote
Copy link
Contributor Author

@bors rollup=never

@bors
Copy link
Collaborator

bors commented Sep 25, 2019

⌛ Testing commit aa10abb with merge c7bc0bf...

bors added a commit that referenced this pull request Sep 25, 2019
Even more `ObligationForest` improvements

Following on from #64545, more speed and readability improvements.

r? @nikomatsakis
@bors
Copy link
Collaborator

bors commented Sep 25, 2019

☀️ Test successful - checks-azure
Approved by: nikomatsakis
Pushing c7bc0bf to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 25, 2019
@bors bors merged commit aa10abb into rust-lang:master Sep 25, 2019
@nnethercote nnethercote deleted the ObligForest-even-more branch September 25, 2019 20:11
tmandry added a commit to tmandry/rust that referenced this pull request Oct 2, 2019
… r=nikomatsakis

Still more `ObligationForest` improvements.

Following on from rust-lang#64627, more readability improvements, but negligible effects on speed.

r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants