libexpr: move eval memory allocation to own struct

Co-authored-by: eldritch horrors <[email protected]>

https://git.lix.systems/lix-project/lix/commit/f5754dc90ae9b1207656d0e29ad2704d3ef1e554

Author: Radvendii

src/libcmd/common-eval-args.cc

@@ -155,7 +155,7 @@ Bindings * MixEvalArgs::getAutoArgs(EvalState & state)
 {
     auto res = state.buildBindings(autoArgs.size());
     for (auto & [name, arg] : autoArgs) {
-        auto v = state.allocValue();
+        auto v = state.mem.allocValue();
         std::visit(
             overloaded{
                 [&](const AutoArgExpr & arg) {

src/libcmd/installables.cc

@@ -454,7 +454,7 @@ ref<eval_cache::EvalCache> openEvalCache(EvalState & state, std::shared_ptr<flak
         if (getEnv("NIX_ALLOW_EVAL").value_or("1") == "0")
             throw Error("not everything is cached, but evaluation is not allowed");
 
-        auto vFlake = state.allocValue();
+        auto vFlake = state.mem.allocValue();
         flake::callFlake(state, *lockedFlake, *vFlake);
 
         state.forceAttrs(*vFlake, noPos, "while parsing cached flake data");
@@ -495,7 +495,7 @@ Installables SourceExprCommand::parseInstallables(ref<Store> store, std::vector<
         }
 
         auto state = getEvalState();
-        auto vFile = state->allocValue();
+        auto vFile = state->mem.allocValue();
 
         if (file == "-") {
             auto e = state->parseStdin();

src/libcmd/repl.cc

@@ -697,7 +697,7 @@ ProcessLineResult NixRepl::processLine(std::string line)
         if (p != std::string::npos && p < line.size() && line[p + 1] != '='
             && isVarName(name = removeWhitespace(line.substr(0, p)))) {
             Expr * e = parseString(line.substr(p + 1));
-            Value & v(*state->allocValue());
+            Value & v(*state->mem.allocValue());
             v.mkThunk(env, e);
             addVarToScope(state->symbols.create(name), v);
         } else {
@@ -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

@@ -160,7 +160,7 @@ nix_value * nix_alloc_value(nix_c_context * context, EvalState * state)
     if (context)
         context->last_err_code = NIX_OK;
     try {
-        nix_value * res = as_nix_value_ptr(state->state.allocValue());
+        nix_value * res = as_nix_value_ptr(state->state.mem.allocValue());
         nix_gc_incref(nullptr, res);
         return res;
     }
@@ -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-tests/derived-path.cc

@@ -22,7 +22,7 @@ TEST_F(DerivedPathExpressionTest, force_init) {}
 
 RC_GTEST_FIXTURE_PROP(DerivedPathExpressionTest, prop_opaque_path_round_trip, (const SingleDerivedPath::Opaque & o))
 {
-    auto * v = state.allocValue();
+    auto * v = state.mem.allocValue();
     state.mkStorePathString(o.path, *v);
     auto d = state.coerceToSingleDerivedPath(noPos, *v, "");
     RC_ASSERT(SingleDerivedPath{o} == d);
@@ -41,7 +41,7 @@ RC_GTEST_FIXTURE_PROP(
     ExperimentalFeatureSettings mockXpSettings;
     mockXpSettings.set("experimental-features", "ca-derivations dynamic-derivations");
 
-    auto * v = state.allocValue();
+    auto * v = state.mem.allocValue();
     state.mkOutputString(*v, b, std::nullopt, mockXpSettings);
     auto [d, _] = state.coerceToSingleDerivedPathUnchecked(noPos, *v, "", mockXpSettings);
     RC_ASSERT(SingleDerivedPath{b} == d);
@@ -55,7 +55,7 @@ RC_GTEST_FIXTURE_PROP(
     ExperimentalFeatureSettings mockXpSettings;
     mockXpSettings.set("experimental-features", "dynamic-derivations");
 
-    auto * v = state.allocValue();
+    auto * v = state.mem.allocValue();
     state.mkOutputString(*v, b, outPath, mockXpSettings);
     auto [d, _] = state.coerceToSingleDerivedPathUnchecked(noPos, *v, "", mockXpSettings);
     RC_ASSERT(SingleDerivedPath{b} == d);

src/libexpr/attr-path.cc

@@ -52,7 +52,7 @@ findAlongAttrPath(EvalState & state, const std::string & attrPath, Bindings & au
         auto attrIndex = string2Int<unsigned int>(attr);
 
         /* Evaluate the expression. */
-        Value * vNew = state.allocValue();
+        Value * vNew = state.mem.allocValue();
         state.autoCallFunction(autoArgs, *v, *vNew);
         v = vNew;
         state.forceValue(*v, noPos);

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++;
-    nrAttrsInAttrsets += capacity;
+    stats.nrAttrsets++;
+    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,
@@ -273,23 +282,14 @@ EvalState::EvalState(
     , importResolutionCache(make_ref<decltype(importResolutionCache)::element_type>())
     , fileEvalCache(make_ref<decltype(fileEvalCache)::element_type>())
     , regexCache(makeRegexCache())
-#if NIX_USE_BOEHMGC
-    , valueAllocCache(std::allocate_shared<void *>(traceable_allocator<void *>(), nullptr))
-    , 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))
-#endif
+    , baseEnv(mem.allocEnv(BASE_ENV_SIZE))
     , staticBaseEnv{std::make_shared<StaticEnv>(nullptr, nullptr)}
 {
     corepkgsFS->setPathDisplay("<nix", ">");
     internalFS->setPathDisplay("«nix-internal»", "");
 
     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");
 
@@ -423,7 +423,7 @@ void EvalState::checkURI(const std::string & uri)
 
 Value * EvalState::addConstant(const std::string & name, Value & v, Constant info)
 {
-    Value * v2 = allocValue();
+    Value * v2 = mem.allocValue();
     *v2 = v;
     addConstant(name, v2, info);
     return v2;
@@ -489,7 +489,7 @@ Value * EvalState::addPrimOp(PrimOp && primOp)
        the primop to a dummy value. */
     if (primOp.arity == 0) {
         primOp.arity = 1;
-        auto vPrimOp = allocValue();
+        auto vPrimOp = mem.allocValue();
         vPrimOp->mkPrimOp(new PrimOp(primOp));
         Value v;
         v.mkApp(vPrimOp, vPrimOp);
@@ -506,7 +506,7 @@ Value * EvalState::addPrimOp(PrimOp && primOp)
     if (hasPrefix(primOp.name, "__"))
         primOp.name = primOp.name.substr(2);
 
-    Value * v = allocValue();
+    Value * v = mem.allocValue();
     v->mkPrimOp(new PrimOp(primOp));
 
     if (primOp.internal)
@@ -885,11 +885,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)
@@ -990,7 +989,7 @@ void EvalState::mkSingleDerivedPathString(const SingleDerivedPath & p, Value & v
 
 Value * Expr::maybeThunk(EvalState & state, Env & env)
 {
-    Value * v = state.allocValue();
+    Value * v = state.mem.allocValue();
     mkThunk(*v, env, this);
     return v;
 }
@@ -1100,7 +1099,7 @@ void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial)
         *resolvedPath,
         nullptr,
         [&](auto & i) {
-            vExpr = allocValue();
+            vExpr = mem.allocValue();
             vExpr->mkThunk(&baseEnv, expr);
             i.second = vExpr;
         },
@@ -1183,7 +1182,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 +1201,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;
@@ -1217,7 +1216,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
         for (auto & i : attrs) {
             Value * vAttr;
             if (hasOverrides && i.second.kind != AttrDef::Kind::Inherited) {
-                vAttr = state.allocValue();
+                vAttr = state.mem.allocValue();
                 mkThunk(*vAttr, *i.second.chooseByKind(&env2, &env, inheritEnv), i.second.e);
             } else
                 vAttr = i.second.e->maybeThunk(state, *i.second.chooseByKind(&env2, &env, inheritEnv));
@@ -1294,7 +1293,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;
@@ -1485,7 +1484,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
     auto makeAppChain = [&]() {
         vRes = vCur;
         for (auto arg : args) {
-            auto fun2 = allocValue();
+            auto fun2 = mem.allocValue();
             *fun2 = vRes;
             vRes.mkPrimOpApp(fun2, arg);
         }
@@ -1500,7 +1499,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;
@@ -1683,7 +1682,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
             /* 'vCur' may be allocated on the stack of the calling
                function, but for functors we may keep a reference, so
                heap-allocate a copy and use that instead. */
-            Value * args2[] = {allocValue(), args[0]};
+            Value * args2[] = {mem.allocValue(), args[0]};
             *args2[0] = vCur;
             try {
                 callFunction(*functor->value, args2, vCur, functor->pos);
@@ -1743,7 +1742,7 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res
     if (fun.type() == nAttrs) {
         auto found = fun.attrs()->get(s.functor);
         if (found) {
-            Value * v = allocValue();
+            Value * v = mem.allocValue();
             callFunction(*found->value, fun, *v, pos);
             forceValue(*v, pos);
             return autoCallFunction(args, *v, res);
@@ -1784,12 +1783,12 @@ values, or passed explicitly with '--arg' or '--argstr'. See
         }
     }
 
-    callFunction(fun, allocValue()->mkAttrs(attrs), res, pos);
+    callFunction(fun, mem.allocValue()->mkAttrs(attrs), res, pos);
 }
 
 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 +2915,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 *);
-    uint64_t bLists = nrListElems * sizeof(Value *);
-    uint64_t bValues = nrValues * sizeof(Value);
-    uint64_t bAttrsets = nrAttrsets * sizeof(Bindings) + nrAttrsInAttrsets * sizeof(Attr);
+    auto memstats = mem.getStats();
+
+    uint64_t bEnvs = memstats.nrEnvs * sizeof(Env) + memstats.nrValuesInEnvs * sizeof(Value *);
+    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,28 +2946,28 @@ void EvalState::printStatistics()
 #endif
     };
     topObj["envs"] = {
-        {"number", nrEnvs.load()},
-        {"elements", nrValuesInEnvs.load()},
+        {"number", memstats.nrEnvs.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"] = {
         {"number", symbols.size()},
         {"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)},