-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Unify SourceFile::name_hash and StableSourceFileId #119139
Unify SourceFile::name_hash and StableSourceFileId #119139
Conversation
r? @davidtwco (rustbot has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue The changes here cause more |
This comment has been minimized.
This comment has been minimized.
…ce-file-id, r=<try> Unify SourceFile::name_hash and StableSourceFileId This PR adapts the existing `StableSourceFileId` type so that it can be used instead of the `name_hash` field of `SourceFile`. This simplifies a few things that were kind of duplicated before. The PR should also fix issues rust-lang#112700 and rust-lang#115835, but I was not able to reproduce these issues in a regression test. As far as I can tell, the root cause of these issues is that the id of the originating crate is not hashed in the `HashStable` impl of `Span` and thus cache entries that should have been considered invalidated were loaded. After this PR, the `stable_id` field of `SourceFile` includes information about the originating crate, so that ICE should not occur anymore.
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (1b4168c): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 671.988s -> 671.084s (-0.13%) |
I don't think the I'm going to tag this as: but cc @rust-lang/wg-compiler-performance, just in case |
agreed (and so does cachegrind) |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (bf8716f): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 669.342s -> 669.392s (0.01%) |
This PR adapts the existing
StableSourceFileId
type so that it can be used instead of thename_hash
field ofSourceFile
. This simplifies a few things that were kind of duplicated before.The PR should also fix issues #112700 and #115835, but I was not able to reproduce these issues in a regression test. As far as I can tell, the root cause of these issues is that the id of the originating crate is not hashed in the
HashStable
impl ofSpan
and thus cache entries that should have been considered invalidated were loaded. After this PR, thestable_id
field ofSourceFile
includes information about the originating crate, so that ICE should not occur anymore.