From 121a8ab3ec4b161cf6e24ebb0251127060def557 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Wed, 24 Sep 2025 19:09:21 -0700 Subject: [PATCH 1/3] shellcheck fix functional/dependencies.builder0.sh --- maintainers/flake-module.nix | 1 - tests/functional/dependencies.builder0.sh | 16 ++++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index a5360675f..3b2ab6785 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -113,7 +113,6 @@ ''^tests/functional/config\.sh$'' ''^tests/functional/db-migration\.sh$'' ''^tests/functional/debugger\.sh$'' - ''^tests/functional/dependencies\.builder0\.sh$'' ''^tests/functional/dependencies\.sh$'' ''^tests/functional/dump-db\.sh$'' ''^tests/functional/dyn-drv/build-built-drv\.sh$'' diff --git a/tests/functional/dependencies.builder0.sh b/tests/functional/dependencies.builder0.sh index 9b11576e0..6fbe4a07a 100644 --- a/tests/functional/dependencies.builder0.sh +++ b/tests/functional/dependencies.builder0.sh @@ -1,16 +1,20 @@ +# shellcheck shell=bash +# shellcheck disable=SC2154 [ "${input1: -2}" = /. ] +# shellcheck disable=SC2154 [ "${input2: -2}" = /. ] -mkdir $out -echo $(cat $input1/foo)$(cat $input2/bar) > $out/foobar +# shellcheck disable=SC2154 +mkdir "$out" +echo "$(cat "$input1"/foo)$(cat "$input2"/bar)" > "$out"/foobar -ln -s $input2 $out/reference-to-input-2 +ln -s "$input2" "$out"/reference-to-input-2 # Self-reference. -ln -s $out $out/self +ln -s "$out" "$out"/self # Executable. -echo program > $out/program -chmod +x $out/program +echo program > "$out"/program +chmod +x "$out"/program echo FOO From 8d257f5510dbfbab8a74e2b7b0ff60bcd720e141 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 3 Jul 2025 13:21:30 +0200 Subject: [PATCH 2/3] EvalState: Make the counters atomic --- src/libexpr/eval.cc | 30 ++++++++++++++-------------- src/libexpr/include/nix/expr/eval.hh | 26 ++++++++++++------------ 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 87b1e73a5..a953e20d7 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -892,7 +892,7 @@ Value * EvalState::getBool(bool b) return b ? &Value::vTrue : &Value::vFalse; } -unsigned long nrThunks = 0; +static std::atomic nrThunks = 0; static inline void mkThunk(Value & v, Env & env, Expr * expr) { @@ -2940,18 +2940,18 @@ void EvalState::printStatistics() #endif }; topObj["envs"] = { - {"number", nrEnvs}, - {"elements", nrValuesInEnvs}, + {"number", nrEnvs.load()}, + {"elements", nrValuesInEnvs.load()}, {"bytes", bEnvs}, }; topObj["nrExprs"] = Expr::nrExprs; topObj["list"] = { - {"elements", nrListElems}, + {"elements", nrListElems.load()}, {"bytes", bLists}, - {"concats", nrListConcats}, + {"concats", nrListConcats.load()}, }; topObj["values"] = { - {"number", nrValues}, + {"number", nrValues.load()}, {"bytes", bValues}, }; topObj["symbols"] = { @@ -2959,9 +2959,9 @@ void EvalState::printStatistics() {"bytes", symbols.totalSize()}, }; topObj["sets"] = { - {"number", nrAttrsets}, + {"number", nrAttrsets.load()}, {"bytes", bAttrsets}, - {"elements", nrAttrsInAttrsets}, + {"elements", nrAttrsInAttrsets.load()}, }; topObj["sizes"] = { {"Env", sizeof(Env)}, @@ -2969,13 +2969,13 @@ void EvalState::printStatistics() {"Bindings", sizeof(Bindings)}, {"Attr", sizeof(Attr)}, }; - topObj["nrOpUpdates"] = nrOpUpdates; - topObj["nrOpUpdateValuesCopied"] = nrOpUpdateValuesCopied; - topObj["nrThunks"] = nrThunks; - topObj["nrAvoided"] = nrAvoided; - topObj["nrLookups"] = nrLookups; - topObj["nrPrimOpCalls"] = nrPrimOpCalls; - topObj["nrFunctionCalls"] = nrFunctionCalls; + topObj["nrOpUpdates"] = nrOpUpdates.load(); + topObj["nrOpUpdateValuesCopied"] = nrOpUpdateValuesCopied.load(); + topObj["nrThunks"] = nrThunks.load(); + topObj["nrAvoided"] = nrAvoided.load(); + topObj["nrLookups"] = nrLookups.load(); + topObj["nrPrimOpCalls"] = nrPrimOpCalls.load(); + topObj["nrFunctionCalls"] = nrFunctionCalls.load(); #if NIX_USE_BOEHMGC topObj["gc"] = { {"heapSize", heapSize}, diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index 8f7a0ec32..958b6fbee 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -961,19 +961,19 @@ private: */ std::string mkSingleDerivedPathStringRaw(const SingleDerivedPath & p); - unsigned long nrEnvs = 0; - unsigned long nrValuesInEnvs = 0; - unsigned long nrValues = 0; - unsigned long nrListElems = 0; - unsigned long nrLookups = 0; - unsigned long nrAttrsets = 0; - unsigned long nrAttrsInAttrsets = 0; - unsigned long nrAvoided = 0; - unsigned long nrOpUpdates = 0; - unsigned long nrOpUpdateValuesCopied = 0; - unsigned long nrListConcats = 0; - unsigned long nrPrimOpCalls = 0; - unsigned long nrFunctionCalls = 0; + 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; bool countCalls; From e8f951289fa44bc36ead0d51b283f09ecac9103b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 25 Aug 2025 20:06:00 +0200 Subject: [PATCH 3/3] 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;