-
-
Notifications
You must be signed in to change notification settings - Fork 14.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
buildRustCrate: do not forcibly disable rustc parallelism #274135
Conversation
buildRustCrate currently sets `codegen-units` to `1` if unspecified. This option controls the amount of process-level parallelism that LLVM employs during the build. The [default is supposed to be 16](https://doc.rust-lang.org/rustc/codegen-options/index.html#codegen-units). By forcing this to `1` we are disabling all intra-derivation parallelism. This makes `buildRustCrate` and `crate2nix` much slower than `cargo`; we shouldn't do that!
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 followed git blame a bit - this has been set to a static value due to nondeterminism if using NIX_BUILD_CORES in #170981.
The discussion in that same PR mentions the following too:
the difference between codegen-units=1 and codegen-units=16 was a bit less than a factor of 2 of build time on my 24-core machine
It also links to #130309 (comment), which prefers 1 due to performance of the resulting binary.
Your PR description "[…] cripples all parallelism […]" suggests a much higher compilation performance penalty.
I'm also not sure about the implications on runtime performance in real-world scenarios. Do you have any numbers to back both things up?
What is the reason for closing this PR ? |
I think this PR was closed in in the light of #50105 (comment). |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/crate2nix-setting-codegenunits-for-all-crates/53014/2 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/crate2nix-setting-codegenunits-for-all-crates/53014/3 |
Our
buildRustCrate
currently cripples all parallelism, unlike ourbuildRustPackage
. We should not do this.Our
buildRustCrate
for setscodegen-units
to1
if unspecified. Thecodegen-units
option controls the amount of process-level parallelism that LLVM employs during the build.The default is supposed to be 16, but we should simply leave the option unspecified in order to track any future changes to the default. By forcing this to
1
we are disabling all intra-derivation parallelism. This makesbuildRustCrate
andcrate2nix
much slower thancargo
, for no good reason.