Add combined AddTempRootReturningPathInfo op to speed up nix eval

[bouk]

Motivation

writeDerivation() currently makes 2 separate daemon IPC round-trips per derivation: addTempRoot then isValidPath. For a NixOS system with thousands of derivations, this adds a lot of IPC overhead.

This adds a new Op::AddTempRootReturningPathInfo that combines both operations into a single round-trip. writeDerivation() now uses addTempRootReturningPathInfo() which for RemoteStore uses the combined op when the daemon supports it, and falls back to sequential calls on the same connection otherwise.

Context

I was profiling the nix eval of a big system we have and figured out that nix eval --read-only was taking ~9 seconds but nix eval was taking 20 seconds.

With the help of Claude I figured out that this was caused by IPC overhead, writeDerivation does two IPCs which adds up since in this case the NixOS system has +8000 derivations. On my system (Apple M3 Pro MacBook Pro, macOS 26.3.1) the overhead per IPC is about 1ms(!).

This PR combines two IPCs into one, which in my case brought nix eval from 20 seconds to 15 seconds, 25% faster! I think we could bring it down even more by putting the writeDerivations into a background queue or something so the eval path isn't dependent on the IPC latency, but that would be better as a separate PR.

Improvement

Command	2.34.2+1	This PR
nix eval --read-only	9s	9s
nix eval	20s	12.5s

---

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

[edolstra]

The general idea looks good to me, but it may be better to combine this with QueryPathInfo rather than IsValidPath. The former is better because it can be cached by pathInfoCache. isValidPath() can use but not populate the pathInfoCache. This is especially a problem when using lazy trees (see DeterminateSystems#157).

Note that the overhead of the daemon calls is much less an issue when using the async path writer (#13798) because then the evaluator doesn't need to wait for the calls to addTempRoot() / isValidPath().

[xokdvium]

Yup, async writing from the evaluator is definitely on the menu. Note that this can also be applied to all the other cases where we cache things client-side (like fetching sources to the store).

[bouk]

Ah great, #13798 is exactly the next step I had in mind.

I can change this PR to combine it with QueryPathInfo, it still makes the async path writer faster.

[bouk]

Does this work correctly? I don't use C++ often

[bouk]

@edolstra I've amended the PR to query path info instead

[bouk]

The ...ReturningPathInfo version seems to be even faster, now 12.5s when checking against the big NixOS system I have locally (not a very controlled benchmark tbh!)

[bouk]

The tests are failing after switching to AddTempRootReturningPathInfo but I don't really understand why :(

[bouk]

I've not been able to reproduce the CI failure locally, not sure how to proceed. Someone with deeper knowledge of nix internals would be more qualified to take a stab at this

[tomberek]

I get the same error as CI:

nix-functional-tests> 165/208 nix-functional-tests:local-overlay-store / delete-refs                       FAIL            0.78s   exit status 1                                                            
...
      > 159/208 nix-functional-tests:local-overlay-store / build                    FAIL            0.82s   exit status 1                                                                                  
       > 165/208 nix-functional-tests:local-overlay-store / delete-refs              FAIL            0.78s   exit status 1                                                                                  
       > 172/208 nix-functional-tests:local-overlay-store / gc                       FAIL            0.78s   exit status 1                                                                                  
   

I'm pretty sure you don't run those on OSX, explaining why it didn't fail in your local case. EIther way, it seems related to being able to check for validity in both the upper and lower stores and that handling needs some care.

[bouk]

I've fixed CI, it seems my change unearthed a problem in queryPathInfoUncached. Please let me know if this fix is good

[Ericson2314]

I have talked to @xokdvium about making a write back cache store rather than introducing a bunch of new operations to accelerate eval. (See also the similar PR from Eelco.)