Skip to content

Conversation

@pkpbynum
Copy link
Contributor

@pkpbynum pkpbynum commented Nov 20, 2025

Motivation

Note: I'm not much of a C dev (yet), and don't know enough about the codebase to know if this is "safe."

I found this while looking at slower-than-expected HTTP Binary Cache store query times. This line immediately blocks on the queryPathInfo promise when it is passed from computeFSClosure here. In the context of an HTTP binary cache store, this prevents HTTP requests from happening concurrently (HTTP2 or HTTP1).

computeFSClosure and computeClosure are used in quite a few places--including path-info, which I used to benchmark just by querying the Linux stdenv build-time closure & adjusting narinfo-cache-positive-ttl:

Before:

$ time nix path-info --store https://cache.nixos.org --recursive --option narinfo-cache-positive-ttl 10 /nix/store/7zz3zmv2a0ssmgqlfhy4rsb6ii6z475a-stdenv-linux.drv
... paths output
real    0m6.904s
user    0m0.314s
sys     0m0.209s

After:

$ time ./src/nix/nix path-info --store https://cache.nixos.org --recursive --option narinfo-cache-positive-ttl 10 /nix/store/7zz3zmv2a0ssmgqlfhy4rsb6ii6z475a-stdenv-linux.drv
... paths output
real    0m0.395s
user    0m0.097s
sys     0m0.072s

Context


Add 👍 to pull requests you find important.

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

@pkpbynum pkpbynum requested a review from edolstra as a code owner November 20, 2025 21:08
@xokdvium
Copy link
Contributor

Threading works in a pinch, but the underlying issue is that queryPathInfo should be async instead.

Sorry for barging in with a much bigger project, that is not blocking in any way if we decide that plugging this issue with threads is the right call.

I've been dying to get started on improving the status quo, so I've filed #14612 with some of my thoughts.

@pkpbynum
Copy link
Contributor Author

Threading works in a pinch, but the underlying issue is that queryPathInfo should be async instead.

Sorry for barging in with a much bigger project, that is not blocking in any way if we decide that plugging this issue with threads is the right call.

I've been dying to get started on improving the status quo, so I've filed #14612 with some of my thoughts.

100%. FWIW I tested an implementation that just "fixed" the usage of the callback in this one spot, but it only had ~20% perf improvement. I imagine there was something downstream that was still blocking that I didn't bother to debug. Let me know if there are areas I can help on #14612 !

@xokdvium
Copy link
Contributor

Let me know if there are areas I can help on #14612 !

Probably the first step would be to:

  1. land on the coroutine helper library, since c++ standard library is flacid as always
  2. work from the grounds up. I somewhat non-painful way would be to convert the libcurl (FileTransfer & filetransfer.cc) completion callbacks into a coroutine and start from there.

If you have experience with practical C++20 coroutines at scale that would help a lot. A somewhat easy path would be to stick to kj-async, since there's prior art from the lix project, but adding a new dependency is a tall ask.

Either way all this depends on a agreed upon solution from the maintainer team, so I've put that up for triage and will bring up during the next meeting. Having input on various approaches we could take would be invaluable tho. Identifying code paths that block a lot would be nice as well, so that we could focus on the larger issues first.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants