rustc: enable codegen units and parallel building #21742

LnL7 · 2017-01-07T20:42:57Z

Motivation for this change

This makes rustc build in about ~30m compared to ~1h15m on my build machine.

I also noticed that patching the shebangs for the dev output is also very slow.
I'm pretty sure we don't have to do that, but I'm not sure how to disable it for a single output.

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

@retrry Did you run into issues with parallel builds? I have built a bunch of times now before I figured out what was causing the tests to fail and I have not seen any issues with it.

mention-bot · 2017-01-07T20:42:58Z

@LnL7, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vcunat, @retrry and @pikajude to be potential reviewers.

retrry · 2017-01-07T20:58:02Z

I remember only commenting that line. Didn't see any problems after that. Before that I didn't use rust so I don't know :)

Is codegen=10 really necessary? Where did you find that number? Quick google search gives me:

Where parallel codegen does help, the optimum number of codegen units is nearly always 4. Occasionally 2 is better, but that is rare. I've never seen >4 give an advantage. (Measured across a bunch of different crates and a few different machines, more data would be nice here).
https://internals.rust-lang.org/t/parallel-codegen-plans/2253

LnL7 · 2017-01-07T21:19:40Z

This is just for building the rust compiler itself, I can imagine that for most rust packages a value of more then 4 won't matter.

http://www.ncameron.org/blog/how-fast-can-i-build-rust/

the-kenny · 2017-01-07T21:34:56Z

pkgs/development/compilers/rust/rustc.nix

@@ -52,6 +42,9 @@ stdenv.mkDerivation {
  # versions.
  RUSTC_BOOTSTRAP = "1";

+  # Increase codegen units to introduce parallelism within the compiler.
+  RUSTFLAGS = "-Ccodegen-units=10";


Maybe we should set this in preBuild? Spares us the re-exporting in checkPhase

We should set this only if enableParallelBuilding is set to true.

That won't help, you'll still have to disable it before the checkPhase anyway. And this is more related to --cores then parallel building, it's still a single rustc process if you use this with parallel building enabled.

Ah! I didn't know that. Sounds fine to me :)

the-kenny · 2017-01-07T21:36:13Z

pkgs/development/compilers/rust/rustc.nix

  outputs = [ "out" "doc" ];
  setOutputFlags = false;

+  # Disable codegen units for the tests.


This line should be just above the export as preCheck does other things.

the-kenny · 2017-01-07T21:37:47Z

Did someone already run this with --option build-repeat N to check for reproducibility already?

LnL7 · 2017-01-07T21:39:15Z

I did not, the rust build is entirely deterministic at the moment?

the-kenny · 2017-01-07T21:40:47Z

As far as I know: Yes. Never heard of anyone running into issues with it. I tried it a version (or so) ago with N=4

the-kenny · 2017-01-08T09:51:02Z

I'll run the check stuff over the day on my machine and merge this if everything checks out.

I also got a few other small rustc improvements which I'll push right after this here is merged so we can benefit from the reduced build time.

LnL7 · 2017-01-08T09:54:34Z

I just checked my build results, looks like does indeed cause issues with reproducibility. I'm building master now to make sure that this was reproducible before.

the-kenny · 2017-01-08T09:56:29Z

Aww that's a bummer :( I'm still on +1 for merging this iff it had the same issues before.

It's also worth checking if the codegen stuff without enableParallelBuilding=true gives us a speedup and keeps reproducibility.

In any case we should add a comment to the derivation to mark it as non-reproducible.

LnL7 · 2017-01-08T11:00:22Z

Indeed, I was thinking the same thing. Running --check indicates that it's not deterministic there either.
Perhaps this is related to #21629?

grahamc · 2017-01-12T12:35:26Z

LGTM, let's merge?

the-kenny · 2017-01-12T13:25:16Z

👍

exi · 2017-01-12T22:35:12Z

@LnL7 this might have broken the build:
http://hydra.nixos.org/build/46497849/nixlog/1

errors like:

thread '[compile-fail] compile-fail/proc-macro/load-panic.rs' panicked at 'explicit panic', /tmp/nix-build-rustc-1.14.drv-0/rust-e8a0123/src/tools/compiletest/src/runtest.rs:2394

---- [compile-fail] compile-fail/macro-crate-doesnt-resolve.rs stdout ----
	

executing x86_64-unknown-linux-gnu/stage2/bin/rustc /tmp/nix-build-rustc-1.14.drv-0/rust-e8a0123/src/test/compile-fail-fulldeps/auxiliary/macro_crate_test.rs -L x86_64-unknown-linux-gnu/test/compile-fail-fulldeps/ --target=x86_64-unknown-linux-gnu --error-format json --crate-type=dylib -L x86_64-unknown-linux-gnu/test/compile-fail-fulldeps/macro-crate-doesnt-resolve.stage2-x86_64-unknown-linux-gnu.compile-fail.libaux -C prefer-dynamic --out-dir x86_64-unknown-linux-gnu/test/compile-fail-fulldeps/macro-crate-doesnt-resolve.stage2-x86_64-unknown-linux-gnu.compile-fail.libaux --cfg rtopt -C rpath -O -L x86_64-unknown-linux-gnu/rt
------stdout------------------------------

------stderr------------------------------
{"message":"could not write output to x86_64-unknown-linux-gnu/test/compile-fail-fulldeps/macro-crate-doesnt-resolve.stage2-x86_64-unknown-linux-gnu.compile-fail.libaux/macro_crate_test.0.o: No such file or directory","code":null,"level":"error","spans":[],"children":[],"rendered":null}

ttuegel · 2017-01-14T16:32:14Z

The build is also broken on my machine and reverting fixes it.

LnL7 · 2017-01-14T16:35:47Z

I'll change it back to enableParallelBuilding=false, but I don't understand why I did not run into it. I've built rustc about 10-15 times with these changes now.

NixOS#21742

ttuegel · 2017-01-14T19:33:35Z

@LnL7 Thank you!

Could this be a problem with enabling more codegen units than there are available cores? I don't know how many cores the Hydra builders have available, but I'm sure it varies. That might explain why it works for you. I will check some different settings when I have time, but for now I'm only just learning Rust! 😃

LnL7 · 2017-01-14T21:11:22Z

I'm fairly certain this issue is not related to the codegen units. My machine only has ssd and does have quite a bit of memory, perhaps that's why I'm not running into it

copumpkin · 2017-06-22T18:09:18Z

@wizeman looks like you've been looking into this a fair bit. Have any more information?

wizeman · 2017-06-22T19:32:57Z

@copumpkin: No, sorry. I simply filed an upstream Rust bug (rust-lang/rust#30181) when I ran into a build failure after setting enableParallelBuilding=true.

The upstream issue seems to have been marked closed with no indication that it was fixed. They asked if I could reproduce it with a current master commit but I currently don't have available time to do it.

oxij · 2018-09-08T23:26:18Z

FYI, I built current rustc with enableParallelBuilding without issues. PR: #46406.

LnL7 added the 6.topic: rust label Jan 7, 2017

LnL7 changed the base branch from staging to master January 7, 2017 20:43

rustc: enable codegen units and parallel building

2d6a00f

LnL7 force-pushed the rust-parallel branch from ff1288e to 2d6a00f Compare January 7, 2017 20:45

the-kenny reviewed Jan 7, 2017

View reviewed changes

the-kenny mentioned this pull request Jan 8, 2017

rustc: disable fragile tests on darwin #21741

Closed

7 tasks

the-kenny merged commit e91840c into NixOS:master Jan 12, 2017

LnL7 deleted the rust-parallel branch January 14, 2017 16:35

LnL7 added a commit to LnL7/nixpkgs that referenced this pull request Jan 14, 2017

rustc: disable parallel building because of rust-lang/rust#30181

e1d0c9a

NixOS#21742

LnL7 mentioned this pull request Jan 14, 2017

rustc: disable parallel building #21884

Merged

7 tasks

vcunat mentioned this pull request Oct 14, 2017

Rust 1.20.0 #30088

Merged

17 tasks

Mic92 mentioned this pull request Sep 9, 2018

rustc: enable parallel building #46406

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc: enable codegen units and parallel building #21742

rustc: enable codegen units and parallel building #21742

LnL7 commented Jan 7, 2017

mention-bot commented Jan 7, 2017

retrry commented Jan 7, 2017

LnL7 commented Jan 7, 2017 •

edited

the-kenny Jan 7, 2017

the-kenny Jan 7, 2017

LnL7 Jan 7, 2017 •

edited

the-kenny Jan 8, 2017

the-kenny Jan 7, 2017

the-kenny commented Jan 7, 2017

LnL7 commented Jan 7, 2017

the-kenny commented Jan 7, 2017

the-kenny commented Jan 8, 2017

LnL7 commented Jan 8, 2017 •

edited

the-kenny commented Jan 8, 2017

LnL7 commented Jan 8, 2017

grahamc commented Jan 12, 2017

the-kenny commented Jan 12, 2017

exi commented Jan 12, 2017

ttuegel commented Jan 14, 2017

LnL7 commented Jan 14, 2017

ttuegel commented Jan 14, 2017

LnL7 commented Jan 14, 2017

copumpkin commented Jun 22, 2017

wizeman commented Jun 22, 2017

oxij commented Sep 8, 2018

rustc: enable codegen units and parallel building #21742

rustc: enable codegen units and parallel building #21742

Conversation

LnL7 commented Jan 7, 2017

Motivation for this change

Things done

mention-bot commented Jan 7, 2017

retrry commented Jan 7, 2017

LnL7 commented Jan 7, 2017 • edited

the-kenny Jan 7, 2017

Choose a reason for hiding this comment

the-kenny Jan 7, 2017

Choose a reason for hiding this comment

LnL7 Jan 7, 2017 • edited

Choose a reason for hiding this comment

the-kenny Jan 8, 2017

Choose a reason for hiding this comment

the-kenny Jan 7, 2017

Choose a reason for hiding this comment

the-kenny commented Jan 7, 2017

LnL7 commented Jan 7, 2017

the-kenny commented Jan 7, 2017

the-kenny commented Jan 8, 2017

LnL7 commented Jan 8, 2017 • edited

the-kenny commented Jan 8, 2017

LnL7 commented Jan 8, 2017

grahamc commented Jan 12, 2017

the-kenny commented Jan 12, 2017

exi commented Jan 12, 2017

ttuegel commented Jan 14, 2017

LnL7 commented Jan 14, 2017

ttuegel commented Jan 14, 2017

LnL7 commented Jan 14, 2017

copumpkin commented Jun 22, 2017

wizeman commented Jun 22, 2017

oxij commented Sep 8, 2018

LnL7 commented Jan 7, 2017 •

edited

LnL7 Jan 7, 2017 •

edited

LnL7 commented Jan 8, 2017 •

edited