diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 9d33a895b..3929fd28a 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -107,7 +107,6 @@ excludes = [ # We haven't linted these files yet ''^scripts/install-systemd-multi-user\.sh$'' - ''^tests/functional/dependencies\.builder0\.sh$'' ''^tests/functional/dump-db\.sh$'' ''^tests/functional/dyn-drv/eval-outputOf\.sh$'' ''^tests/functional/dyn-drv/old-daemon-error-hack\.sh$'' diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 87b1e73a5..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; } -unsigned long 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()) { @@ -2940,18 +2941,18 @@ void EvalState::printStatistics() #endif }; topObj["envs"] = { - {"number", nrEnvs}, - {"elements", nrValuesInEnvs}, + {"number", nrEnvs.load()}, + {"elements", nrValuesInEnvs.load()}, {"bytes", bEnvs}, }; - topObj["nrExprs"] = Expr::nrExprs; + topObj["nrExprs"] = Expr::nrExprs.load(); 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 +2960,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 +2970,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/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 8f7a0ec32..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); - 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; + 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; 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