Merge pull request NixOS#14544 from Radvendii/evalmemory-alocbytes

Ericson2314 · web-flow · commit af0ac14021a1 · 2025-11-11T17:29:55.000Z
move allocBytes() into EvalMemory
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
@@ -852,25 +852,26 @@ void Value::mkString(std::string_view s)
     mkStringNoCopy(StringData::make(s));
 }
 
-Value::StringWithContext::Context * Value::StringWithContext::Context::fromBuilder(const NixStringContext & context)
+Value::StringWithContext::Context *
+Value::StringWithContext::Context::fromBuilder(const NixStringContext & context, EvalMemory & mem)
 {
     if (context.empty())
         return nullptr;
 
-    auto ctx = new (allocBytes(sizeof(Context) + context.size() * sizeof(value_type))) Context(context.size());
+    auto ctx = new (mem.allocBytes(sizeof(Context) + context.size() * sizeof(value_type))) Context(context.size());
     std::ranges::transform(
         context, ctx->elems, [](const NixStringContextElem & elt) { return &StringData::make(elt.to_string()); });
     return ctx;
 }
 
-void Value::mkString(std::string_view s, const NixStringContext & context)
+void Value::mkString(std::string_view s, const NixStringContext & context, EvalMemory & mem)
 {
-    mkStringNoCopy(StringData::make(s), Value::StringWithContext::Context::fromBuilder(context));
+    mkStringNoCopy(StringData::make(s), Value::StringWithContext::Context::fromBuilder(context, mem));
 }
 
-void Value::mkStringMove(const StringData & s, const NixStringContext & context)
+void Value::mkStringMove(const StringData & s, const NixStringContext & context, EvalMemory & mem)
 {
-    mkStringNoCopy(s, Value::StringWithContext::Context::fromBuilder(context));
+    mkStringNoCopy(s, Value::StringWithContext::Context::fromBuilder(context, mem));
 }
 
 void Value::mkPath(const SourcePath & path)
@@ -911,9 +912,9 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval)
     }
 }
 
-ListBuilder::ListBuilder(size_t size)
+ListBuilder::ListBuilder(EvalMemory & mem, size_t size)
     : size(size)
-    , elems(size <= 2 ? inlineElems : (Value **) allocBytes(size * sizeof(Value *)))
+    , elems(size <= 2 ? inlineElems : (Value **) mem.allocBytes(size * sizeof(Value *)))
 {
 }
 
@@ -953,7 +954,8 @@ void EvalState::mkStorePathString(const StorePath & p, Value & v)
         store->printStorePath(p),
         NixStringContext{
             NixStringContextElem::Opaque{.path = p},
-        });
+        },
+        mem);
 }
 
 std::string EvalState::mkOutputStringRaw(
@@ -975,7 +977,7 @@ void EvalState::mkOutputString(
     std::optional<StorePath> optStaticOutputPath,
     const ExperimentalFeatureSettings & xpSettings)
 {
-    value.mkString(mkOutputStringRaw(b, optStaticOutputPath, xpSettings), NixStringContext{b});
+    value.mkString(mkOutputStringRaw(b, optStaticOutputPath, xpSettings), NixStringContext{b}, mem);
 }
 
 std::string EvalState::mkSingleDerivedPathStringRaw(const SingleDerivedPath & p)
@@ -1010,7 +1012,8 @@ void EvalState::mkSingleDerivedPathString(const SingleDerivedPath & p, Value & v
         mkSingleDerivedPathStringRaw(p),
         NixStringContext{
             std::visit([](auto && v) -> NixStringContextElem { return v; }, p),
-        });
+        },
+        mem);
 }
 
 Value * Expr::maybeThunk(EvalState & state, Env & env)
@@ -2143,7 +2146,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
             tmp += part->size();
         }
         *tmp = '\0';
-        v.mkStringMove(resultStr, context);
+        v.mkStringMove(resultStr, context, state.mem);
     }
 }
 
diff --git a/src/libexpr/include/nix/expr/eval-inline.hh b/src/libexpr/include/nix/expr/eval-inline.hh
@@ -12,7 +12,7 @@ namespace nix {
  * Note: Various places expect the allocated memory to be zeroed.
  */
 [[gnu::always_inline]]
-inline void * allocBytes(size_t n)
+inline void * EvalMemory::allocBytes(size_t n)
 {
     void * p;
 #if NIX_USE_BOEHMGC
diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh
@@ -335,6 +335,7 @@ public:
     EvalMemory & operator=(const EvalMemory &) = delete;
     EvalMemory & operator=(EvalMemory &&) = delete;
 
+    inline void * allocBytes(size_t n);
     inline Value * allocValue();
     inline Env & allocEnv(size_t size);
 
@@ -348,7 +349,7 @@ public:
     ListBuilder buildList(size_t size)
     {
         stats.nrListElems += size;
-        return ListBuilder(size);
+        return ListBuilder(*this, size);
     }
 
     const Statistics & getStats() const &
diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh
@@ -88,6 +88,7 @@ class PosIdx;
 struct Pos;
 class StorePath;
 class EvalState;
+class EvalMemory;
 class XMLWriter;
 class Printer;
 
@@ -161,7 +162,7 @@ class ListBuilder
     Value * inlineElems[2] = {nullptr, nullptr};
 public:
     Value ** elems;
-    ListBuilder(size_t size);
+    ListBuilder(EvalMemory & mem, size_t size);
 
     // NOTE: Can be noexcept because we are just copying integral values and
     // raw pointers.
@@ -364,7 +365,7 @@ struct ValueBase
             /**
              * @return null pointer when context.empty()
              */
-            static Context * fromBuilder(const NixStringContext & context);
+            static Context * fromBuilder(const NixStringContext & context, EvalMemory & mem);
         };
 
         /**
@@ -1148,9 +1149,9 @@ public:
 
     void mkString(std::string_view s);
 
-    void mkString(std::string_view s, const NixStringContext & context);
+    void mkString(std::string_view s, const NixStringContext & context, EvalMemory & mem);
 
-    void mkStringMove(const StringData & s, const NixStringContext & context);
+    void mkStringMove(const StringData & s, const NixStringContext & context, EvalMemory & mem);
 
     void mkPath(const SourcePath & path);
 
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
@@ -222,7 +222,8 @@ void derivationToValue(
             path2,
             {
                 NixStringContextElem::DrvDeep{.drvPath = storePath},
-            });
+            },
+            state.mem);
     attrs.alloc(state.s.name).mkString(drv.env["name"]);
 
     auto list = state.buildList(drv.outputs.size());
@@ -1811,7 +1812,8 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
             drvPathS,
             {
                 NixStringContextElem::DrvDeep{.drvPath = drvPath},
-            });
+            },
+            state.mem);
     for (auto & i : drv.outputs)
         mkOutputString(state, result, drvPath, i);
 
@@ -1864,7 +1866,7 @@ static void prim_toPath(EvalState & state, const PosIdx pos, Value ** args, Valu
     NixStringContext context;
     auto path =
         state.coerceToPath(pos, *args[0], context, "while evaluating the first argument passed to builtins.toPath");
-    v.mkString(path.path.abs(), context);
+    v.mkString(path.path.abs(), context, state.mem);
 }
 
 static RegisterPrimOp primop_toPath({
@@ -1907,7 +1909,7 @@ static void prim_storePath(EvalState & state, const PosIdx pos, Value ** args, V
     if (!settings.readOnlyMode)
         state.store->ensurePath(path2);
     context.insert(NixStringContextElem::Opaque{.path = path2});
-    v.mkString(path.abs(), context);
+    v.mkString(path.abs(), context, state.mem);
 }
 
 static RegisterPrimOp primop_storePath({
@@ -1989,7 +1991,8 @@ static void prim_baseNameOf(EvalState & state, const PosIdx pos, Value ** args,
     v.mkString(
         legacyBaseNameOf(*state.coerceToString(
             pos, *args[0], context, "while evaluating the first argument passed to builtins.baseNameOf", false, false)),
-        context);
+        context,
+        state.mem);
 }
 
 static RegisterPrimOp primop_baseNameOf({
@@ -2025,11 +2028,11 @@ static void prim_dirOf(EvalState & state, const PosIdx pos, Value ** args, Value
             pos, *args[0], context, "while evaluating the first argument passed to 'builtins.dirOf'", false, false);
         auto pos = path->rfind('/');
         if (pos == path->npos)
-            v.mkStringMove("."_sds, context);
+            v.mkStringMove("."_sds, context, state.mem);
         else if (pos == 0)
-            v.mkStringMove("/"_sds, context);
+            v.mkStringMove("/"_sds, context, state.mem);
         else
-            v.mkString(path->substr(0, pos), context);
+            v.mkString(path->substr(0, pos), context, state.mem);
     }
 }
 
@@ -2071,7 +2074,7 @@ static void prim_readFile(EvalState & state, const PosIdx pos, Value ** args, Va
                 .path = std::move((StorePath &&) p),
             });
     }
-    v.mkString(s, context);
+    v.mkString(s, context, state.mem);
 }
 
 static RegisterPrimOp primop_readFile({
@@ -2467,7 +2470,7 @@ static void prim_toXML(EvalState & state, const PosIdx pos, Value ** args, Value
     std::ostringstream out;
     NixStringContext context;
     printValueAsXML(state, true, false, *args[0], out, context, pos);
-    v.mkString(out.view(), context);
+    v.mkString(out.view(), context, state.mem);
 }
 
 static RegisterPrimOp primop_toXML({
@@ -2575,7 +2578,7 @@ static void prim_toJSON(EvalState & state, const PosIdx pos, Value ** args, Valu
     std::ostringstream out;
     NixStringContext context;
     printValueAsJSON(state, true, *args[0], pos, out, context);
-    v.mkString(out.view(), context);
+    v.mkString(out.view(), context, state.mem);
 }
 
 static RegisterPrimOp primop_toJSON({
@@ -4404,7 +4407,7 @@ static void prim_toString(EvalState & state, const PosIdx pos, Value ** args, Va
     NixStringContext context;
     auto s = state.coerceToString(
         pos, *args[0], context, "while evaluating the first argument passed to builtins.toString", true, false);
-    v.mkString(*s, context);
+    v.mkString(*s, context, state.mem);
 }
 
 static RegisterPrimOp primop_toString({
@@ -4477,7 +4480,7 @@ static void prim_substring(EvalState & state, const PosIdx pos, Value ** args, V
     auto s = state.coerceToString(
         pos, *args[2], context, "while evaluating the third argument (the string) passed to builtins.substring");
 
-    v.mkString(NixUInt(start) >= s->size() ? "" : s->substr(start, _len), context);
+    v.mkString(NixUInt(start) >= s->size() ? "" : s->substr(start, _len), context, state.mem);
 }
 
 static RegisterPrimOp primop_substring({
@@ -4875,7 +4878,7 @@ static void prim_concatStringsSep(EvalState & state, const PosIdx pos, Value **
             "while evaluating one element of the list of strings to concat passed to builtins.concatStringsSep");
     }
 
-    v.mkString(res, context);
+    v.mkString(res, context, state.mem);
 }
 
 static RegisterPrimOp primop_concatStringsSep({
@@ -4950,7 +4953,7 @@ static void prim_replaceStrings(EvalState & state, const PosIdx pos, Value ** ar
         }
     }
 
-    v.mkString(res, context);
+    v.mkString(res, context, state.mem);
 }
 
 static RegisterPrimOp primop_replaceStrings({
diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc
@@ -69,7 +69,7 @@ static void prim_unsafeDiscardOutputDependency(EvalState & state, const PosIdx p
         }
     }
 
-    v.mkString(*s, context2);
+    v.mkString(*s, context2, state.mem);
 }
 
 static RegisterPrimOp primop_unsafeDiscardOutputDependency(
@@ -137,7 +137,7 @@ static void prim_addDrvOutputDependencies(EvalState & state, const PosIdx pos, V
             context.begin()->raw)}),
     };
 
-    v.mkString(*s, context2);
+    v.mkString(*s, context2, state.mem);
 }
 
 static RegisterPrimOp primop_addDrvOutputDependencies(
@@ -321,7 +321,7 @@ static void prim_appendContext(EvalState & state, const PosIdx pos, Value ** arg
         }
     }
 
-    v.mkString(orig, context);
+    v.mkString(orig, context, state.mem);
 }
 
 static RegisterPrimOp primop_appendContext({.name = "__appendContext", .arity = 2, .fun = prim_appendContext});

Original file line number	Diff line number	Diff line change
`@@ -852,25 +852,26 @@ void Value::mkString(std::string_view s)`
`852`	`852`	`mkStringNoCopy(StringData::make(s));`
`853`	`853`	`}`
`854`	`854`
`855`		`-Value::StringWithContext::Context * Value::StringWithContext::Context::fromBuilder(const NixStringContext & context)`
	`855`	`+Value::StringWithContext::Context *`
	`856`	`+Value::StringWithContext::Context::fromBuilder(const NixStringContext & context, EvalMemory & mem)`
`856`	`857`	`{`
`857`	`858`	`if (context.empty())`
`858`	`859`	`return nullptr;`
`859`	`860`
`860`		`- auto ctx = new (allocBytes(sizeof(Context) + context.size() * sizeof(value_type))) Context(context.size());`
	`861`	`+ auto ctx = new (mem.allocBytes(sizeof(Context) + context.size() * sizeof(value_type))) Context(context.size());`
`861`	`862`	`std::ranges::transform(`
`862`	`863`	`context, ctx->elems, [](const NixStringContextElem & elt) { return &StringData::make(elt.to_string()); });`
`863`	`864`	`return ctx;`
`864`	`865`	`}`
`865`	`866`
`866`		`-void Value::mkString(std::string_view s, const NixStringContext & context)`
	`867`	`+void Value::mkString(std::string_view s, const NixStringContext & context, EvalMemory & mem)`
`867`	`868`	`{`
`868`		`- mkStringNoCopy(StringData::make(s), Value::StringWithContext::Context::fromBuilder(context));`
	`869`	`+ mkStringNoCopy(StringData::make(s), Value::StringWithContext::Context::fromBuilder(context, mem));`
`869`	`870`	`}`
`870`	`871`
`871`		`-void Value::mkStringMove(const StringData & s, const NixStringContext & context)`
	`872`	`+void Value::mkStringMove(const StringData & s, const NixStringContext & context, EvalMemory & mem)`
`872`	`873`	`{`
`873`		`- mkStringNoCopy(s, Value::StringWithContext::Context::fromBuilder(context));`
	`874`	`+ mkStringNoCopy(s, Value::StringWithContext::Context::fromBuilder(context, mem));`
`874`	`875`	`}`
`875`	`876`
`876`	`877`	`void Value::mkPath(const SourcePath & path)`
`@@ -911,9 +912,9 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval)`
`911`	`912`	`}`
`912`	`913`	`}`
`913`	`914`
`914`		`-ListBuilder::ListBuilder(size_t size)`
	`915`	`+ListBuilder::ListBuilder(EvalMemory & mem, size_t size)`
`915`	`916`	`: size(size)`
`916`		`- , elems(size <= 2 ? inlineElems : (Value *) allocBytes(size sizeof(Value *)))`
	`917`	`+ , elems(size <= 2 ? inlineElems : (Value *) mem.allocBytes(size sizeof(Value *)))`
`917`	`918`	`{`
`918`	`919`	`}`
`919`	`920`
`@@ -953,7 +954,8 @@ void EvalState::mkStorePathString(const StorePath & p, Value & v)`
`953`	`954`	`store->printStorePath(p),`
`954`	`955`	`NixStringContext{`
`955`	`956`	`NixStringContextElem::Opaque{.path = p},`
`956`		`- });`
	`957`	`+ },`
	`958`	`+ mem);`
`957`	`959`	`}`
`958`	`960`
`959`	`961`	`std::string EvalState::mkOutputStringRaw(`
`@@ -975,7 +977,7 @@ void EvalState::mkOutputString(`
`975`	`977`	`std::optional<StorePath> optStaticOutputPath,`
`976`	`978`	`const ExperimentalFeatureSettings & xpSettings)`
`977`	`979`	`{`
`978`		`- value.mkString(mkOutputStringRaw(b, optStaticOutputPath, xpSettings), NixStringContext{b});`
	`980`	`+ value.mkString(mkOutputStringRaw(b, optStaticOutputPath, xpSettings), NixStringContext{b}, mem);`
`979`	`981`	`}`
`980`	`982`
`981`	`983`	`std::string EvalState::mkSingleDerivedPathStringRaw(const SingleDerivedPath & p)`
`@@ -1010,7 +1012,8 @@ void EvalState::mkSingleDerivedPathString(const SingleDerivedPath & p, Value & v`
`1010`	`1012`	`mkSingleDerivedPathStringRaw(p),`
`1011`	`1013`	`NixStringContext{`
`1012`	`1014`	`std::visit([](auto && v) -> NixStringContextElem { return v; }, p),`
`1013`		`- });`
	`1015`	`+ },`
	`1016`	`+ mem);`
`1014`	`1017`	`}`
`1015`	`1018`
`1016`	`1019`	`Value * Expr::maybeThunk(EvalState & state, Env & env)`
`@@ -2143,7 +2146,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)`
`2143`	`2146`	`tmp += part->size();`
`2144`	`2147`	`}`
`2145`	`2148`	`*tmp = '\0';`
`2146`		`- v.mkStringMove(resultStr, context);`
	`2149`	`+ v.mkStringMove(resultStr, context, state.mem);`
`2147`	`2150`	`}`
`2148`	`2151`	`}`
`2149`	`2152`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ namespace nix {`
`12`	`12`	`* Note: Various places expect the allocated memory to be zeroed.`
`13`	`13`	`*/`
`14`	`14`	`[[gnu::always_inline]]`
`15`		`-inline void * allocBytes(size_t n)`
	`15`	`+inline void * EvalMemory::allocBytes(size_t n)`
`16`	`16`	`{`
`17`	`17`	`void * p;`
`18`	`18`	`#if NIX_USE_BOEHMGC`
Original file line number	Diff line number	Diff line change
`@@ -335,6 +335,7 @@ public:`
`335`	`335`	`EvalMemory & operator=(const EvalMemory &) = delete;`
`336`	`336`	`EvalMemory & operator=(EvalMemory &&) = delete;`
`337`	`337`
	`338`	`+ inline void * allocBytes(size_t n);`
`338`	`339`	`inline Value * allocValue();`
`339`	`340`	`inline Env & allocEnv(size_t size);`
`340`	`341`
`@@ -348,7 +349,7 @@ public:`
`348`	`349`	`ListBuilder buildList(size_t size)`
`349`	`350`	`{`
`350`	`351`	`stats.nrListElems += size;`
`351`		`- return ListBuilder(size);`
	`352`	`+ return ListBuilder(*this, size);`
`352`	`353`	`}`
`353`	`354`
`354`	`355`	`const Statistics & getStats() const &`
Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ static void prim_unsafeDiscardOutputDependency(EvalState & state, const PosIdx p`
`69`	`69`	`}`
`70`	`70`	`}`
`71`	`71`
`72`		`- v.mkString(*s, context2);`
	`72`	`+ v.mkString(*s, context2, state.mem);`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`static RegisterPrimOp primop_unsafeDiscardOutputDependency(`
`@@ -137,7 +137,7 @@ static void prim_addDrvOutputDependencies(EvalState & state, const PosIdx pos, V`
`137`	`137`	`context.begin()->raw)}),`
`138`	`138`	`};`
`139`	`139`
`140`		`- v.mkString(*s, context2);`
	`140`	`+ v.mkString(*s, context2, state.mem);`
`141`	`141`	`}`
`142`	`142`
`143`	`143`	`static RegisterPrimOp primop_addDrvOutputDependencies(`
`@@ -321,7 +321,7 @@ static void prim_appendContext(EvalState & state, const PosIdx pos, Value ** arg`
`321`	`321`	`}`
`322`	`322`	`}`
`323`	`323`
`324`		`- v.mkString(orig, context);`
	`324`	`+ v.mkString(orig, context, state.mem);`
`325`	`325`	`}`
`326`	`326`
`327`	`327`	`static RegisterPrimOp primop_appendContext({.name = "__appendContext", .arity = 2, .fun = prim_appendContext});`