Merge branch 'master' into fzakaria/shellcheck-functional-db-migration

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$''

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 (Counter::enabled) {
-
-    if (showStats) {
         // 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},
+        {"number", nrEnvs.load()},
-        {"elements", nrValuesInEnvs},
+        {"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["nrOpUpdates"] = nrOpUpdates.load();
-    topObj["nrOpUpdateValuesCopied"] = nrOpUpdateValuesCopied;
+    topObj["nrOpUpdateValuesCopied"] = nrOpUpdateValuesCopied.load();
-    topObj["nrThunks"] = nrThunks;
+    topObj["nrThunks"] = nrThunks.load();
-    topObj["nrAvoided"] = nrAvoided;
+    topObj["nrAvoided"] = nrAvoided.load();
-    topObj["nrLookups"] = nrLookups;
+    topObj["nrLookups"] = nrLookups.load();
-    topObj["nrPrimOpCalls"] = nrPrimOpCalls;
+    topObj["nrPrimOpCalls"] = nrPrimOpCalls.load();
-    topObj["nrFunctionCalls"] = nrFunctionCalls;
+    topObj["nrFunctionCalls"] = nrFunctionCalls.load();
 #if NIX_USE_BOEHMGC
     topObj["gc"] = {
         {"heapSize", heapSize},

src/libexpr/include/nix/expr/counter.hh

@@ -0,0 +1,70 @@
+#pragma once
+
+#include <atomic>
+#include <cstdint>
+
+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<value_type> 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

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;
+    Counter nrEnvs;
-    unsigned long nrValuesInEnvs = 0;
+    Counter nrValuesInEnvs;
-    unsigned long nrValues = 0;
+    Counter nrValues;
-    unsigned long nrListElems = 0;
+    Counter nrListElems;
-    unsigned long nrLookups = 0;
+    Counter nrLookups;
-    unsigned long nrAttrsets = 0;
+    Counter nrAttrsets;
-    unsigned long nrAttrsInAttrsets = 0;
+    Counter nrAttrsInAttrsets;
-    unsigned long nrAvoided = 0;
+    Counter nrAvoided;
-    unsigned long nrOpUpdates = 0;
+    Counter nrOpUpdates;
-    unsigned long nrOpUpdateValuesCopied = 0;
+    Counter nrOpUpdateValuesCopied;
-    unsigned long nrListConcats = 0;
+    Counter nrListConcats;
-    unsigned long nrPrimOpCalls = 0;
+    Counter nrPrimOpCalls;
-    unsigned long nrFunctionCalls = 0;
+    Counter nrFunctionCalls;
 
     bool countCalls;
 

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',

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()
     {

src/libexpr/nixexpr.cc

@@ -11,7 +11,7 @@
 
 namespace nix {
 
-unsigned long Expr::nrExprs = 0;
+Counter Expr::nrExprs;
 
 ExprBlackHole eBlackHole;
 

tests/functional/dependencies.builder0.sh

@@ -1,16 +1,20 @@
+# shellcheck shell=bash
+# shellcheck disable=SC2154
 [ "${input1: -2}" = /. ]
+# shellcheck disable=SC2154
 [ "${input2: -2}" = /. ]
 
-mkdir $out
+# shellcheck disable=SC2154
-echo $(cat $input1/foo)$(cat $input2/bar) > $out/foobar
+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
+echo program > "$out"/program
-chmod +x $out/program
+chmod +x "$out"/program
 
 echo FOO