libexpr: move eval memory allocation to own struct

Co-authored-by: eldritch horrors <pennae@lix.systems>
Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>

See original commit on lix:
f5754dc90a

src/libcmd/repl.cc

@@ -760,7 +760,7 @@ void NixRepl::loadFlake(const std::string & flakeRefS)
 
 void NixRepl::initEnv()
 {
-    env = &state->allocEnv(envSize);
+    env = &state->mem.allocEnv(envSize);
     env->up = &state->baseEnv;
     displ = 0;
     staticEnv->vars.clear();

src/libexpr-c/nix_api_value.cc

@@ -679,7 +679,7 @@ nix_err nix_bindings_builder_insert(nix_c_context * context, BindingsBuilder * b
         context->last_err_code = NIX_OK;
     try {
         auto & v = check_value_not_null(value);
-        nix::Symbol s = bb->builder.state.get().symbols.create(name);
+        nix::Symbol s = bb->builder.symbols.get().create(name);
         bb->builder.insert(s, &v);
     }
     NIXC_CATCH_ERRS

src/libexpr/attr-set.cc

@@ -10,27 +10,27 @@ Bindings Bindings::emptyBindings;
 /* Allocate a new array of attributes for an attribute set with a specific
    capacity. The space is implicitly reserved after the Bindings
    structure. */
-Bindings * EvalState::allocBindings(size_t capacity)
+Bindings * EvalMemory::allocBindings(size_t capacity)
 {
     if (capacity == 0)
         return &Bindings::emptyBindings;
     if (capacity > std::numeric_limits<Bindings::size_type>::max())
         throw Error("attribute set of size %d is too big", capacity);
-    nrAttrsets++;
+    stats.nrAttrsets++;
-    nrAttrsInAttrsets += capacity;
+    stats.nrAttrsInAttrsets += capacity;
     return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings();
 }
 
 Value & BindingsBuilder::alloc(Symbol name, PosIdx pos)
 {
-    auto value = state.get().allocValue();
+    auto value = mem.get().allocValue();
     bindings->push_back(Attr(name, value, pos));
     return *value;
 }
 
 Value & BindingsBuilder::alloc(std::string_view name, PosIdx pos)
 {
-    return alloc(state.get().symbols.create(name), pos);
+    return alloc(symbols.get().create(name), pos);
 }
 
 void Bindings::sort()

src/libexpr/eval.cc

@@ -194,6 +194,15 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
 
 static constexpr size_t BASE_ENV_SIZE = 128;
 
+EvalMemory::EvalMemory()
+#if NIX_USE_BOEHMGC
+    : valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
+    , env1AllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
+#endif
+{
+    assertGCInitialized();
+}
+
 EvalState::EvalState(
     const LookupPath & lookupPathFromArguments,
     ref<Store> store,
@@ -274,12 +283,10 @@ EvalState::EvalState(
     , fileEvalCache(make_ref<decltype(fileEvalCache)::element_type>())
     , regexCache(makeRegexCache())
 #if NIX_USE_BOEHMGC
-    , valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
+    , baseEnvP(std::allocate_shared<Env *>(traceable_allocator<Env *>(), &mem.allocEnv(BASE_ENV_SIZE)))
-    , env1AllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
-    , baseEnvP(std::allocate_shared<Env *>(traceable_allocator<Env *>(), &allocEnv(BASE_ENV_SIZE)))
     , baseEnv(**baseEnvP)
 #else
-    , baseEnv(allocEnv(BASE_ENV_SIZE))
+    , baseEnv(mem.allocEnv(BASE_ENV_SIZE))
 #endif
     , staticBaseEnv{std::make_shared<StaticEnv>(nullptr, nullptr)}
 {
@@ -288,8 +295,6 @@ EvalState::EvalState(
 
     countCalls = getEnv("NIX_COUNT_CALLS").value_or("0") != "0";
 
-    assertGCInitialized();
-
     static_assert(sizeof(Env) <= 16, "environment must be <= 16 bytes");
     static_assert(sizeof(Counter) == 64, "counters must be 64 bytes");
 
@@ -885,11 +890,10 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval)
     }
 }
 
-ListBuilder::ListBuilder(EvalState & state, size_t size)
+ListBuilder::ListBuilder(size_t size)
     : size(size)
     , elems(size <= 2 ? inlineElems : (Value **) allocBytes(size * sizeof(Value *)))
 {
-    state.nrListElems += size;
 }
 
 Value * EvalState::getBool(bool b)
@@ -1183,7 +1187,7 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v)
 
 Env * ExprAttrs::buildInheritFromEnv(EvalState & state, Env & up)
 {
-    Env & inheritEnv = state.allocEnv(inheritFromExprs->size());
+    Env & inheritEnv = state.mem.allocEnv(inheritFromExprs->size());
     inheritEnv.up = &up;
 
     Displacement displ = 0;
@@ -1202,7 +1206,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
     if (recursive) {
         /* Create a new environment that contains the attributes in
            this `rec'. */
-        Env & env2(state.allocEnv(attrs.size()));
+        Env & env2(state.mem.allocEnv(attrs.size()));
         env2.up = &env;
         dynamicEnv = &env2;
         Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr;
@@ -1294,7 +1298,7 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
 {
     /* Create a new environment that contains the attributes in this
        `let'. */
-    Env & env2(state.allocEnv(attrs->attrs.size()));
+    Env & env2(state.mem.allocEnv(attrs->attrs.size()));
     env2.up = &env;
 
     Env * inheritEnv = attrs->inheritFromExprs ? attrs->buildInheritFromEnv(state, env2) : nullptr;
@@ -1500,7 +1504,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
             ExprLambda & lambda(*vCur.lambda().fun);
 
             auto size = (!lambda.arg ? 0 : 1) + (lambda.hasFormals() ? lambda.formals->formals.size() : 0);
-            Env & env2(allocEnv(size));
+            Env & env2(mem.allocEnv(size));
             env2.up = vCur.lambda().env;
 
             Displacement displ = 0;
@@ -1789,7 +1793,7 @@ https://nix.dev/manual/nix/stable/language/syntax.html#functions.)",
 
 void ExprWith::eval(EvalState & state, Env & env, Value & v)
 {
-    Env & env2(state.allocEnv(1));
+    Env & env2(state.mem.allocEnv(1));
     env2.up = &env;
     env2.values[0] = attrs->maybeThunk(state, env);
 
@@ -2916,10 +2920,12 @@ void EvalState::printStatistics()
     std::chrono::microseconds cpuTimeDuration = getCpuUserTime();
     float cpuTime = std::chrono::duration_cast<std::chrono::duration<float>>(cpuTimeDuration).count();
 
-    uint64_t bEnvs = nrEnvs * sizeof(Env) + nrValuesInEnvs * sizeof(Value *);
+    auto & memstats = mem.getStats();
-    uint64_t bLists = nrListElems * sizeof(Value *);
+
-    uint64_t bValues = nrValues * sizeof(Value);
+    uint64_t bEnvs = memstats.nrEnvs * sizeof(Env) + memstats.nrValuesInEnvs * sizeof(Value *);
-    uint64_t bAttrsets = nrAttrsets * sizeof(Bindings) + nrAttrsInAttrsets * sizeof(Attr);
+    uint64_t bLists = memstats.nrListElems * sizeof(Value *);
+    uint64_t bValues = memstats.nrValues * sizeof(Value);
+    uint64_t bAttrsets = memstats.nrAttrsets * sizeof(Bindings) + memstats.nrAttrsInAttrsets * sizeof(Attr);
 
 #if NIX_USE_BOEHMGC
     GC_word heapSize, totalBytes;
@@ -2945,18 +2951,18 @@ void EvalState::printStatistics()
 #endif
     };
     topObj["envs"] = {
-        {"number", nrEnvs.load()},
+        {"number", memstats.nrEnvs.load()},
-        {"elements", nrValuesInEnvs.load()},
+        {"elements", memstats.nrValuesInEnvs.load()},
         {"bytes", bEnvs},
     };
     topObj["nrExprs"] = Expr::nrExprs.load();
     topObj["list"] = {
-        {"elements", nrListElems.load()},
+        {"elements", memstats.nrListElems.load()},
         {"bytes", bLists},
         {"concats", nrListConcats.load()},
     };
     topObj["values"] = {
-        {"number", nrValues.load()},
+        {"number", memstats.nrValues.load()},
         {"bytes", bValues},
     };
     topObj["symbols"] = {
@@ -2964,9 +2970,9 @@ void EvalState::printStatistics()
         {"bytes", symbols.totalSize()},
     };
     topObj["sets"] = {
-        {"number", nrAttrsets.load()},
+        {"number", memstats.nrAttrsets.load()},
         {"bytes", bAttrsets},
-        {"elements", nrAttrsInAttrsets.load()},
+        {"elements", memstats.nrAttrsInAttrsets.load()},
     };
     topObj["sizes"] = {
         {"Env", sizeof(Env)},

src/libexpr/include/nix/expr/attr-set.hh

@@ -13,7 +13,7 @@
 
 namespace nix {
 
-class EvalState;
+class EvalMemory;
 struct Value;
 
 /**
@@ -426,7 +426,7 @@ public:
         return res;
     }
 
-    friend class EvalState;
+    friend class EvalMemory;
 };
 
 static_assert(std::forward_iterator<Bindings::iterator>);
@@ -448,12 +448,13 @@ private:
     Bindings * bindings;
     Bindings::size_type capacity_;
 
-    friend class EvalState;
+    friend class EvalMemory;
 
-    BindingsBuilder(EvalState & state, Bindings * bindings, size_type capacity)
+    BindingsBuilder(EvalMemory & mem, SymbolTable & symbols, Bindings * bindings, size_type capacity)
         : bindings(bindings)
         , capacity_(capacity)
-        , state(state)
+        , mem(mem)
+        , symbols(symbols)
     {
     }
 
@@ -471,7 +472,8 @@ private:
     }
 
 public:
-    std::reference_wrapper<EvalState> state;
+    std::reference_wrapper<EvalMemory> mem;
+    std::reference_wrapper<SymbolTable> symbols;
 
     void insert(Symbol name, Value * value, PosIdx pos = noPos)
     {

src/libexpr/include/nix/expr/eval-inline.hh

@@ -26,7 +26,7 @@ inline void * allocBytes(size_t n)
 }
 
 [[gnu::always_inline]]
-Value * EvalState::allocValue()
+Value * EvalMemory::allocValue()
 {
 #if NIX_USE_BOEHMGC
     /* We use the boehm batch allocator to speed up allocations of Values (of which there are many).
@@ -48,15 +48,15 @@ Value * EvalState::allocValue()
     void * p = allocBytes(sizeof(Value));
 #endif
 
-    nrValues++;
+    stats.nrValues++;
     return (Value *) p;
 }
 
 [[gnu::always_inline]]
-Env & EvalState::allocEnv(size_t size)
+Env & EvalMemory::allocEnv(size_t size)
 {
-    nrEnvs++;
+    stats.nrEnvs++;
-    nrValuesInEnvs += size;
+    stats.nrValuesInEnvs += size;
 
     Env * env;
 

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

@@ -302,6 +302,63 @@ struct StaticEvalSymbols
     }
 };
 
+class EvalMemory
+{
+#if NIX_USE_BOEHMGC
+    /**
+     * Allocation cache for GC'd Value objects.
+     */
+    std::shared_ptr<void *> valueAllocCache;
+
+    /**
+     * Allocation cache for size-1 Env objects.
+     */
+    std::shared_ptr<void *> env1AllocCache;
+#endif
+
+public:
+    struct Statistics
+    {
+        Counter nrEnvs;
+        Counter nrValuesInEnvs;
+        Counter nrValues;
+        Counter nrAttrsets;
+        Counter nrAttrsInAttrsets;
+        Counter nrListElems;
+    };
+
+    EvalMemory();
+
+    EvalMemory(const EvalMemory &) = delete;
+    EvalMemory(EvalMemory &&) = delete;
+    EvalMemory & operator=(const EvalMemory &) = delete;
+    EvalMemory & operator=(EvalMemory &&) = delete;
+
+    inline Value * allocValue();
+    inline Env & allocEnv(size_t size);
+
+    Bindings * allocBindings(size_t capacity);
+
+    BindingsBuilder buildBindings(SymbolTable & symbols, size_t capacity)
+    {
+        return BindingsBuilder(*this, symbols, allocBindings(capacity), capacity);
+    }
+
+    ListBuilder buildList(size_t size)
+    {
+        stats.nrListElems += size;
+        return ListBuilder(size);
+    }
+
+    const Statistics & getStats() const &
+    {
+        return stats;
+    }
+
+private:
+    Statistics stats;
+};
+
 class EvalState : public std::enable_shared_from_this<EvalState>
 {
 public:
@@ -312,6 +369,8 @@ public:
     SymbolTable symbols;
     PosTable positions;
 
+    EvalMemory mem;
+
     /**
      * If set, force copying files to the Nix store even if they
      * already exist there.
@@ -441,18 +500,6 @@ private:
      */
     std::shared_ptr<RegexCache> regexCache;
 
-#if NIX_USE_BOEHMGC
-    /**
-     * Allocation cache for GC'd Value objects.
-     */
-    std::shared_ptr<void *> valueAllocCache;
-
-    /**
-     * Allocation cache for size-1 Env objects.
-     */
-    std::shared_ptr<void *> env1AllocCache;
-#endif
-
 public:
 
     EvalState(
@@ -463,6 +510,15 @@ public:
         std::shared_ptr<Store> buildStore = nullptr);
     ~EvalState();
 
+    /**
+     * A wrapper around EvalMemory::allocValue() to avoid code churn when it
+     * was introduced.
+     */
+    inline Value * allocValue()
+    {
+        return mem.allocValue();
+    }
+
     LookupPath getLookupPath()
     {
         return lookupPath;
@@ -834,22 +890,14 @@ public:
      */
     void autoCallFunction(const Bindings & args, Value & fun, Value & res);
 
-    /**
-     * Allocation primitives.
-     */
-    inline Value * allocValue();
-    inline Env & allocEnv(size_t size);
-
-    Bindings * allocBindings(size_t capacity);
-
     BindingsBuilder buildBindings(size_t capacity)
     {
-        return BindingsBuilder(*this, allocBindings(capacity), capacity);
+        return mem.buildBindings(symbols, capacity);
     }
 
     ListBuilder buildList(size_t size)
     {
-        return ListBuilder(*this, size);
+        return mem.buildList(size);
     }
 
     /**
@@ -966,13 +1014,7 @@ private:
      */
     std::string mkSingleDerivedPathStringRaw(const SingleDerivedPath & p);
 
-    Counter nrEnvs;
-    Counter nrValuesInEnvs;
-    Counter nrValues;
-    Counter nrListElems;
     Counter nrLookups;
-    Counter nrAttrsets;
-    Counter nrAttrsInAttrsets;
     Counter nrAvoided;
     Counter nrOpUpdates;
     Counter nrOpUpdateValuesCopied;

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

@@ -155,7 +155,7 @@ class ListBuilder
     Value * inlineElems[2] = {nullptr, nullptr};
 public:
     Value ** elems;
-    ListBuilder(EvalState & state, size_t size);
+    ListBuilder(size_t size);
 
     // NOTE: Can be noexcept because we are just copying integral values and
     // raw pointers.

src/libexpr/primops.cc

@@ -262,7 +262,7 @@ static void scopedImport(EvalState & state, const PosIdx pos, SourcePath & path,
 {
     state.forceAttrs(*vScope, pos, "while evaluating the first argument passed to builtins.scopedImport");
 
-    Env * env = &state.allocEnv(vScope->attrs()->size());
+    Env * env = &state.mem.allocEnv(vScope->attrs()->size());
     env->up = &state.baseEnv;
 
     auto staticEnv = std::make_shared<StaticEnv>(nullptr, state.staticBaseEnv, vScope->attrs()->size());
@@ -3161,7 +3161,7 @@ static void prim_listToAttrs(EvalState & state, const PosIdx pos, Value ** args,
     // Step 1. Sort the name-value attrsets in place using the memory we allocate for the result
     auto listView = args[0]->listView();
     size_t listSize = listView.size();
-    auto & bindings = *state.allocBindings(listSize);
+    auto & bindings = *state.mem.allocBindings(listSize);
     using ElemPtr = decltype(&bindings[0].value);
 
     for (const auto & [n, v2] : enumerate(listView)) {