Skip to content

buildRustCrate: make default value for codegenUnits configurable#383716

Merged
Kranzes merged 1 commit intoNixOS:masterfrom
niklaskorz:buildRustCrate-codegenUnits
Mar 8, 2025
Merged

buildRustCrate: make default value for codegenUnits configurable#383716
Kranzes merged 1 commit intoNixOS:masterfrom
niklaskorz:buildRustCrate-codegenUnits

Conversation

@niklaskorz
Copy link
Member

@niklaskorz niklaskorz commented Feb 20, 2025

As of now, buildRustCrate defaults to using a single codegen unit per crate, which is far off from what cargo is doing:

The default value, if not specified, is 16 for non-incremental builds. For incremental builds the default is 256 which allows caching to be more granular.

Before debating whether the value itself should be increased, we should at least make it easy to override this for all crates at once, by introducing a parameter defaultCodegenUnits. In its current state, the codegenUnits parameter must be overriden per crate to increase codegen parallelization.

Note that this change does not affect anything inside nixpkgs, but mainly consumers of crate2nix (also see #373744).

Example usage for crate2nix:

{ pkgs, ... }:
let
  rustPkgs = pkgs.callPackage ./Cargo.nix {
    buildRustCrateForPkgs =
      pkgs:
      pkgs.buildRustCrate.override {
        defaultCodegenUnits = 16;
      };
  };
in
# ...

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. label Feb 20, 2025
@nix-owners nix-owners bot requested review from figsoda, winterqt and zowoq February 20, 2025 18:13
@niklaskorz niklaskorz requested a review from flokli February 20, 2025 18:16
@niklaskorz niklaskorz force-pushed the buildRustCrate-codegenUnits branch from c1e414a to c862c3b Compare February 20, 2025 18:16
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Feb 20, 2025
@niklaskorz niklaskorz force-pushed the buildRustCrate-codegenUnits branch from c862c3b to 091a959 Compare February 20, 2025 18:37
@flokli
Copy link
Member

flokli commented Feb 24, 2025

As of now, buildRustCrate defaults to using a single codegen unit per crate, which is far off from what cargo is doing:

The default value, if not specified, is 16 for non-incremental builds. For incremental builds the default is 256 which allows caching to be more granular.

Before debating whether the value itself should be increased, we should at least make it easy to override this for all crates at once, by introducing a parameter defaultCodegenUnits. In its current state, the codegenUnits parameter must be overriden per crate to increase codegen parallelization.

There's already been a bit of a debate on what this should be set to, see #130309 (and follow the breadcrumbs from there).

Also see https://discourse.nixos.org/t/crate2nix-setting-codegenunits-for-all-crates/53014/9 for some recent benchmarking.

@niklaskorz
Copy link
Member Author

@flokli thank you for the feedback, but as I said, I'm not here to debate the default, but just want to make it easier to change the default for all crates at once

@drakon64
Copy link
Contributor

Also see https://discourse.nixos.org/t/crate2nix-setting-codegenunits-for-all-crates/53014/9 for some recent benchmarking.

I'm not entirely sure this is a great benchmark since it's just one crate which was hastily tested, and included still-experimental compiler flags

@flokli
Copy link
Member

flokli commented Feb 24, 2025

@flokli thank you for the feedback, but as I said, I'm not here to debate the default, but just want to make it easier to change the default for all crates at once

Not objecting the PR, just providing additional context, especially on why it might be set to 1 currently.

@niklaskorz
Copy link
Member Author

Also, fwiw, in our own monorepo, increasing codegen units to 16 decreased our cold build time from 9 to 7 minutes.

@niklaskorz niklaskorz requested a review from Kranzes March 6, 2025 12:47
@Kranzes Kranzes merged commit 762169c into NixOS:master Mar 8, 2025
67 checks passed
@niklaskorz niklaskorz deleted the buildRustCrate-codegenUnits branch November 10, 2025 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants