From d23e3a09a44c428e298dfc7c202cebb8218f7b8e Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 28 Sep 2025 00:02:20 +0200 Subject: [PATCH] libexpr: alloc list storage as a (length, vla) type memory overhead is minimal and performance impact not measurable. once we've done something like this for all value types that don't fit in a single machine word we can cut a word from Value, offsetting the cost. Change-Id: I9813bacd7e851957ad3426aed8f74033179a4212 --- lix/legacy/nix-env.cc | 2 +- lix/legacy/user-env.cc | 15 ++-- lix/libcmd/repl.cc | 7 +- lix/libexpr/eval.cc | 33 ++++--- lix/libexpr/eval.hh | 10 ++- lix/libexpr/json-to-value.cc | 6 +- lix/libexpr/primops.cc | 140 +++++++++++++++++------------- lix/libexpr/primops/context.cc | 6 +- lix/libexpr/primops/fromTOML.cc | 6 +- lix/libexpr/value.cc | 3 +- lix/libexpr/value.hh | 50 +++++------ tests/unit/libexpr/value/print.cc | 80 +++++++++-------- 12 files changed, 201 insertions(+), 157 deletions(-) diff --git a/lix/legacy/nix-env.cc b/lix/legacy/nix-env.cc index 9f17f3457..36ae06776 100644 --- a/lix/legacy/nix-env.cc +++ b/lix/legacy/nix-env.cc @@ -181,7 +181,7 @@ static void loadSourceExpr(EvalState & state, const SourcePath & path_, Value & directory). */ else if (st.type == InputAccessor::tDirectory) { auto attrs = state.ctx.buildBindings(maxAttrs); - attrs.alloc("_combineChannels").mkList(0); + attrs.alloc("_combineChannels") = Value::EMPTY_LIST; StringSet seen; getAllExprs(state.ctx, path, seen, attrs); v.mkAttrs(attrs); diff --git a/lix/legacy/user-env.cc b/lix/legacy/user-env.cc index 94590789f..326e13578 100644 --- a/lix/legacy/user-env.cc +++ b/lix/legacy/user-env.cc @@ -1,4 +1,5 @@ #include "user-env.hh" +#include "lix/libexpr/value.hh" #include "lix/libstore/derivations.hh" #include "lix/libstore/store-api.hh" #include "lix/libstore/path-with-outputs.hh" @@ -32,7 +33,8 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, /* Construct the whole top level derivation. */ StorePathSet references; - Value manifest = state.ctx.mem.newList(elems.size()); + auto manifest = state.ctx.mem.newList(elems.size()); + Value vManifest{NewValueAs::list, manifest}; size_t n = 0; for (auto & i : elems) { /* Create a pseudo-derivation containing the name, system, @@ -55,9 +57,10 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, // Copy each output meant for installation. auto & vOutputs = attrs.alloc(state.ctx.s.outputs); - vOutputs = state.ctx.mem.newList(outputs.size()); + auto outputsList = state.ctx.mem.newList(outputs.size()); + vOutputs = {NewValueAs::list, outputsList}; for (const auto & [m, j] : enumerate(outputs)) { - (vOutputs.listElems()[m] = state.ctx.mem.allocValue())->mkString(j.first); + (outputsList->elems[m] = state.ctx.mem.allocValue())->mkString(j.first); auto outputAttrs = state.ctx.buildBindings(2); outputAttrs.alloc(state.ctx.s.outPath).mkString(state.ctx.store->printStorePath(*j.second)); attrs.alloc(j.first).mkAttrs(outputAttrs); @@ -80,7 +83,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, attrs.alloc(state.ctx.s.meta).mkAttrs(meta); - (manifest.listElems()[n++] = state.ctx.mem.allocValue())->mkAttrs(attrs); + (manifest->elems[n++] = state.ctx.mem.allocValue())->mkAttrs(attrs); if (drvPath) references.insert(*drvPath); } @@ -89,7 +92,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, the store; we need it for future modifications of the environment. */ std::ostringstream str; - printAmbiguous(manifest, state.ctx.symbols, str, nullptr, std::numeric_limits::max()); + printAmbiguous(vManifest, state.ctx.symbols, str, nullptr, std::numeric_limits::max()); auto manifestFile = state.aio.blockOn(state.ctx.store->addTextToStore("env-manifest.nix", str.str(), references)); @@ -103,7 +106,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, builder with the manifest as argument. */ auto attrs = state.ctx.buildBindings(3); state.ctx.paths.mkStorePathString(manifestFile, attrs.alloc("manifest")); - attrs.insert(state.ctx.symbols.create("derivations"), &manifest); + attrs.insert(state.ctx.symbols.create("derivations"), &vManifest); Value args; args.mkAttrs(attrs); diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index 242093e14..52644fd86 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -5,6 +5,7 @@ #include #include +#include "lix/libexpr/value.hh" #include "lix/libutil/box_ptr.hh" #include "lix/libcmd/repl-interacter.hh" #include "lix/libcmd/repl.hh" @@ -1000,8 +1001,8 @@ Value * NixRepl::getReplOverlaysEvalFunction() Value * NixRepl::replOverlays() { Value * replInits(evaluator.mem.allocValue()); - *replInits = evaluator.mem.newList(evalSettings.replOverlays.get().size()); - Value ** replInitElems = replInits->listElems(); + auto replInitStorage = evaluator.mem.newList(evalSettings.replOverlays.get().size()); + *replInits = {NewValueAs::list, replInitStorage}; size_t i = 0; for (auto path : evalSettings.replOverlays.get()) { @@ -1037,7 +1038,7 @@ Value * NixRepl::replOverlays() .debugThrow(); } - replInitElems[i] = replInit; + replInitStorage->elems[i] = replInit; i++; } diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 9aafba783..e198a36c1 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -854,13 +854,13 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval) } } -Value EvalMemory::newList(size_t size) +Value::List * EvalMemory::newList(size_t size) { - Value v; - v.mkList(size); - v._list.elems = allocType(size); + auto list = + reinterpret_cast(allocBytes(sizeof(Value::List) + size * sizeof(Value *))); + list->size = size; stats.nrListElems += size; - return v; + return list; } @@ -1210,9 +1210,11 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v) void ExprList::eval(EvalState & state, Env & env, Value & v) { - v = state.ctx.mem.newList(elems.size()); - for (auto [n, v2] : enumerate(v.listItems())) - const_cast(v2) = elems[n]->maybeThunk(state, env); + auto result = state.ctx.mem.newList(elems.size()); + v = {NewValueAs::list, result}; + for (auto [n, v2] : enumerate(result->span())) { + const_cast(v2) = elems[n]->maybeThunk(state, env); + } } @@ -1925,15 +1927,17 @@ void ExprOpConcatLists::eval(EvalState & state, Env & env, Value & v) else if (l2 == 0) v = v1; else { - v = state.ctx.mem.newList(len); - auto out = v.listElems(); + auto list = state.ctx.mem.newList(len); + v = {NewValueAs::list, list}; + auto out = list->elems; std::copy(v1.listElems(), v1.listElems() + l1, out); std::copy(v2.listElems(), v2.listElems() + l2, out + l1); } } - -void EvalState::concatLists(Value & v, size_t nrLists, Value * * lists, const PosIdx pos, std::string_view errorCtx) +void EvalState::concatLists( + Value & v, size_t nrLists, Value * const * lists, const PosIdx pos, std::string_view errorCtx +) { ctx.stats.nrListConcats++; @@ -1951,8 +1955,9 @@ void EvalState::concatLists(Value & v, size_t nrLists, Value * * lists, const Po return; } - v = ctx.mem.newList(len); - auto out = v.listElems(); + auto list = ctx.mem.newList(len); + v = {NewValueAs::list, list}; + auto out = list->elems; for (size_t n = 0, pos = 0; n < nrLists; ++n) { auto l = lists[n]->listSize(); if (l) diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh index 15c6e9a20..7b31086c1 100644 --- a/lix/libexpr/eval.hh +++ b/lix/libexpr/eval.hh @@ -259,7 +259,7 @@ public: inline Env & allocEnv(size_t size); Bindings * allocBindings(size_t capacity); - Value newList(size_t length); + Value::List * newList(size_t length); BindingsBuilder buildBindings(SymbolTable & symbols, size_t capacity) { @@ -888,7 +888,13 @@ public: const SingleDerivedPath & p, Value & v); - void concatLists(Value & v, size_t nrLists, Value * * lists, const PosIdx pos, std::string_view errorCtx); + void concatLists( + Value & v, + size_t nrLists, + Value * const * lists, + const PosIdx pos, + std::string_view errorCtx + ); private: diff --git a/lix/libexpr/json-to-value.cc b/lix/libexpr/json-to-value.cc index 58c91a0bd..a8745c721 100644 --- a/lix/libexpr/json-to-value.cc +++ b/lix/libexpr/json-to-value.cc @@ -60,10 +60,10 @@ class JSONSax : nlohmann::json_sax { ValueVector values; std::unique_ptr resolve(EvalState & state) override { - Value & v = parent->value(state); - v = state.ctx.mem.newList(values.size()); + auto list = state.ctx.mem.newList(values.size()); + parent->value(state) = {NewValueAs::list, list}; for (size_t n = 0; n < values.size(); ++n) { - v.listElems()[n] = values[n]; + list->elems[n] = values[n]; } return std::move(parent); } diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index 88e90f0c7..91e463dcf 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -18,6 +18,7 @@ #include "lix/libfetchers/fetch-to-store.hh" #include "lix/libutil/regex.hh" #include "lix/libutil/types.hh" +#include "value.hh" #include #include @@ -188,11 +189,12 @@ static void import(EvalState & state, Value & vPath, Value * vScope, Value & v) }); attrs.alloc(state.ctx.s.name).mkString(drv.env["name"]); auto & outputsVal = attrs.alloc(state.ctx.s.outputs); - outputsVal = state.ctx.mem.newList(drv.outputs.size()); + auto outputsList = state.ctx.mem.newList(drv.outputs.size()); + outputsVal = {NewValueAs::list, outputsList}; for (const auto & [i, o] : enumerate(drv.outputs)) { mkOutputString(state, attrs, *storePath, o); - (outputsVal.listElems()[i] = state.ctx.mem.allocValue())->mkString(o.first); + (outputsList->elems[i] = state.ctx.mem.allocValue())->mkString(o.first); } auto w = state.ctx.mem.allocValue(); @@ -568,10 +570,11 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v) } /* Create the result list. */ - v = state.ctx.mem.newList(res.size()); + auto result = state.ctx.mem.newList(res.size()); + v = {NewValueAs::list, result}; unsigned int n = 0; for (auto & i : res) - v.listElems()[n++] = i; + result->elems[n++] = i; } @@ -1644,13 +1647,14 @@ static void prim_attrNames(EvalState & state, Value * * args, Value & v) { state.forceAttrs(*args[0], noPos, "while evaluating the argument passed to builtins.attrNames"); - v = state.ctx.mem.newList(args[0]->attrs()->size()); + auto result = state.ctx.mem.newList(args[0]->attrs()->size()); + v = {NewValueAs::list, result}; size_t n = 0; for (auto & i : *args[0]->attrs()) - v.listElems()[n++] = const_cast(state.ctx.symbols[i.name].toValuePtr()); + result->elems[n++] = const_cast(state.ctx.symbols[i.name].toValuePtr()); - std::sort(v.listElems(), v.listElems() + n, [](Value * v1, Value * v2) { + std::sort(result->elems, result->elems + n, [](Value * v1, Value * v2) { return v1->str() < v2->str(); }); } @@ -1661,23 +1665,23 @@ static void prim_attrValues(EvalState & state, Value * * args, Value & v) { state.forceAttrs(*args[0], noPos, "while evaluating the argument passed to builtins.attrValues"); - v = state.ctx.mem.newList(args[0]->attrs()->size()); + auto result = state.ctx.mem.newList(args[0]->attrs()->size()); + v = {NewValueAs::list, result}; // FIXME: this is incredibly evil, *why* // NOLINTBEGIN(cppcoreguidelines-pro-type-cstyle-cast) unsigned int n = 0; for (auto & i : *args[0]->attrs()) - v.listElems()[n++] = (Value *) &i; + result->elems[n++] = (Value *) &i; - std::sort(v.listElems(), v.listElems() + n, - [&](Value * v1, Value * v2) { - std::string_view s1 = state.ctx.symbols[((Attr *) v1)->name], - s2 = state.ctx.symbols[((Attr *) v2)->name]; - return s1 < s2; - }); + std::sort(result->elems, result->elems + n, [&](Value * v1, Value * v2) { + std::string_view s1 = state.ctx.symbols[((Attr *) v1)->name], + s2 = state.ctx.symbols[((Attr *) v2)->name]; + return s1 < s2; + }); for (unsigned int i = 0; i < n; ++i) - v.listElems()[i] = ((Attr *) v.listElems()[i])->value; + result->elems[i] = ((Attr *) result->elems[i])->value; // NOLINTEND(cppcoreguidelines-pro-type-cstyle-cast) } @@ -1913,9 +1917,10 @@ static void prim_catAttrs(EvalState & state, Value * * args, Value & v) } } - v = state.ctx.mem.newList(found); + auto result = state.ctx.mem.newList(found); + v = {NewValueAs::list, result}; for (size_t n = 0; n < found; ++n) { - v.listElems()[n] = res[n]; + result->elems[n] = res[n]; } } @@ -1986,8 +1991,9 @@ static void prim_zipAttrsWith(EvalState & state, Value * * args, Value & v) for (auto & [sym, elem] : attrsSeen) { /* Take care of the returned lists. */ auto list = state.ctx.mem.allocValue(); - *list = state.ctx.mem.newList(elem.first); - elem.second = list->listElems(); + auto content = state.ctx.mem.newList(elem.first); + *list = {NewValueAs::list, content}; + elem.second = content->elems; /* Construct a `fn name list` function call value. */ auto name = const_cast(state.ctx.symbols[sym].toValuePtr()); @@ -2056,9 +2062,10 @@ static void prim_tail(EvalState & state, Value * * args, Value & v) if (args[0]->listSize() == 0) state.ctx.errors.make("'tail' called on an empty list").debugThrow(); - v = state.ctx.mem.newList(args[0]->listSize() - 1); + auto result = state.ctx.mem.newList(args[0]->listSize() - 1); + v = {NewValueAs::list, result}; for (unsigned int n = 0; n < v.listSize(); ++n) - v.listElems()[n] = args[0]->listElems()[n + 1]; + result->elems[n] = args[0]->listElems()[n + 1]; } /* Apply a function to every element of a list. */ @@ -2073,10 +2080,10 @@ static void prim_map(EvalState & state, Value * * args, Value & v) state.forceFunction(*args[0], noPos, "while evaluating the first argument passed to builtins.map"); - v = state.ctx.mem.newList(args[1]->listSize()); + auto result = state.ctx.mem.newList(args[1]->listSize()); + v = {NewValueAs::list, result}; for (unsigned int n = 0; n < v.listSize(); ++n) - (v.listElems()[n] = state.ctx.mem.allocValue())->mkApp( - args[0], args[1]->listElems()[n]); + (result->elems[n] = state.ctx.mem.allocValue())->mkApp(args[0], args[1]->listElems()[n]); } /* Filter a list using a predicate; that is, return a list containing @@ -2110,8 +2117,11 @@ static void prim_filter(EvalState & state, Value * * args, Value & v) if (same) v = *args[1]; else { - v = state.ctx.mem.newList(k); - for (unsigned int n = 0; n < k; ++n) v.listElems()[n] = vs[n]; + auto result = state.ctx.mem.newList(k); + v = {NewValueAs::list, result}; + for (unsigned int n = 0; n < k; ++n) { + result->elems[n] = vs[n]; + } } } @@ -2212,11 +2222,12 @@ static void prim_genList(EvalState & state, Value * * args, Value & v) // as evaluating map without accessing any values makes little sense. state.forceFunction(*args[0], noPos, "while evaluating the first argument passed to builtins.genList"); - v = state.ctx.mem.newList(len); + auto result = state.ctx.mem.newList(len); + v = {NewValueAs::list, result}; for (size_t n = 0; n < len; ++n) { auto arg = state.ctx.mem.allocValue(); arg->mkInt(n); - (v.listElems()[n] = state.ctx.mem.allocValue())->mkApp(args[0], arg); + (result->elems[n] = state.ctx.mem.allocValue())->mkApp(args[0], arg); } } @@ -2235,10 +2246,11 @@ static void prim_sort(EvalState & state, Value * * args, Value & v) state.forceFunction(*args[0], noPos, "while evaluating the first argument passed to builtins.sort"); - v = state.ctx.mem.newList(len); + auto list = state.ctx.mem.newList(len); + v = {NewValueAs::list, list}; for (unsigned int n = 0; n < len; ++n) { state.forceValue(*args[1]->listElems()[n], noPos); - v.listElems()[n] = args[1]->listElems()[n]; + list->elems[n] = args[1]->listElems()[n]; } auto comparator = [&](Value * a, Value * b) { @@ -2261,7 +2273,7 @@ static void prim_sort(EvalState & state, Value * * args, Value & v) /* FIXME: std::sort can segfault if the comparator is not a strict weak ordering. What to do? std::stable_sort() seems more resilient, but no guarantees... */ - std::stable_sort(v.listElems(), v.listElems() + len, comparator); + std::stable_sort(list->elems, list->elems + len, comparator); } static void prim_partition(EvalState & state, Value * * args, Value & v) @@ -2288,15 +2300,19 @@ static void prim_partition(EvalState & state, Value * * args, Value & v) auto & vRight = attrs.alloc(state.ctx.s.right); auto rsize = right.size(); - vRight = state.ctx.mem.newList(rsize); - if (rsize) - memcpy(vRight.listElems(), right.data(), sizeof(Value *) * rsize); + auto rlist = state.ctx.mem.newList(rsize); + vRight = {NewValueAs::list, rlist}; + if (rsize) { + memcpy(rlist->elems, right.data(), sizeof(Value *) * rsize); + } auto & vWrong = attrs.alloc(state.ctx.s.wrong); auto wsize = wrong.size(); - vWrong = state.ctx.mem.newList(wsize); - if (wsize) - memcpy(vWrong.listElems(), wrong.data(), sizeof(Value *) * wsize); + auto wlist = state.ctx.mem.newList(wsize); + vWrong = {NewValueAs::list, wlist}; + if (wsize) { + memcpy(wlist->elems, wrong.data(), sizeof(Value *) * wsize); + } v.mkAttrs(attrs); } @@ -2322,8 +2338,9 @@ static void prim_groupBy(EvalState & state, Value * * args, Value & v) for (auto & i : attrs) { auto & list = attrs2.alloc(i.first); auto size = i.second.size(); - list = state.ctx.mem.newList(size); - memcpy(list.listElems(), i.second.data(), sizeof(Value *) * size); + auto content = state.ctx.mem.newList(size); + list = {NewValueAs::list, content}; + memcpy(content->elems, i.second.data(), sizeof(Value *) * size); } v.mkAttrs(attrs2.alreadySorted()); @@ -2346,8 +2363,9 @@ static void prim_concatMap(EvalState & state, Value * * args, Value & v) len += lists[n].listSize(); } - v = state.ctx.mem.newList(len); - auto out = v.listElems(); + auto result = state.ctx.mem.newList(len); + v = {NewValueAs::list, result}; + auto out = result->elems; for (unsigned int n = 0, pos = 0; n < nrLists; ++n) { auto l = lists[n].listSize(); if (l) @@ -2610,12 +2628,13 @@ void prim_match(EvalState & state, Value * * args, Value & v) // the first match is the whole string const size_t len = match.size() - 1; - v = state.ctx.mem.newList(len); + auto result = state.ctx.mem.newList(len); + v = {NewValueAs::list, result}; for (size_t i = 0; i < len; ++i) { if (!match[i+1].matched) - (v.listElems()[i] = state.ctx.mem.allocValue())->mkNull(); + (result->elems[i] = state.ctx.mem.allocValue())->mkNull(); else - (v.listElems()[i] = state.ctx.mem.allocValue())->mkString(match[i + 1].str()); + (result->elems[i] = state.ctx.mem.allocValue())->mkString(match[i + 1].str()); } } catch (regex::Error & e) { @@ -2641,11 +2660,12 @@ void prim_split(EvalState & state, Value * * args, Value & v) // Any matches results are surrounded by non-matching results. const size_t len = std::distance(begin, end); - v = state.ctx.mem.newList(2 * len + 1); + auto result = state.ctx.mem.newList(2 * len + 1); + v = {NewValueAs::list, result}; size_t idx = 0; if (len == 0) { - v.listElems()[idx++] = args[1]; + result->elems[idx++] = args[1]; return; } @@ -2654,24 +2674,26 @@ void prim_split(EvalState & state, Value * * args, Value & v) auto match = *i; // Add a string for non-matched characters. - (v.listElems()[idx++] = state.ctx.mem.allocValue())->mkString(match.prefix().str()); + (result->elems[idx++] = state.ctx.mem.allocValue())->mkString(match.prefix().str()); // Add a list for matched substrings. const size_t slen = match.size() - 1; - auto elem = v.listElems()[idx++] = state.ctx.mem.allocValue(); + auto elem = result->elems[idx++] = state.ctx.mem.allocValue(); // Start at 1, beacause the first match is the whole string. - *elem = state.ctx.mem.newList(slen); + auto content = state.ctx.mem.newList(slen); + *elem = {NewValueAs::list, content}; for (size_t si = 0; si < slen; ++si) { if (!match[si + 1].matched) - (elem->listElems()[si] = state.ctx.mem.allocValue())->mkNull(); + (content->elems[si] = state.ctx.mem.allocValue())->mkNull(); else - (elem->listElems()[si] = state.ctx.mem.allocValue())->mkString(match[si + 1].str()); + (content->elems[si] = state.ctx.mem.allocValue()) + ->mkString(match[si + 1].str()); } // Add a string for non-matched suffix characters. if (idx == 2 * len) - (v.listElems()[idx++] = state.ctx.mem.allocValue())->mkString(match.suffix().str()); + (result->elems[idx++] = state.ctx.mem.allocValue())->mkString(match.suffix().str()); } assert(idx == 2 * len + 1); @@ -2793,9 +2815,10 @@ static void prim_splitVersion(EvalState & state, Value * * args, Value & v) break; components.emplace_back(component); } - v = state.ctx.mem.newList(components.size()); + auto result = state.ctx.mem.newList(components.size()); + v = {NewValueAs::list, result}; for (const auto & [n, component] : enumerate(components)) - (v.listElems()[n] = state.ctx.mem.allocValue())->mkString(std::move(component)); + (result->elems[n] = state.ctx.mem.allocValue())->mkString(std::move(component)); } @@ -2816,16 +2839,15 @@ RegisterPrimOp::RegisterPrimOp(PrimOp && primOp) Value EvalBuiltins::prepareNixPath(const SearchPath & searchPath) { - Value v; - v = mem.newList(searchPath.elements.size()); + auto v = mem.newList(searchPath.elements.size()); int n = 0; for (auto & i : searchPath.elements) { auto attrs = mem.buildBindings(symbols, 2); attrs.alloc("path").mkString(i.path.s); attrs.alloc("prefix").mkString(i.prefix.s); - (v.listElems()[n++] = mem.allocValue())->mkAttrs(attrs); + (v->elems[n++] = mem.allocValue())->mkAttrs(attrs); } - return v; + return {NewValueAs::list, v}; } void EvalBuiltins::createBaseEnv(const SearchPath & searchPath, const Path & storeDir) diff --git a/lix/libexpr/primops/context.cc b/lix/libexpr/primops/context.cc index 7a6ddb3cb..cd577cca9 100644 --- a/lix/libexpr/primops/context.cc +++ b/lix/libexpr/primops/context.cc @@ -3,6 +3,7 @@ #include "lix/libstore/derivations.hh" #include "lix/libstore/store-api.hh" #include "lix/libutil/types.hh" +#include "value.hh" namespace nix { @@ -147,9 +148,10 @@ void prim_getContext(EvalState & state, Value * * args, Value & v) infoAttrs.alloc(sAllOutputs).mkBool(true); if (!info.second.outputs.empty()) { auto & outputsVal = infoAttrs.alloc(state.ctx.s.outputs); - outputsVal = state.ctx.mem.newList(info.second.outputs.size()); + auto content = state.ctx.mem.newList(info.second.outputs.size()); + outputsVal = {NewValueAs::list, content}; for (const auto & [i, output] : enumerate(info.second.outputs)) - (outputsVal.listElems()[i] = state.ctx.mem.allocValue())->mkString(output); + (content->elems[i] = state.ctx.mem.allocValue())->mkString(output); } attrs.alloc(state.ctx.store->printStorePath(info.first)).mkAttrs(infoAttrs); } diff --git a/lix/libexpr/primops/fromTOML.cc b/lix/libexpr/primops/fromTOML.cc index 9d4b5e6ab..d9d84cdd2 100644 --- a/lix/libexpr/primops/fromTOML.cc +++ b/lix/libexpr/primops/fromTOML.cc @@ -1,5 +1,6 @@ #include "lix/libexpr/eval.hh" #include "lix/libexpr/extra-primops.hh" +#include "value.hh" #include #include @@ -30,9 +31,10 @@ void prim_fromTOML(EvalState & state, Value ** args, Value & val) auto array = toml::get>(t); size_t size = array.size(); - v = state.ctx.mem.newList(size); + auto list = state.ctx.mem.newList(size); + v = {NewValueAs::list, list}; for (size_t i = 0; i < size; ++i) { - self(*(v.listElems()[i] = state.ctx.mem.allocValue()), array[i]); + self(*(list->elems[i] = state.ctx.mem.allocValue()), array[i]); } } break; case toml::value_t::boolean: diff --git a/lix/libexpr/value.cc b/lix/libexpr/value.cc index 7e0902957..e23c32d9d 100644 --- a/lix/libexpr/value.cc +++ b/lix/libexpr/value.cc @@ -9,7 +9,8 @@ namespace nix { -Value Value::EMPTY_LIST{Value::list_t{}, {}}; +static const Value::List emptyListData{.size = 0}; +Value Value::EMPTY_LIST{Value::list_t{}, &emptyListData}; static void copyContextToValue(Value & v, const NixStringContext & context) { diff --git a/lix/libexpr/value.hh b/lix/libexpr/value.hh index e9cbce3b7..221d7280a 100644 --- a/lix/libexpr/value.hh +++ b/lix/libexpr/value.hh @@ -236,6 +236,8 @@ public: USING_VALUETYPE(blackhole_t); #undef USING_VALUETYPE + struct List; + /// Default constructor which is still used in the codebase but should not /// be used in new code. Zero initializes its members. [[deprecated]] Value() @@ -373,12 +375,7 @@ public: /// smaller, the list is stored inline, and the Value pointers in /// @ref items are shallow copied into this structure, without dynamically /// allocating memory. - Value(list_t, std::span items) - { - this->internalType = tList; - this->_list.size = items.size(); - this->_list.elems = items.data(); - } + Value(list_t, const List * items) : internalType(tList), _list(items), _list_pad(0) {} /// Constructs a nix language value of type "list", with an element array /// initialized by applying @ref transformer to each element in @ref items. @@ -394,12 +391,14 @@ public: Value(list_t, SizedIterableT & items, TransformerT const & transformer) { this->internalType = tList; - this->_list.size = items.size(); - this->_list.elems = gcAllocType(items.size()); + auto list = + reinterpret_cast(gcAllocBytes(sizeof(List) + items.size() * sizeof(Value *))); + list->size = items.size(); auto it = items.begin(); for (size_t i = 0; i < items.size(); i++, it++) { - this->_list.elems[i] = transformer(*it); + list->elems[i] = transformer(*it); } + _list = list; } /// Constructs a nix language value of the singleton type "null". @@ -519,6 +518,17 @@ public: inline bool isPrimOp() const { return internalType == tPrimOp; }; inline bool isPrimOpApp() const { return internalType == tPrimOpApp; }; + struct List + { + size_t size; + Value * elems[0]; + + std::span span() + { + return {elems, elems + size}; + } + }; + union { /// Dummy field, which takes up as much space as the largest union variants @@ -567,9 +577,9 @@ public: uintptr_t _attrs_pad; }; struct { - size_t size; - Value * * elems; - } _list; + const List * _list; + uintptr_t _list_pad; + }; struct { Env * env; Expr * expr; @@ -692,13 +702,6 @@ public: Value & mkAttrs(BindingsBuilder & bindings); - inline void mkList(size_t size) - { - clearValue(); - internalType = tList; - _list.size = size; - } - inline void mkThunk(Env * e, Expr & ex) { internalType = tThunk; @@ -759,19 +762,14 @@ public: return internalType == tList; } - Value * * listElems() - { - return _list.elems; - } - Value * const * listElems() const { - return _list.elems; + return _list->elems; } size_t listSize() const { - return _list.size; + return _list->size; } /** diff --git a/tests/unit/libexpr/value/print.cc b/tests/unit/libexpr/value/print.cc index c2296cbad..8e16deff8 100644 --- a/tests/unit/libexpr/value/print.cc +++ b/tests/unit/libexpr/value/print.cc @@ -82,12 +82,12 @@ TEST_F(ValuePrintingTests, tList) Value vTwo; vTwo.mkInt(2); - Value vList = evaluator.mem.newList(5); - vList._list.elems[0] = &vOne; - vList._list.elems[1] = &vTwo; - vList._list.size = 3; + auto vList = evaluator.mem.newList(5); + vList->elems[0] = &vOne; + vList->elems[1] = &vTwo; + vList->size = 3; - test(vList, "[ 1 2 «nullptr» ]"); + test(Value(NewValueAs::list, vList), "[ 1 2 «nullptr» ]"); } TEST_F(ValuePrintingTests, vThunk) @@ -260,12 +260,13 @@ TEST_F(ValuePrintingTests, depthList) Value vNested; vNested.mkAttrs(builder2.finish()); - Value vList = evaluator.mem.newList(5); - vList._list.elems[0] = &vOne; - vList._list.elems[1] = &vTwo; - vList._list.elems[2] = &vNested; - vList._list.size = 3; + auto list = evaluator.mem.newList(5); + list->elems[0] = &vOne; + list->elems[1] = &vTwo; + list->elems[2] = &vNested; + list->size = 3; + Value vList{NewValueAs::list, list}; test(vList, "[ 1 2 { ... } ]", PrintOptions { .maxDepth = 1 }); test(vList, "[ 1 2 { nested = { ... }; one = 1; two = 2; } ]", PrintOptions { .maxDepth = 2 }); test(vList, "[ 1 2 { nested = { one = 1; two = 2; }; one = 1; two = 2; } ]", PrintOptions { .maxDepth = 3 }); @@ -533,16 +534,17 @@ TEST_F(ValuePrintingTests, ansiColorsList) Value vTwo; vTwo.mkInt(2); - Value vList = evaluator.mem.newList(5); - vList._list.elems[0] = &vOne; - vList._list.elems[1] = &vTwo; - vList._list.size = 3; + auto vList = evaluator.mem.newList(5); + vList->elems[0] = &vOne; + vList->elems[1] = &vTwo; + vList->size = 3; - test(vList, - "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_CYAN "2" ANSI_NORMAL " " ANSI_MAGENTA "«nullptr»" ANSI_NORMAL " ]", - PrintOptions { - .ansiColors = true - }); + test( + Value(NewValueAs::list, vList), + "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_CYAN "2" ANSI_NORMAL " " ANSI_MAGENTA + "«nullptr»" ANSI_NORMAL " ]", + PrintOptions{.ansiColors = true} + ); } TEST_F(ValuePrintingTests, ansiColorsLambda) @@ -671,16 +673,16 @@ TEST_F(ValuePrintingTests, ansiColorsListRepeated) Value vInner; vInner.mkAttrs(innerBuilder.finish()); - Value vList = evaluator.mem.newList(3); - vList._list.elems[0] = &vInner; - vList._list.elems[1] = &vInner; - vList._list.size = 2; + auto vList = evaluator.mem.newList(3); + vList->elems[0] = &vInner; + vList->elems[1] = &vInner; + vList->size = 2; - test(vList, - "[ { x = " ANSI_CYAN "0" ANSI_NORMAL "; } " ANSI_MAGENTA "«repeated»" ANSI_NORMAL " ]", - PrintOptions { - .ansiColors = true - }); + test( + Value(NewValueAs::list, vList), + "[ { x = " ANSI_CYAN "0" ANSI_NORMAL "; } " ANSI_MAGENTA "«repeated»" ANSI_NORMAL " ]", + PrintOptions{.ansiColors = true} + ); } TEST_F(ValuePrintingTests, listRepeated) @@ -694,11 +696,12 @@ TEST_F(ValuePrintingTests, listRepeated) Value vInner; vInner.mkAttrs(innerBuilder.finish()); - Value vList = evaluator.mem.newList(3); - vList._list.elems[0] = &vInner; - vList._list.elems[1] = &vInner; - vList._list.size = 2; + auto list = evaluator.mem.newList(3); + list->elems[0] = &vInner; + list->elems[1] = &vInner; + list->size = 2; + Value vList(NewValueAs::list, list); test(vList, "[ { x = 0; } «repeated» ]", PrintOptions { }); test(vList, "[ { x = 0; } { x = 0; } ]", @@ -751,10 +754,11 @@ TEST_F(ValuePrintingTests, ansiColorsListElided) Value vTwo; vTwo.mkInt(2); - Value vList = evaluator.mem.newList(4); - vList._list.elems[0] = &vOne; - vList._list.elems[1] = &vTwo; - vList._list.size = 2; + auto list = evaluator.mem.newList(4); + Value vList{NewValueAs::list, list}; + list->elems[0] = &vOne; + list->elems[1] = &vTwo; + list->size = 2; test(vList, "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_FAINT "«1 item elided»" ANSI_NORMAL " ]", @@ -766,8 +770,8 @@ TEST_F(ValuePrintingTests, ansiColorsListElided) Value vThree; vThree.mkInt(3); - vList._list.elems[2] = &vThree; - vList._list.size = 3; + list->elems[2] = &vThree; + list->size = 3; test(vList, "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_FAINT "«2 items elided»" ANSI_NORMAL " ]",