Parallel eval improvements#534
Draft
edolstra wants to merge 13 commits into
Draft
Conversation
The previous implementation used a std::map with traceable_allocator, which costs one GC_MALLOC_UNCOLLECTABLE() and one GC_FREE() per distinct attribute name. Uncollectable allocations always take the global GC allocation lock, making this map a major source of mutex contention during parallel evaluation (~12% of all futex calls in a 16-core 'nix search' run, plus the corresponding GC_free traffic). Instead, collect the (name, value) pairs into a plain std::vector and group them by stable-sorting on the name, which yields the same attribute order and per-attribute value order as the map. The vector doesn't need to be visible to the GC since the values it points to are kept alive by the attrsets in args[1] for the duration of the call. At 16 eval cores (with GC disabled via GC_INITIAL_HEAP_SIZE=40G), this cuts kernel time from ~21s to ~8-9s, context switches from 2.6M to 1.5M, and elapsed time from ~6.2s to ~4.9s for 'nix search nixpkgs --no-eval-cache fizzbuzz'. Single-core performance is unchanged. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Previously every RootValue was created with std::allocate_shared<Value *>(traceable_allocator<Value *>()), which costs a GC_MALLOC_UNCOLLECTABLE() / GC_FREE() pair per root value. Uncollectable allocations always take the global GC allocation lock, making this a significant source of mutex contention during parallel evaluation: the eval cache allocates a root value per AttrCursor (i.e. per attribute visited by 'nix search'), and the parallel evaluator allocates one per queued work item. Instead, carve root slots out of large uncollectable slabs (which are permanently part of the GC root set) and recycle them through a free list protected by a plain mutex, whose critical section is a few instructions rather than the entire GC allocator. Slots are cleared on release so they don't keep values alive; liveness semantics are unchanged. The slabs are never freed, so pool memory is bounded by the peak number of simultaneously live root values (8 bytes per slot). At 16 eval cores (with GC disabled via GC_INITIAL_HEAP_SIZE=40G), this cuts kernel time from ~8-9s to ~7s, context switches from 1.46M to 0.99M, and elapsed time from ~4.9s to ~4.3-4.5s for 'nix search nixpkgs --no-eval-cache fizzbuzz'. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
This makes a huge performance impact once GC kicks in (e.g. it speeds up a 12-core `nix search nixpkgs` from 10.5 to 6.5 seconds).
Add a patch to boehmgc that makes GC_generic_malloc_many() hand out up to GC_many_blocks heap blocks worth of objects per acquisition of the global GC allocation lock, instead of exactly one 4 KiB block. All evaluator threads replenish their thread-local free lists (Values, Envs, Bindings) through this function, so during parallel evaluation they otherwise serialize on the allocation lock: at 12 eval cores, batching 64 blocks reduces futex syscalls by 6x (3.58M to 0.58M) for 'nix search nixpkgs --no-eval-cache fizzbuzz', and reduces both kernel time and elapsed time at higher core counts. The batch size defaults to GC_MANY_BLOCKS_DEFAULT (set to 64 here) and can be overridden at runtime via the GC_MALLOC_MANY_BLOCKS environment variable (1-64, where 1 restores the upstream behavior). Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Off-CPU profiling showed that ~2 of 12 worker threads were idle on average during the parallel phase of 'nix search', because work items were only spawned after fully enumerating each attrset: while one thread enumerated legacyPackages.x86_64-linux (~120k attributes), and later each large package subset (pythonPackages, perlPackages, ...), the other workers had nothing new to pick up. Two changes: * Spawn work items incrementally (every 256 attributes) during enumeration instead of in one batch at the end, so idle workers can start on the first attributes while enumeration continues. * Executor::spawn(): Generate queue keys with a thread-local mt19937_64 instead of calling std::random_device per work item, which costs hundreds of cycles per call (RDRAND / /dev/urandom). The key only needs to spread same-priority items around the queue, not be cryptographically random. At 12 eval cores (with GC disabled via GC_INITIAL_HEAP_SIZE=40G), this reduces elapsed time from ~4.6s to ~4.3s, user CPU from ~28s to ~26s, and kernel time from ~5s to ~3.9s for 'nix search nixpkgs --no-eval-cache fizzbuzz'. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Previously every work item had its own std::promise/std::future pair. Fulfilling a promise performs an unconditional futex wake syscall even when nobody is blocked on the future (which is almost always the case here, since FutureVector only awaits completion in aggregate), and the shared state is a heap allocation, freed from many threads and thus contending on the glibc malloc arena locks. With hundreds of thousands of work items per evaluation, this accounted for ~100k futex syscalls per 'nix search' run. Instead, work items now share a single Completion object per FutureVector, consisting of an atomic pending counter, a list of exceptions thrown by work items, and a condition variable that is only signalled when the counter drops to zero. finishAll() waits for that and then rethrows the first recorded exception, preserving the previous semantics (log all but the first, unless interrupted). Spawns without a FutureVector (e.g. builtins.parallel) pass a null completion and remain fire-and-forget. At 12 eval cores (with GC disabled via GC_INITIAL_HEAP_SIZE=40G and GC_MALLOC_MANY_BLOCKS=64), this halves context switches (650-770k to 273-420k) and removes ~100k futex calls for 'nix search nixpkgs --no-eval-cache fizzbuzz'. Elapsed time is unchanged on 12 physical cores; the reduced lock traffic is headroom for higher core counts. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
edolstra
added
the
flake-regression-test
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Evaluating an empty attrset literal wrote its position into
Bindings::emptyBindings, the shared static object that
EvalMemory::allocBindings() returns for zero-capacity bindings. Under
parallel evaluation this is a data race (the position of every '{ }'
was whichever one was evaluated last), and 'perf c2c' on an Intel
i7-1260P showed it also causes false sharing: emptyBindings happens
to share a cache line with Counter::enabled, which is read by every
thread in allocValue()/callFunction()/maybeThunk(), so each write
invalidated that line across all cores.
Skip the write for the shared empty bindings; '{ }' now has a
deterministic (undefined) position instead of a racy one, and the
cache line stays clean. Verified with perf c2c that the line no
longer appears among contended cache lines.
Assisted-by: Claude Fable 5 <noreply@anthropic.com>
perf c2c on an Intel i7-1260P showed contention on the cache line at the start of SourceAccessor objects: make_ref/make_shared co-locates the std::shared_ptr control block with the object, so the atomic reference count (updated by every SourcePath copy, on every thread) shared a cache line with the object's vtable pointer and `number` field, which are read by every virtual accessor call (maybeLstat/resolveSymlinks/isAllowed/...) and every SourcePath hash. Each refcount update thus invalidated the line needed for all file access dispatch on all other cores. Aligning SourceAccessor to the cache line size pushes the object onto its own line(s), leaving only the control block on the shared line. Verified with perf c2c that accessor field/vtable reads no longer appear on contended lines; the remaining (much smaller) contention is the reference count itself. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
resolveExprPath() constructed three SourcePath temporaries per loop iteration (parent(), resolveSymlinks(), operator/), and parseExprFromFile() one per file, each copying the accessor ref (an atomic refcount update on a cache line shared between all threads) and the path string. Work on the accessor and CanonPaths directly instead. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
edolstra
force-pushed
the
parallel-eval-improvements
branch
from
3088a34 to
edf6271
Compare
This was referenced Jul 6, 2026
Apply the same optimization to builtins.groupBy as to builtins.zipAttrsWith (162a34378c): instead of accumulating the groups in a std::map<Symbol, ValueVector> with traceable_allocator - which costs a GC_MALLOC_UNCOLLECTABLE() per distinct name plus traceable growth allocations for every per-group vector, all taking the global GC allocation lock - collect the (name, value) pairs into a plain std::vector and group them by stable-sorting on the name. The vector doesn't need to be visible to the GC since the values it points to are kept alive by the list in args[1] for the duration of the call. The shared machinery (the NameValue type, sorting/counting the names, and iterating over the per-name runs) is factored out into helpers used by both primops. This removes the last user of ValueVectorMap, so drop that typedef. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
RootValue (std::shared_ptr<Value *>) adds avoidable overhead per GC
root: a malloc'd control block, atomic reference count updates on
copies (a measured source of cache line contention during parallel
evaluation), and 16 bytes per handle. Most users never copy their
roots, so introduce UniqueRootValue, a move-only RAII wrapper around
a pointer to a root slot that returns the slot to the pool on
destruction.
The pool internals move from allocRootValue() into
allocRootValueSlot() / freeRootValueSlot(), shared by both handle
types, and the whole root value machinery moves into new files
root-value.{hh,cc}. The non-Boehm fallback now also goes through the
slot functions (new/delete) instead of a make_shared special case.
Converted to UniqueRootValue: ValMap, fileEvalCache, internalPrimOps,
genericClosure's work/result lists, EvalCache::value,
AttrCursor::_value, InstallableAttrPath::v, and the JSON parser
state. RootValue remains (documented as such) for roots captured in
std::function-backed lambdas, which require copyability.
Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Replace the Sync<std::vector<Value **>> free slot pool with an intrusive linked list: each slot is a union of Value * (in use) and a pointer to the next free slot, with a Sync<Slot *> head. Allocation pops the head; freeing pushes in O(1). This avoids the auxiliary vector memory (8 bytes per free slot), the 4096-iteration push_back loop when carving a new slab, and most importantly the possibility of a vector reallocation (a malloc) while holding the pool lock on the free path. GC safety is unchanged: slabs remain uncollectable and conservatively scanned. In-use slots contain a Value * and root the value; free slots contain a pointer into a slab (or null), which is scanned harmlessly since slabs are never freed anyway. Writing the next pointer on free overwrites the Value *, which doubles as the "stop rooting the value" step. Assisted-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Motivation
Various vibe-coded parallel eval improvements. Together they give a significant improvement to elapsed time and user/kernel time (on
nix search nixpkgs):Context