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

rustc: Avoid HashStable for DefIndex queries #44584

Closed

Conversation

alexcrichton
Copy link
Member

I'm not 100% familiar with this trait and this location, but it looks like
there's a default implementation of DepNodeParams for anything that implements
HashStable, but types like DefId which have precalculated hashes bypass this
default implementation for a speedier one.

This commit applies what I believe is the same optimization to DefIndex,
looking up the local hash for it rather than going through the full HashStable
rigamarole

cc #44575

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@alexcrichton
Copy link
Member Author

r? @nikomatsakis

@alexcrichton alexcrichton added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 15, 2017
@arielb1
Copy link
Contributor

arielb1 commented Sep 18, 2017

This LGTM.

@bors
Copy link
Contributor

bors commented Sep 18, 2017

☔ The latest upstream changes (presumably #44678) made this pull request unmergeable. Please resolve the merge conflicts.

I'm not 100% familiar with this trait and this location, but it looks like
there's a default implementation of `DepNodeParams` for anything that implements
`HashStable`, but types like `DefId` which have precalculated hashes bypass this
default implementation for a speedier one.

This commit applies what I believe is the same optimization to `DefIndex`,
looking up the local hash for it rather than going through the full `HashStable`
rigamarole

cc rust-lang#44575
@alexcrichton
Copy link
Member Author

Rebased

@alexcrichton
Copy link
Member Author

This landed in #44364

@alexcrichton alexcrichton deleted the maybe-fix-regression branch September 19, 2017 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants