Skip to content

Fix legacyPackages performance#423290

Merged
roberth merged 2 commits intoNixOS:masterfrom
roberth:fix-legacyPackages-perf
Jul 13, 2025
Merged

Fix legacyPackages performance#423290
roberth merged 2 commits intoNixOS:masterfrom
roberth:fix-legacyPackages-perf

Conversation

@roberth
Copy link
Member

@roberth roberth commented Jul 7, 2025

Remove one pkgs construction.
This makes .#hello.outPath 1.24 ± 0.03 times faster to evaluate.
Larger closures will see a less dramatic effect. Expect a 100 to 300 ms improvement, depending on hardware etc.

(I don't like the impurities which are now more visible, but not they're not new; just preserved. Presumably they're used with --impure, and I'd like to call that OT because unchanged. I don't like the altered lib either.)

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@nix-owners nix-owners bot requested a review from Ericson2314 July 7, 2025 18:58
roberth added 2 commits July 7, 2025 21:02
No change in behavior.
By factoring this out, we can pass an extra overlay in addition to
the impure ones in callers.
This saves the construction of an unused `pkgs` attrset and all that
is required to reach that, which is not a lot, but still significant.

Benchmark 1: nix eval .?ref=42608bc8e65c5bf596d6155ad2f0ded6253b1e69#hello.outPath --no-eval-cache
  Time (mean ± σ):      1.062 s ±  0.011 s    [User: 0.674 s, System: 0.226 s]
  Range (min … max):    1.040 s …  1.080 s    10 runs

Benchmark 2: nix eval .?ref=8a7e18b270cb2256a0526b9cda1d9e410aacd440#hello.outPath --no-eval-cache
  Time (mean ± σ):      1.317 s ±  0.033 s    [User: 0.835 s, System: 0.270 s]
  Range (min … max):    1.282 s …  1.371 s    10 runs

Summary
  nix eval .?ref=42608bc8e65c5bf596d6155ad2f0ded6253b1e69#hello.outPath --no-eval-cache ran
    1.24 ± 0.03 times faster than nix eval .?ref=8a7e18b270cb2256a0526b9cda1d9e410aacd440#hello.outPath --no-eval-cache

(where 42608bc had the same tree as this commit)
@roberth roberth force-pushed the fix-legacyPackages-perf branch from 7ff3f04 to b78a8c9 Compare July 7, 2025 19:02
@nixpkgs-ci nixpkgs-ci bot added 6.topic: flakes The experimental Nix feature 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 Jul 7, 2025
@Aleksanaa
Copy link
Member

Aleksanaa commented Jul 10, 2025

cpuTime increases by 0.5% in https://github.com/NixOS/nixpkgs/actions/runs/16125573939#summary-45501864095. This isn't too important, but it's a little weird considering we're reducing eval time.

@roberth
Copy link
Member Author

roberth commented Jul 10, 2025

The eval performance comparison uses the stable CLI, not flakes, so those times won't improve; they're already good in this regard.

Also that p value seems way too low, probably because it's measuring the wrong thing.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jul 13, 2025
@roberth roberth merged commit 5d22f1f into NixOS:master Jul 13, 2025
30 of 31 checks passed
@roberth
Copy link
Member Author

roberth commented Jul 13, 2025

Thank you Arian!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: flakes The experimental Nix feature 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. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants