From e8f951289fa44bc36ead0d51b283f09ecac9103b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 25 Aug 2025 20:06:00 +0200 Subject: [PATCH] EvalState: Don't maintain stats by default These counters are extremely expensive in a multi-threaded program. For instance, disabling them speeds up evaluation of the NixOS/nix/2.21.2 from 32.6s to 17.8s. --- src/libexpr/eval.cc | 11 ++-- src/libexpr/include/nix/expr/counter.hh | 70 ++++++++++++++++++++++++ src/libexpr/include/nix/expr/eval.hh | 27 ++++----- src/libexpr/include/nix/expr/meson.build | 1 + src/libexpr/include/nix/expr/nixexpr.hh | 3 +- src/libexpr/nixexpr.cc | 2 +- 6 files changed, 94 insertions(+), 20 deletions(-) create mode 100644 src/libexpr/include/nix/expr/counter.hh diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index a953e20d7..0ec819809 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -287,6 +287,7 @@ EvalState::EvalState( assertGCInitialized(); static_assert(sizeof(Env) <= 16, "environment must be <= 16 bytes"); + static_assert(sizeof(Counter) == 64, "counters must be 64 bytes"); /* Construct the Nix expression search path. */ assert(lookupPath.elements.empty()); @@ -892,7 +893,7 @@ Value * EvalState::getBool(bool b) return b ? &Value::vTrue : &Value::vFalse; } -static std::atomic nrThunks = 0; +static Counter nrThunks; static inline void mkThunk(Value & v, Env & env, Expr * expr) { @@ -2891,11 +2892,11 @@ bool EvalState::fullGC() #endif } +bool Counter::enabled = getEnv("NIX_SHOW_STATS").value_or("0") != "0"; + void EvalState::maybePrintStats() { - bool showStats = getEnv("NIX_SHOW_STATS").value_or("0") != "0"; - - if (showStats) { + if (Counter::enabled) { // Make the final heap size more deterministic. #if NIX_USE_BOEHMGC if (!fullGC()) { @@ -2944,7 +2945,7 @@ void EvalState::printStatistics() {"elements", nrValuesInEnvs.load()}, {"bytes", bEnvs}, }; - topObj["nrExprs"] = Expr::nrExprs; + topObj["nrExprs"] = Expr::nrExprs.load(); topObj["list"] = { {"elements", nrListElems.load()}, {"bytes", bLists}, diff --git a/src/libexpr/include/nix/expr/counter.hh b/src/libexpr/include/nix/expr/counter.hh new file mode 100644 index 000000000..efbf23de3 --- /dev/null +++ b/src/libexpr/include/nix/expr/counter.hh @@ -0,0 +1,70 @@ +#pragma once + +#include +#include + +namespace nix { + +/** + * An atomic counter aligned on a cache line to prevent false sharing. + * The counter is only enabled when the `NIX_SHOW_STATS` environment + * variable is set. This is to prevent contention on these counters + * when multi-threaded evaluation is enabled. + */ +struct alignas(64) Counter +{ + using value_type = uint64_t; + + std::atomic inner{0}; + + static bool enabled; + + Counter() {} + + operator value_type() const noexcept + { + return inner; + } + + void operator=(value_type n) noexcept + { + inner = n; + } + + value_type load() const noexcept + { + return inner; + } + + value_type operator++() noexcept + { + return enabled ? ++inner : 0; + } + + value_type operator++(int) noexcept + { + return enabled ? inner++ : 0; + } + + value_type operator--() noexcept + { + return enabled ? --inner : 0; + } + + value_type operator--(int) noexcept + { + return enabled ? inner-- : 0; + } + + value_type operator+=(value_type n) noexcept + { + return enabled ? inner += n : 0; + } + + value_type operator-=(value_type n) noexcept + { + return enabled ? inner -= n : 0; + } +}; + +} // namespace nix diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index 958b6fbee..1c2552991 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -16,6 +16,7 @@ #include "nix/expr/search-path.hh" #include "nix/expr/repl-exit-status.hh" #include "nix/util/ref.hh" +#include "nix/expr/counter.hh" // For `NIX_USE_BOEHMGC`, and if that's set, `GC_THREADS` #include "nix/expr/config.hh" @@ -961,19 +962,19 @@ private: */ std::string mkSingleDerivedPathStringRaw(const SingleDerivedPath & p); - std::atomic nrEnvs = 0; - std::atomic nrValuesInEnvs = 0; - std::atomic nrValues = 0; - std::atomic nrListElems = 0; - std::atomic nrLookups = 0; - std::atomic nrAttrsets = 0; - std::atomic nrAttrsInAttrsets = 0; - std::atomic nrAvoided = 0; - std::atomic nrOpUpdates = 0; - std::atomic nrOpUpdateValuesCopied = 0; - std::atomic nrListConcats = 0; - std::atomic nrPrimOpCalls = 0; - std::atomic nrFunctionCalls = 0; + Counter nrEnvs; + Counter nrValuesInEnvs; + Counter nrValues; + Counter nrListElems; + Counter nrLookups; + Counter nrAttrsets; + Counter nrAttrsInAttrsets; + Counter nrAvoided; + Counter nrOpUpdates; + Counter nrOpUpdateValuesCopied; + Counter nrListConcats; + Counter nrPrimOpCalls; + Counter nrFunctionCalls; bool countCalls; diff --git a/src/libexpr/include/nix/expr/meson.build b/src/libexpr/include/nix/expr/meson.build index 04f8eaf71..44ff171c2 100644 --- a/src/libexpr/include/nix/expr/meson.build +++ b/src/libexpr/include/nix/expr/meson.build @@ -10,6 +10,7 @@ config_pub_h = configure_file( headers = [ config_pub_h ] + files( 'attr-path.hh', 'attr-set.hh', + 'counter.hh', 'eval-cache.hh', 'eval-error.hh', 'eval-gc.hh', diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh index 7721918c3..e0203c732 100644 --- a/src/libexpr/include/nix/expr/nixexpr.hh +++ b/src/libexpr/include/nix/expr/nixexpr.hh @@ -9,6 +9,7 @@ #include "nix/expr/symbol-table.hh" #include "nix/expr/eval-error.hh" #include "nix/util/pos-idx.hh" +#include "nix/expr/counter.hh" namespace nix { @@ -92,7 +93,7 @@ struct Expr Symbol sub, lessThan, mul, div, or_, findFile, nixPath, body; }; - static unsigned long nrExprs; + static Counter nrExprs; Expr() { diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index c0a25d1d4..43e85cb16 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -11,7 +11,7 @@ namespace nix { -unsigned long Expr::nrExprs = 0; +Counter Expr::nrExprs; ExprBlackHole eBlackHole;