-
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
Steps towards reproducible builds #35984
Conversation
r? @Aatch (rust_highfive has picked a reviewer for you, use r? to override) |
|
||
use super::InferCtxt; | ||
use super::unify_key::ToType; | ||
|
||
pub struct TypeFreshener<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { | ||
infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, | ||
freshen_count: u32, | ||
freshen_map: hash_map::HashMap<ty::InferTy, Ty<'tcx>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a performance problem (I mean, using a non-FNV hash here). This is a rather hot function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was surprised to see this, too. I'll try to do a little benchmark to see if this patch improves type checking performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some tests on syntex_syntax
:
With this PR:
time: 0.557; rss: 230MB item-types checking
time: 5.720; rss: 262MB item-bodies checking
time: 0.574; rss: 230MB item-types checking
time: 5.779; rss: 262MB item-bodies checking
time: 0.559; rss: 231MB item-types checking
time: 5.761; rss: 261MB item-bodies checking
time: 0.566; rss: 232MB item-types checking
time: 5.708; rss: 263MB item-bodies checking
Without this PR (current master):
time: 0.560; rss: 232MB item-types checking
time: 5.768; rss: 263MB item-bodies checking
time: 0.565; rss: 230MB item-types checking
time: 5.907; rss: 261MB item-bodies checking
time: 0.561; rss: 230MB item-types checking
time: 5.810; rss: 261MB item-bodies checking
time: 0.557; rss: 231MB item-types checking
time: 5.812; rss: 261MB item-bodies checking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
r? @eddyb |
@bors r+ |
📌 Commit 729d3c3 has been approved by |
Works every time 😜 |
⌛ Testing commit 729d3c3 with merge 25d871a... |
💔 Test failed - auto-win-gnu-32-opt-rustbuild |
New spurious failure? |
@jonas-schievink - looks like it, hopefully it's fixed now. I'm going to be optimistic and retry @bors retry |
☔ The latest upstream changes (presumably #36030) made this pull request unmergeable. Please resolve the merge conflicts. |
* A step towards #34902 * More stable error messages in some places related to crate loading * Possible slight performance improvements since all `HashMap`s replaced had small keys where `FnvHashMap` should be faster (although I didn't measure)
`ty::Predicate` was being used as a key for a hash map, but its hash implementation indirectly hashed addresses, which vary between each compiler run. This is fixed by sorting predicates by their ID before encoding them. In my tests, rustc is now able to produce deterministic results when compiling libcore and libstd. I've beefed up `run-make/reproducible-build` to compare the produced artifacts bit-by-bit. This doesn't catch everything, but should be a good start. cc #34902
@bors r+ |
📌 Commit e468ede has been approved by |
Steps towards reproducible builds cc #34902 Running `make dist` twice will result in a rustc tarball where only `librustc_back.so`, `librustc_llvm.so` and `librustc_trans.so` differ. Building `libstd` and `libcore` twice with the same compiler and flags produces identical artifacts. The third commit should close #24473
💔 Test failed - auto-win-msvc-64-opt-rustbuild |
Apparently it doesn't really work on MSVC (no idea what's up). I've reverted the test changes in order to at least get something. |
@bors r=eddyb |
📌 Commit 8766c18 has been approved by |
Steps towards reproducible builds cc #34902 Running `make dist` twice will result in a rustc tarball where only `librustc_back.so`, `librustc_llvm.so` and `librustc_trans.so` differ. Building `libstd` and `libcore` twice with the same compiler and flags produces identical artifacts. The third commit should close #24473
cc #34902
Running
make dist
twice will result in a rustc tarball where onlylibrustc_back.so
,librustc_llvm.so
andlibrustc_trans.so
differ. Buildinglibstd
andlibcore
twice with the same compiler and flags produces identical artifacts.The third commit should close #24473