incr.comp.: Store DepNode colors in a dense array instead of a hashmap. by michaelwoerister · Pull Request #48206 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation12 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
⌛ Trying commit d4b8475 with merge b9b23ff5e8fb80c09f74cc608d96763149daecfa...
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
| 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, |
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.
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.
Looks like a mild improvement (a few percent) in some cases; no regressions.
📌 Commit d4b8475 has been approved by nikomatsakis
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
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
Manishearth added a commit to Manishearth/rust that referenced this pull request
bors added a commit that referenced this pull request
Rollup of 17 pull requests
- Successful merges: #48052, #48072, #48082, #48083, #48084, #48106, #48123, #48157, #48185, #48197, #48198, #48206, #48208, #48221, #48258, #48314, #48335
- Failed merges:
Manishearth added a commit to Manishearth/rust that referenced this pull request
bors added a commit that referenced this pull request
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
bors added a commit that referenced this pull request
Rollup of 15 pull requests
- Successful merges: #47987, #48056, #48061, #48084, #48143, #48185, #48206, #48208, #48232, #48246, #48258, #48317, #48353, #48356, #48402
- Failed merges:
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.