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

Steps towards reproducible builds #35984

Merged
merged 6 commits into from
Aug 28, 2016
Merged

Steps towards reproducible builds #35984

merged 6 commits into from
Aug 28, 2016

Conversation

jonas-schievink
Copy link
Contributor

@jonas-schievink jonas-schievink commented Aug 25, 2016

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

@rust-highfive
Copy link
Collaborator

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>>,
Copy link
Contributor

@arielb1 arielb1 Aug 25, 2016

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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

Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

@jonas-schievink
Copy link
Contributor Author

r? @eddyb

@rust-highfive rust-highfive assigned eddyb and unassigned Aatch Aug 26, 2016
@eddyb
Copy link
Member

eddyb commented Aug 26, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Aug 26, 2016

📌 Commit 729d3c3 has been approved by eddyb

@jonas-schievink
Copy link
Contributor Author

Works every time 😜

@bors
Copy link
Contributor

bors commented Aug 26, 2016

⌛ Testing commit 729d3c3 with merge 25d871a...

@bors
Copy link
Contributor

bors commented Aug 26, 2016

💔 Test failed - auto-win-gnu-32-opt-rustbuild

@jonas-schievink
Copy link
Contributor Author

thread '[incremental] incremental\ich_method_call_trait_scope.rs' panicked at 'called `Result::unwrap()` on an `Err` value: Error { repr: Os { code: 3, message: "The system cannot find the path specified." } }', C:\bot\slave\auto-win-gnu-32-opt-rustbuild\build\src\libcore\result.rs:783

New spurious failure?

@sophiajt
Copy link
Contributor

@jonas-schievink - looks like it, hopefully it's fixed now. I'm going to be optimistic and retry

@bors retry

@bors
Copy link
Contributor

bors commented Aug 27, 2016

☔ 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
@eddyb
Copy link
Member

eddyb commented Aug 27, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Aug 27, 2016

📌 Commit e468ede has been approved by eddyb

@bors
Copy link
Contributor

bors commented Aug 28, 2016

⌛ Testing commit e468ede with merge 82e5788...

bors added a commit that referenced this pull request Aug 28, 2016
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
@bors
Copy link
Contributor

bors commented Aug 28, 2016

💔 Test failed - auto-win-msvc-64-opt-rustbuild

@jonas-schievink
Copy link
Contributor Author

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.

@arielb1
Copy link
Contributor

arielb1 commented Aug 28, 2016

@bors r=eddyb

@bors
Copy link
Contributor

bors commented Aug 28, 2016

📌 Commit 8766c18 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Aug 28, 2016

⌛ Testing commit 8766c18 with merge e4791e0...

bors added a commit that referenced this pull request Aug 28, 2016
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rustdoc output slightly nondeterministic
7 participants