Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incr.comp.: Store DepNode colors in a dense array instead of a hashmap. #48206

Merged
merged 1 commit into from
Feb 24, 2018

Conversation

michaelwoerister
Copy link
Member

Implements half of #47293.

r? @nikomatsakis

@michaelwoerister
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 14, 2018

⌛ Trying commit d4b8475 with merge b9b23ff5e8fb80c09f74cc608d96763149daecfa...

@pietroalbini pietroalbini added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 14, 2018
@bors
Copy link
Contributor

bors commented Feb 14, 2018

☀️ Test successful - status-travis
State: approved= try=True

@michaelwoerister
Copy link
Member Author

@Mark-Simulacrum Could you start a perf-run, please?

@Mark-Simulacrum
Copy link
Member

Queued.

@pietroalbini pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 14, 2018
fn insert(&mut self, index: SerializedDepNodeIndex, color: DepNodeColor) {
self.values[index] = match color {
DepNodeColor::Red => COMPRESSED_RED,
DepNodeColor::Green(index) => index.0 + COMPRESSED_FIRST_GREEN,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're storing this at the index anyway, do we need to store the index value in it? Seems like perhaps no, in which case we could fairly easily optimize the IndexVec to have u8 values instead of `u32.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thought but it's a different index. We address into the array by SerializedDepNodeIndex (that is the index the node had in the depgraph from the previous session) and what we store is the current DepNodeIndex, which is the index the node has in the depgraph currently being built.

@nikomatsakis
Copy link
Contributor

Looks like a mild improvement (a few percent) in some cases; no regressions.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 14, 2018

📌 Commit d4b8475 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 14, 2018
@michaelwoerister
Copy link
Member Author

Looks like a mild improvement (a few percent) in some cases; no regressions.

Cool, that's pretty much the best that I expected :)

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 19, 2018
…komatsakis

incr.comp.: Store DepNode colors in a dense array instead of a hashmap.

Implements half of rust-lang#47293.

r? @nikomatsakis
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 20, 2018
bors added a commit that referenced this pull request Feb 20, 2018
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 24, 2018
…komatsakis

incr.comp.: Store DepNode colors in a dense array instead of a hashmap.

Implements half of rust-lang#47293.

r? @nikomatsakis
bors added a commit that referenced this pull request Feb 24, 2018
Rollup of 15 pull requests

- Successful merges: #47987, #48056, #48061, #48084, #48143, #48185, #48206, #48208, #48232, #48246, #48258, #48317, #48353, #48356, #48402
- Failed merges:
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 24, 2018
…komatsakis

incr.comp.: Store DepNode colors in a dense array instead of a hashmap.

Implements half of rust-lang#47293.

r? @nikomatsakis
bors added a commit that referenced this pull request Feb 24, 2018
Rollup of 15 pull requests

- Successful merges: #47987, #48056, #48061, #48084, #48143, #48185, #48206, #48208, #48232, #48246, #48258, #48317, #48353, #48356, #48402
- Failed merges:
@bors bors merged commit d4b8475 into rust-lang:master Feb 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants