stdenv: Move all stdenv lookups out of mkDerivation by YorikSar · Pull Request #431756 · NixOS/nixpkgs

YorikSar · 2025-08-07T13:50:04Z

All lookups and expressions that don't depend on derivation attrs are moved to the outer let block. Saves over 10% lookups for eval on x86_64-linux, along with 53MiB memory (over 1%).
For some reason lists concats go 14% up, but defaultConfigurePlatformsFlags optimisation removes ~7% of them.

Overall time is down ~3%.

Things done

Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

philiptaron · 2025-08-11T17:52:24Z

I still have this on my list to review, but I won't be able to be at a computer until a little later this week. These sorts of PRs need careful attention…Although I have a strong belief that you have not introduced any regressions with this change.

philiptaron

Requesting changes only for the defaultConfigurePlatformsFlags searching and the system hoisting. Everything else is me puzzling over how this whole thing came to be.

pkgs/stdenv/generic/make-derivation.nix

philiptaron · 2025-08-12T17:03:55Z

pkgs/stdenv/generic/make-derivation.nix

This isn't true anymore, based on the usage of ++ below. That would cause an eval failure.

Yeah, I don't know where it came from. Maybe this was the case in the days of yore.

I'd prefer to remove it in this PR, since we're mutating configureFlags so much. I leave that preference in your hands.

Ah, sorry, I wasn't fast enough here. I guess, such trivial change can be done any time later :)

pkgs/stdenv/generic/make-derivation.nix

philiptaron · 2025-08-12T17:18:44Z

pkgs/stdenv/generic/make-derivation.nix

We can avoid this list concatenation in the common case by testing the following booleans.

attrs ? configureFlags: the derivation specified configureFlags

attrs ? configurePlatforms: the derivation specified configurePlatforms

The common case is !attrs ? configureFlags and !attrs ? configurePlatforms, which ought to just be defaultConfigurePlatformsFlags.

Here's the truth table that I'm thinking of.

attrs ? configureFlags attrs ? configurePlatforms Meaning Result

false false Defaults in every case defaultConfigurePlatformsFlags

false true Non-default platforms, need to test for target Do elem "target" logic on the platforms

true false Some flags are set, default platforms (other common case) configureFlags ++ defaultConfigurePlatformsFlags

true true Rare case configureFlags plus the elem target logic.

What do you think?

Do elem "target" logic on the platforms

Note that we'll need to do this logic for all variants, not just for "target".
I also wonder if this elem logic could be replaced with one concatMap instead. It might change the order of arguments in some cases not present in Nixpkgs.
Overall I'd prefer to do this optimisation separately to see its effect in isolation.

In this case, because the logic is so mutated in this PR, and there was this line in the headmatter:

For some reason lists concats go 14% up, but defaultConfigurePlatformsFlags optimisation removes ~7% of them.

I suspect all of these list concats are here, in this logic, and most could be removed through judicious refactors of logic.

That's why I was nudging for something like this truth table be extracted into a set of Nix expressions and used in this PR, to make it be even more a Pareto improvement.

I suspect all of these list concats are here, in this logic, and most could be removed through judicious refactors of logic.

But I strictly reduced the overall number of concats here: it was 3 for all derivations, and even before defaultConfigurePlatformsFlags it remained 3, with this optimisation, it's only 3 for ~100 or so packages that set configurePlatforms. All this means that I missed some list concats somewhere, but not here...

All lookups and expressions that don't depend on derivation attrs are moved to the outer let block. Saves over 10% lookups for eval on x86_64-linux, along with 53MiB memory (over 1%). For some reason lists concats go 14% up, but defaultConfigurePlatformsFlags optimisation removes ~7% of them. Overall time is down ~3%.

philiptaron

I still want the system thing changed the way I outline in this review, but I won't block the PR on that preference.

Strongly suspect that the change to use if ... then ... else ... instead of optional* will be net positive in this file.

philiptaron · 2025-08-22T15:49:44Z

pkgs/stdenv/generic/make-derivation.nix

I'd prefer to remove it in this PR, since we're mutating configureFlags so much. I leave that preference in your hands.

philiptaron · 2025-08-22T15:56:35Z

pkgs/stdenv/generic/make-derivation.nix

In this case, because the logic is so mutated in this PR, and there was this line in the headmatter:

For some reason lists concats go 14% up, but defaultConfigurePlatformsFlags optimisation removes ~7% of them.

I suspect all of these list concats are here, in this logic, and most could be removed through judicious refactors of logic.

That's why I was nudging for something like this truth table be extracted into a set of Nix expressions and used in this PR, to make it be even more a Pareto improvement.

philiptaron · 2025-08-22T15:57:26Z

pkgs/stdenv/generic/make-derivation.nix

+  stdenvHasCC = stdenv.hasCC;
+  stdenvShell = stdenv.shell;
+
+  buildPlatformSystem = buildPlatform.system;


Suggested change

buildPlatformSystem = buildPlatform.system;

philiptaron · 2025-08-22T15:58:07Z

pkgs/stdenv/generic/make-derivation.nix

            # attribute of `buildPlatform` matches Nix's degree of specificity.
            # exactly.
-            inherit (stdenv.buildPlatform) system;
+            system = buildPlatformSystem;


Suggested change

system = buildPlatformSystem;

inherit (buildPlatform) system;

My mental model of how Nix holds references says that this inherit both matches the comment and is at least as good as the extraction into the let block, possibly better.

Huh, it would be interesting to see the effect of such change. I thought that inherit (x) y; is strictly equivalent to y = x.y, and Nix doesn't try to optimise x.y at all.

In fact, here: https://github.com/NixOS/nix/blob/218cd6c16c0981cc32a45e3a15be1d3c1a68eb85/src/libexpr/parser.y#L419-L422 the parser essentially replaces inherit (x) y; with x = x.y; and then in https://github.com/NixOS/nix/blob/218cd6c16c0981cc32a45e3a15be1d3c1a68eb85/src/libexpr/eval.cc#L1237-L1242 we allocate additional env (I don't quite understand, why) if there is any inherit (x) y attrs and then we do necessary binding at eval time. It means that my removing all "inherit from" attrs from this attrset we save one env allocation per call.

YorikSar

@philiptaron I'm sorry for taking too long to respond and thanks a lot for a quick merge. I hope I'll be able to find some time for deeper optimisations in this area in the future, this was fun.

YorikSar · 2025-08-28T12:04:40Z

pkgs/stdenv/generic/make-derivation.nix

I suspect all of these list concats are here, in this logic, and most could be removed through judicious refactors of logic.

But I strictly reduced the overall number of concats here: it was 3 for all derivations, and even before defaultConfigurePlatformsFlags it remained 3, with this optimisation, it's only 3 for ~100 or so packages that set configurePlatforms. All this means that I missed some list concats somewhere, but not here...

YorikSar · 2025-08-28T12:05:32Z

pkgs/stdenv/generic/make-derivation.nix

Ah, sorry, I wasn't fast enough here. I guess, such trivial change can be done any time later :)

YorikSar · 2025-08-28T12:07:11Z

pkgs/stdenv/generic/make-derivation.nix

            # attribute of `buildPlatform` matches Nix's degree of specificity.
            # exactly.
-            inherit (stdenv.buildPlatform) system;
+            system = buildPlatformSystem;


Huh, it would be interesting to see the effect of such change. I thought that inherit (x) y; is strictly equivalent to y = x.y, and Nix doesn't try to optimise x.y at all.

philiptaron · 2025-09-06T13:09:24Z

@YorikSar are you at NixCon 2025?

YorikSar · 2025-09-06T14:17:10Z

@philiptaron no, I’m not. I’ll also be barely available in the coming month.

YorikSar requested review from infinisil and philiptaron August 7, 2025 13:50

nix-owners bot requested review from Artturin, Ericson2314, RossComputerGuy, emilazy and reckenrode August 7, 2025 13:51

nixpkgs-ci 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. 6.topic: stdenv Standard environment labels Aug 7, 2025

github-project-automation bot added this to Stdenv Aug 7, 2025

YorikSar mentioned this pull request Aug 7, 2025

stdenv: Replace "// optionalAttrs" with nullable attr name #430969

Merged

1 task

philiptaron requested a review from wolfgangwalther August 7, 2025 15:39

philiptaron requested changes Aug 12, 2025

View reviewed changes

github-project-automation bot moved this to In Progress in Stdenv Aug 12, 2025

YorikSar force-pushed the mkderivation-stdenv-lookup-optimisation branch from a29daaa to 7151ae1 Compare August 22, 2025 13:31

philiptaron approved these changes Aug 22, 2025

View reviewed changes

nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 22, 2025

philiptaron merged commit 78256f5 into NixOS:master Aug 27, 2025
26 of 28 checks passed

github-project-automation bot moved this from In Progress to Done in Stdenv Aug 27, 2025

YorikSar deleted the mkderivation-stdenv-lookup-optimisation branch August 28, 2025 12:01

YorikSar commented Aug 28, 2025

View reviewed changes

philiptaron mentioned this pull request Aug 28, 2025

ci/eval: add a shebang to the stats comparison script #437697

Closed

2 tasks

YorikSar mentioned this pull request Nov 3, 2025

Reapply "stdenv: Add CPE fields to meta" #439074

Merged

`attrs ? configureFlags`	`attrs ? configurePlatforms`	Meaning	Result
`false`	`false`	Defaults in every case	`defaultConfigurePlatformsFlags`
`false`	`true`	Non-default platforms, need to test for target	Do `elem "target"` logic on the platforms
`true`	`false`	Some flags are set, default platforms (other common case)	`configureFlags ++ defaultConfigurePlatformsFlags`
`true`	`true`	Rare case	`configureFlags` plus the elem target logic.

	system = buildPlatformSystem;
	inherit (buildPlatform) system;

Uh oh!

Conversation

YorikSar commented Aug 7, 2025

Things done

Uh oh!

philiptaron commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philiptaron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YorikSar Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philiptaron left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

YorikSar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philiptaron commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YorikSar commented Sep 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

philiptaron commented Aug 11, 2025 •

edited

Loading

YorikSar Aug 22, 2025 •

edited

Loading

philiptaron commented Sep 6, 2025 •

edited

Loading