Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The C++ serializer emits the NAR incrementally and calls back into Rust for each piece. Every callback does a
Vec::from(data), so each one becomes its own heap allocation. The stream contains a lot of metadata along with the ~64 KiB content chunks, which means we end up doing a huge number of allocations of all sorts of different sizes.Attic is not asking the kernel for that memory directly, it goes through glibc, which splits the heap into arenas to reduce lock contention between threads (up to 8 x nproc of them). Freeing a Vec does not mean the memory is returned to the OS. With lots of differently sized allocations, the arenas become fragmented, which makes it harder for glibc to hand large chunks back. So from Attic's point of view the objects have been freed, but from the kernel's point of view the process can still own a large amount of memory. On a machine with a high core count and a lot of store paths going through
watch-store, that adds up to tens of GiB.The other half of the problem is that the channel between the serializer and the uploader is unbounded, so there is no back pressure. The producer runs at disk speed while the consumer is limited by network speed, and any difference between the two just gets queued.
The patch fixes this where the NAR crosses from the serializer into the uploader. Instead of turning every callback into its own Vec, the sender combines the incoming pieces into uniformly sized buffers of
NAR_CHUNK_SIZEand only sends when one is full, flushing the remainder at EOF. The allocator then mostly sees buffers of the same size, which is a much more regular pattern and avoids the fragmentation. The channel is also made bounded, so the serializer has to wait onceNAR_CHANNEL_CAPACITYbuffers are queued instead of running ahead indefinitely.That gives an actual upper bound on what can sit between serialization and upload:
So the two parts are:
Note that #362 proposes the bounded channel half of this. This includes that and adds the coalescing, which covers the fragmentation side that bounding alone does not: with a Vec per callback the queue is capped but the churn remains.