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
replace in-memory nar caching with on-disk nar caching when >256MB #2981
Conversation
This is still a StringSource, and should be fixed
Hm, I did something similar a while ago in cf4a7ae. I would prefer that one since it's a lot simpler (it touches only 2 files rather than 31). |
Most of the changes to other files are simple refactorings (
|
Confirm: it work as expected for me. |
@edolstra any progress here/chance to integrate? Nix from this PR just work for me. |
@edolstra any progress on merging/adopting it? AFAIK that fixes long-term nix issue with disability to store big files, which could be pretty useful in nix-as-a-development-environment workflow, where bunch of language-specific libraries yet installed by language-specific tool and reused as artifacts. |
I'd suggest openining a separate PR for refactoring and rebasing the rest on top. |
I marked this as stale due to inactivity. → More info |
Completely diverged with master. |
Nix runs out of memory when dumping large files on systems without enough ram, and the whole saving and hashing is not as efficient as it could be (multiple passes through files, multiple copies). I'm out of time for now, but this fixes all the main problematic cases. It would be very cool to just pass FD's over to the nix daemon where possible, zero-copying into the nix store on filesystems with reflinks.
The one in
RemoteFSAccessor::fetch
still remains.In more detail, this introduces a
HashedBufferSink
sink that buffers data in memory, then switches to a file when the data gets too big. Afterwards, it can be converted to a Source. It also keeps a hash of the data. It also switches some of the code tostd::string_view
, so we can use file-backed memory via mmap there.There also seemed to be an
accessor
argument toaddToStore
that was never actually used and complicated theBinaryCacheStore
code quite a bit, I've removed that.These changes do not seem to be covered well by the tests, but I've run some builds and
nix add-to-store
's to verify the output and behavior is the same (minus the running out of memory).cc @Pitometsu