From ea60a6a0dd39b19dc093a24fe6301b76f2d96282 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 6 Jul 2026 13:17:50 +0200 Subject: [PATCH] ExprAttrs::eval: Don't write to the shared empty Bindings 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 --- src/libexpr/eval.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 9c10a9b3a6a7..2cea7f1a3600 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1366,7 +1366,12 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) sort = true; } - bindings.bindings->pos = pos; + /* Empty attrsets share the static Bindings::emptyBindings, which we + must not write to: apart from being a data race, it causes false + sharing on emptyBindings' cache line (which may also hold other hot + globals such as Counter::enabled) between all evaluator threads. */ + if (bindings.bindings != &Bindings::emptyBindings) + bindings.bindings->pos = pos; v.mkAttrs(sort ? bindings.finish() : bindings.alreadySorted()); }