From 2b4c94482364c236b03f7f8b61aca7076ab4079d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 4 Jan 2022 20:29:17 +0100 Subject: [PATCH] Remove EvalState::mkAttrs() --- src/libexpr/attr-set.cc | 13 +++-------- src/libexpr/attr-set.hh | 10 +++++++-- src/libexpr/eval.cc | 20 +++++++++-------- src/libexpr/eval.hh | 3 ++- src/libexpr/json-to-value.cc | 6 +++--- src/libexpr/primops.cc | 42 +++++++++++++++++++++--------------- 6 files changed, 52 insertions(+), 42 deletions(-) diff --git a/src/libexpr/attr-set.cc b/src/libexpr/attr-set.cc index fc971957d..52ac47e9b 100644 --- a/src/libexpr/attr-set.cc +++ b/src/libexpr/attr-set.cc @@ -7,6 +7,7 @@ namespace nix { + /* Allocate a new array of attributes for an attribute set with a specific capacity. The space is implicitly reserved after the Bindings structure. */ @@ -16,15 +17,9 @@ Bindings * EvalState::allocBindings(size_t capacity) return &emptyBindings; if (capacity > std::numeric_limits::max()) throw Error("attribute set of size %d is too big", capacity); - return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity); -} - - -void EvalState::mkAttrs(Value & v, size_t capacity) -{ - v.mkAttrs(allocBindings(capacity)); nrAttrsets++; nrAttrsInAttrsets += capacity; + return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity); } @@ -67,9 +62,7 @@ void Bindings::sort() Value & Value::mkAttrs(BindingsBuilder & bindings) { - clearValue(); - internalType = tAttrs; - attrs = bindings.finish(); + mkAttrs(bindings.finish()); return *this; } diff --git a/src/libexpr/attr-set.hh b/src/libexpr/attr-set.hh index 903289b69..82c348287 100644 --- a/src/libexpr/attr-set.hh +++ b/src/libexpr/attr-set.hh @@ -118,13 +118,14 @@ public: call finish(), which sorts the bindings. */ class BindingsBuilder { - EvalState & state; Bindings * bindings; public: + EvalState & state; + BindingsBuilder(EvalState & state, Bindings * bindings) - : state(state), bindings(bindings) + : bindings(bindings), state(state) { } void insert(Symbol name, Value * value, ptr pos = ptr(&noPos)) @@ -146,6 +147,11 @@ public: bindings->sort(); return bindings; } + + Bindings * alreadySorted() + { + return bindings; + } }; } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 112690189..81aa86641 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -145,7 +145,7 @@ void printValue(std::ostream & str, std::set & active, const Valu str << v.fpoint; break; default: - throw Error("invalid value"); + abort(); } active.erase(&v); @@ -1065,8 +1065,8 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v) void ExprAttrs::eval(EvalState & state, Env & env, Value & v) { - state.mkAttrs(v, attrs.size() + dynamicAttrs.size()); - Env *dynamicEnv = &env; + v.mkAttrs(state.buildBindings(attrs.size() + dynamicAttrs.size()).finish()); + auto dynamicEnv = &env; if (recursive) { /* Create a new environment that contains the attributes in @@ -1592,7 +1592,7 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v) if (v1.attrs->size() == 0) { v = v2; return; } if (v2.attrs->size() == 0) { v = v1; return; } - state.mkAttrs(v, v1.attrs->size() + v2.attrs->size()); + auto attrs = state.buildBindings(v1.attrs->size() + v2.attrs->size()); /* Merge the sets, preferring values from the second set. Make sure to keep the resulting vector in sorted order. */ @@ -1601,17 +1601,19 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v) while (i != v1.attrs->end() && j != v2.attrs->end()) { if (i->name == j->name) { - v.attrs->push_back(*j); + attrs.insert(*j); ++i; ++j; } else if (i->name < j->name) - v.attrs->push_back(*i++); + attrs.insert(*i++); else - v.attrs->push_back(*j++); + attrs.insert(*j++); } - while (i != v1.attrs->end()) v.attrs->push_back(*i++); - while (j != v2.attrs->end()) v.attrs->push_back(*j++); + while (i != v1.attrs->end()) attrs.insert(*i++); + while (j != v2.attrs->end()) attrs.insert(*j++); + + v.mkAttrs(attrs.alreadySorted()); state.nrOpUpdateValuesCopied += v.attrs->size(); } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index c0d88201a..89814785e 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -347,7 +347,6 @@ public: } void mkList(Value & v, size_t length); - void mkAttrs(Value & v, size_t capacity); void mkThunk_(Value & v, Expr * expr); void mkPos(Value & v, ptr pos); @@ -400,6 +399,8 @@ private: friend struct ExprSelect; friend void prim_getAttr(EvalState & state, const Pos & pos, Value * * args, Value & v); friend void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v); + + friend struct Value; }; diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 4dc41729a..88716250c 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -37,10 +37,10 @@ class JSONSax : nlohmann::json_sax { ValueMap attrs; std::unique_ptr resolve(EvalState & state) override { - Value & v = parent->value(state); - state.mkAttrs(v, attrs.size()); + auto attrs2 = state.buildBindings(attrs.size()); for (auto & i : attrs) - v.attrs->push_back(Attr(i.first, i.second)); + attrs2.insert(i.first, i.second); + parent->value(state).mkAttrs(attrs2.alreadySorted()); return std::move(parent); } void add() override { v = nullptr; } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 879dfa0a3..d7382af6b 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2274,11 +2274,12 @@ static void prim_removeAttrs(EvalState & state, const Pos & pos, Value * * args, /* Copy all attributes not in that set. Note that we don't need to sort v.attrs because it's a subset of an already sorted vector. */ - state.mkAttrs(v, args[0]->attrs->size()); + auto attrs = state.buildBindings(args[0]->attrs->size()); for (auto & i : *args[0]->attrs) { if (!names.count(i.name)) - v.attrs->push_back(i); + attrs.insert(i); } + v.mkAttrs(attrs.alreadySorted()); } static RegisterPrimOp primop_removeAttrs({ @@ -2369,13 +2370,15 @@ static void prim_intersectAttrs(EvalState & state, const Pos & pos, Value * * ar state.forceAttrs(*args[0], pos); state.forceAttrs(*args[1], pos); - state.mkAttrs(v, std::min(args[0]->attrs->size(), args[1]->attrs->size())); + auto attrs = state.buildBindings(std::min(args[0]->attrs->size(), args[1]->attrs->size())); for (auto & i : *args[0]->attrs) { Bindings::iterator j = args[1]->attrs->find(i.name); if (j != args[1]->attrs->end()) - v.attrs->push_back(*j); + attrs.insert(*j); } + + v.mkAttrs(attrs.alreadySorted()); } static RegisterPrimOp primop_intersectAttrs({ @@ -2429,7 +2432,7 @@ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args { state.forceValue(*args[0], pos); if (args[0]->isPrimOpApp() || args[0]->isPrimOp()) { - state.mkAttrs(v, 0); + v.mkAttrs(&state.emptyBindings); return; } if (!args[0]->isLambda()) @@ -2439,7 +2442,7 @@ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args }); if (!args[0]->lambda.fun->hasFormals()) { - state.mkAttrs(v, 0); + v.mkAttrs(&state.emptyBindings); return; } @@ -2472,15 +2475,17 @@ static void prim_mapAttrs(EvalState & state, const Pos & pos, Value * * args, Va { state.forceAttrs(*args[1], pos); - state.mkAttrs(v, args[1]->attrs->size()); + auto attrs = state.buildBindings(args[1]->attrs->size()); for (auto & i : *args[1]->attrs) { Value * vName = state.allocValue(); Value * vFun2 = state.allocValue(); vName->mkString(i.name); vFun2->mkApp(args[0], vName); - state.allocAttr(v, i.name)->mkApp(vFun2, i.value); + attrs.alloc(i.name).mkApp(vFun2, i.value); } + + v.mkAttrs(attrs.alreadySorted()); } static RegisterPrimOp primop_mapAttrs({ @@ -2526,12 +2531,13 @@ static void prim_zipAttrsWith(EvalState & state, const Pos & pos, Value * * args } } - state.mkAttrs(v, attrsSeen.size()); + auto attrs = state.buildBindings(attrsSeen.size()); for (auto & [sym, elem] : attrsSeen) { - Value * list = state.allocAttr(v, sym); - state.mkList(*list, elem.first); - elem.second = list->listElems(); + auto & list = attrs.alloc(sym); + state.mkList(list, elem.first); + elem.second = list.listElems(); } + v.mkAttrs(attrs.alreadySorted()); for (unsigned int n = 0; n < listSize; ++n) { Value * vElem = listElems[n]; @@ -3054,14 +3060,16 @@ static void prim_groupBy(EvalState & state, const Pos & pos, Value * * args, Val vector->second.push_back(vElem); } - state.mkAttrs(v, attrs.size()); + auto attrs2 = state.buildBindings(attrs.size()); for (auto & i : attrs) { - Value * list = state.allocAttr(v, i.first); + auto & list = attrs2.alloc(i.first); auto size = i.second.size(); - state.mkList(*list, size); - memcpy(list->listElems(), i.second.data(), sizeof(Value *) * size); + state.mkList(list, size); + memcpy(list.listElems(), i.second.data(), sizeof(Value *) * size); } + + v.mkAttrs(attrs2.alreadySorted()); } static RegisterPrimOp primop_groupBy({ @@ -3829,7 +3837,7 @@ void EvalState::createBaseEnv() Value v; /* `builtins' must be first! */ - mkAttrs(v, 128); + v.mkAttrs(buildBindings(128).finish()); addConstant("builtins", v); v.mkBool(true);