-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
libfetchers: avoid re-copying substituted inputs #14041
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libfetchers: avoid re-copying substituted inputs #14041
Conversation
|
This doesn't seem right to me, since it leads to an input returning a different fingerprint depending on whether it's in the Nix store or not. |
5e90b64 to
c17e3ba
Compare
|
Took a different approach by just copying the fix from #12911 directly here instead of trying to re-use the whole code path. Seems to have the same effect! |
Previously, Nix would not create a cache entry for substituted/cached inputs This led to severe slowdowns in some scenarios where a large input (like Nixpkgs) had already been unpacked to the store but didn't exist in a users cache, as described in NixOS#11228 Using the same method as NixOS#12911, we can create a cache entry for the fingerprint of substituted/cached inputs and avoid this problem entirely
c17e3ba to
74305d5
Compare
|
Added a release note as well, since it seems this bug has a bit of a following 😄 |
|
This will be better fixed by #14050, right? |
|
I actually don't know. @edolstra ? |
|
While #14050 is an obvious improvement to This sorta makes sense? As it only touches the unsubstituted code path there, plus the explicit caching originally introduced in #12911 remains Now I don't really know much about libfetchers yet (or how Nix works generally...), so take this with a grain of salt: I'm pretty sure this patch is still necessary since the accessor we're constructing here hasn't undergone any previous fingerprinting or received an entry to the cache. This ends up similar to what the regular |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Motivation
Previously, Nix would not create a cache entry for substituted/cached
inputs
This led to severe slowdowns in some scenarios where a large input (like
Nixpkgs) had already been unpacked to the store but didn't exist in a
users cache, as described in #11228
In my testing (on WSL) the speedup can be pretty drastic. As an example, here's
running
nix evalin this repo after already ensuring inputs are in the storewith
nix flake prefetch-inputsBefore:
After:
This is a ~91% increase in performance!
Speaking of
nix flake prefetch-inputs, it seems this bug also affected it, and paths would be re-copied on any uncached invocation. In practice on my machine, this made a run of take 45.5s with this repo's flake (after runningrm -rf ~/.cache/nix) even though all inputs were already in the store. With this PR, it now takes about 300ms (...and I assume this is just evaluating the flake itself)Context
Hopefully fixes #11228
This is basically the fix introduced in #12911, but adopted in a different, similarly affected code path
I'm not exactly sure if this the best solution to do that...but from what Iunderstand, I think it should be safe to keep the original
Inputinformationand pair it with an accessor with a fingerprint of a technically different
input. After all, they should have the same contents given the same hash, and
attributes like
lastModifiedgo unaffected as they aren't stored in theaccessor anyways
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.