From dd7d7450a58d42911623d2c8e230ca1c21d4810b Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Tue, 3 Dec 2024 20:38:41 +0100 Subject: [PATCH] libexpr: move eval statistics into new struct there's no reason for these to be private. much more important things that are now part of EvalState are not private, and having statistics be private forces us to add otherwise unnecessary friend declarations Change-Id: Ib37097d94a9f55c2b21969fb6c51049b1c914515 --- lix/libexpr/eval.cc | 70 +++++++++++++++++++++--------------------- lix/libexpr/eval.hh | 58 +++++++++++++--------------------- lix/libexpr/primops.cc | 2 +- 3 files changed, 57 insertions(+), 73 deletions(-) diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 070d02813..7a7f44c54 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -335,7 +335,7 @@ EvalState::EvalState( } , errors{positions, debug.get()} { - countCalls = getEnv("NIX_COUNT_CALLS").value_or("0") != "0"; + stats.countCalls = getEnv("NIX_COUNT_CALLS").value_or("0") != "0"; static_assert(sizeof(Env) <= 16, "environment must be <= 16 bytes"); } @@ -758,7 +758,7 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval) forceAttrs(*env->values[0], fromWith->pos, "while evaluating the first subexpression of a with expression"); Bindings::iterator j = env->values[0]->attrs->find(var.name); if (j != env->values[0]->attrs->end()) { - if (countCalls) attrSelects[j->pos]++; + if (stats.countCalls) stats.attrSelects[j->pos]++; return j->value; } if (!fromWith->parentWith) @@ -782,7 +782,7 @@ Value EvalMemory::newList(size_t size) void EvalState::evalLazily(Expr & e, Value & v) { v.mkThunk(&builtins.env, e); - nrThunks++; + stats.nrThunks++; } @@ -882,7 +882,7 @@ Value * Expr::maybeThunk(EvalState & state, Env & env) { Value * v = state.mem.allocValue(); v->mkThunk(&env, *this); - state.nrThunks++; + state.stats.nrThunks++; return v; } @@ -892,32 +892,32 @@ Value * ExprVar::maybeThunk(EvalState & state, Env & env) Value * v = state.lookupVar(&env, *this, true); /* The value might not be initialised in the environment yet. In that case, ignore it. */ - if (v) { state.nrAvoided++; return v; } + if (v) { state.stats.nrAvoided++; return v; } return Expr::maybeThunk(state, env); } Value * ExprString::maybeThunk(EvalState & state, Env & env) { - state.nrAvoided++; + state.stats.nrAvoided++; return &v; } Value * ExprInt::maybeThunk(EvalState & state, Env & env) { - state.nrAvoided++; + state.stats.nrAvoided++; return &v; } Value * ExprFloat::maybeThunk(EvalState & state, Env & env) { - state.nrAvoided++; + state.stats.nrAvoided++; return &v; } Value * ExprPath::maybeThunk(EvalState & state, Env & env) { - state.nrAvoided++; + state.stats.nrAvoided++; return &v; } @@ -1082,7 +1082,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) if (hasOverrides && i.second.kind != AttrDef::Kind::Inherited) { vAttr = state.mem.allocValue(); vAttr->mkThunk(i.second.chooseByKind(&env2, &env, inheritEnv), *i.second.e); - state.nrThunks++; + state.stats.nrThunks++; } else vAttr = i.second.e->maybeThunk(state, *i.second.chooseByKind(&env2, &env, inheritEnv)); env2.values[displ++] = vAttr; @@ -1263,7 +1263,7 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) : nullptr; for (auto const & [partIdx, currentAttrName] : enumerate(attrPath)) { - state.nrLookups++; + state.stats.nrLookups++; Symbol const name = getName(currentAttrName, state, env); @@ -1350,7 +1350,7 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) // Set our currently operated-on attrset to this one, and keep going. vCurrent = attrIt->value; posCurrent = attrIt->pos; - if (state.countCalls) state.attrSelects[posCurrent]++; + if (state.stats.countCalls) state.stats.attrSelects[posCurrent]++; } state.forceValue(*vCurrent, (posCurrent ? posCurrent : this->pos)); @@ -1561,8 +1561,8 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & } - nrFunctionCalls++; - if (countCalls) incrFunctionCall(&lambda); + stats.nrFunctionCalls++; + if (stats.countCalls) stats.addCall(lambda); /* Evaluate the body. */ try { @@ -1601,8 +1601,8 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & /* We have all the arguments, so call the primop. */ auto * fn = vCur.primOp; - nrPrimOpCalls++; - if (countCalls) primOpCalls[fn->name]++; + stats.nrPrimOpCalls++; + if (stats.countCalls) stats.primOpCalls[fn->name]++; try { fn->fun(*this, vCur.determinePos(noPos), args, vCur); @@ -1655,8 +1655,8 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & vArgs[argsDone + i] = args[i]; auto fn = primOp->primOp; - nrPrimOpCalls++; - if (countCalls) primOpCalls[fn->name]++; + stats.nrPrimOpCalls++; + if (stats.countCalls) stats.primOpCalls[fn->name]++; try { // TODO: @@ -1736,9 +1736,9 @@ void ExprCall::eval(EvalState & state, Env & env, Value & v) // Lifted out of callFunction() because it creates a temporary that // prevents tail-call optimisation. -void EvalState::incrFunctionCall(ExprLambda * fun) +void EvalStatistics::addCall(ExprLambda & fun) { - functionCalls[fun]++; + functionCalls[&fun]++; } @@ -1866,7 +1866,7 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v) state.evalAttrs(env, *e1, v1, pos, "in the left operand of the update (//) operator"); state.evalAttrs(env, *e2, v2, pos, "in the right operand of the update (//) operator"); - state.nrOpUpdates++; + state.stats.nrOpUpdates++; if (v1.attrs->size() == 0) { v = v2; return; } if (v2.attrs->size() == 0) { v = v1; return; } @@ -1894,7 +1894,7 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v) v.mkAttrs(attrs.alreadySorted()); - state.nrOpUpdateValuesCopied += v.attrs->size(); + state.stats.nrOpUpdateValuesCopied += v.attrs->size(); } @@ -1909,7 +1909,7 @@ void ExprOpConcatLists::eval(EvalState & state, Env & env, Value & v) void EvalState::concatLists(Value & v, size_t nrLists, Value * * lists, const PosIdx pos, std::string_view errorCtx) { - nrListConcats++; + stats.nrListConcats++; Value * nonEmpty = 0; size_t len = 0; @@ -2582,7 +2582,7 @@ void EvalState::printStatistics() topObj["list"] = { {"elements", mem.nrListElems}, {"bytes", bLists}, - {"concats", nrListConcats}, + {"concats", stats.nrListConcats}, }; topObj["values"] = { {"number", mem.nrValues}, @@ -2603,13 +2603,13 @@ void EvalState::printStatistics() {"Bindings", sizeof(Bindings)}, {"Attr", sizeof(Attr)}, }; - topObj["nrOpUpdates"] = nrOpUpdates; - topObj["nrOpUpdateValuesCopied"] = nrOpUpdateValuesCopied; - topObj["nrThunks"] = nrThunks; - topObj["nrAvoided"] = nrAvoided; - topObj["nrLookups"] = nrLookups; - topObj["nrPrimOpCalls"] = nrPrimOpCalls; - topObj["nrFunctionCalls"] = nrFunctionCalls; + topObj["nrOpUpdates"] = stats.nrOpUpdates; + topObj["nrOpUpdateValuesCopied"] = stats.nrOpUpdateValuesCopied; + topObj["nrThunks"] = stats.nrThunks; + topObj["nrAvoided"] = stats.nrAvoided; + topObj["nrLookups"] = stats.nrLookups; + topObj["nrPrimOpCalls"] = stats.nrPrimOpCalls; + topObj["nrFunctionCalls"] = stats.nrFunctionCalls; #if HAVE_BOEHMGC topObj["gc"] = { {"heapSize", heapSize}, @@ -2617,12 +2617,12 @@ void EvalState::printStatistics() }; #endif - if (countCalls) { - topObj["primops"] = primOpCalls; + if (stats.countCalls) { + topObj["primops"] = stats.primOpCalls; { auto& list = topObj["functions"]; list = json::array(); - for (auto & [fun, count] : functionCalls) { + for (auto & [fun, count] : stats.functionCalls) { json obj = json::object(); if (fun->name) obj["name"] = (std::string_view) symbols[fun->name]; @@ -2641,7 +2641,7 @@ void EvalState::printStatistics() { auto list = topObj["attributes"]; list = json::array(); - for (auto & i : attrSelects) { + for (auto & i : stats.attrSelects) { json obj = json::object(); if (auto pos = positions[i.first]) { if (auto path = std::get_if(&pos.origin)) diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh index bed5aecce..90fca9443 100644 --- a/lix/libexpr/eval.hh +++ b/lix/libexpr/eval.hh @@ -464,6 +464,26 @@ public: [[nodiscard]] StringMap realiseContext(const NixStringContext & context); }; +struct EvalStatistics +{ + unsigned long nrLookups = 0; + unsigned long nrAvoided = 0; + unsigned long nrOpUpdates = 0; + unsigned long nrOpUpdateValuesCopied = 0; + unsigned long nrListConcats = 0; + unsigned long nrPrimOpCalls = 0; + unsigned long nrFunctionCalls = 0; + unsigned long nrThunks = 0; + + bool countCalls = false; + + std::map primOpCalls; + std::map functionCalls; + std::map attrSelects; + + void addCall(ExprLambda & fun); +}; + class EvalState { @@ -477,6 +497,7 @@ public: EvalRuntimeCaches caches; EvalPaths paths; EvalBuiltins builtins; + EvalStatistics stats; /** * If set, force copying files to the Nix store even if they @@ -772,43 +793,6 @@ private: */ std::string mkSingleDerivedPathStringRaw( const SingleDerivedPath & p); - - unsigned long nrLookups = 0; - unsigned long nrAvoided = 0; - unsigned long nrOpUpdates = 0; - unsigned long nrOpUpdateValuesCopied = 0; - unsigned long nrListConcats = 0; - unsigned long nrPrimOpCalls = 0; - unsigned long nrFunctionCalls = 0; - unsigned long nrThunks = 0; - - bool countCalls; - - using PrimOpCalls = std::map; - PrimOpCalls primOpCalls; - - using FunctionCalls = std::map; - FunctionCalls functionCalls; - - void incrFunctionCall(ExprLambda * fun); - - using AttrSelects = std::map; - AttrSelects attrSelects; - - friend struct Expr; - friend struct ExprOpUpdate; - friend struct ExprOpConcatLists; - friend struct ExprVar; - friend struct ExprString; - friend struct ExprInt; - friend struct ExprFloat; - friend struct ExprPath; - friend struct ExprSelect; - friend void prim_getAttr(EvalState & state, const PosIdx pos, Value * * args, Value & v); - friend void prim_match(EvalState & state, const PosIdx pos, Value * * args, Value & v); - friend void prim_split(EvalState & state, const PosIdx pos, Value * * args, Value & v); - - friend struct Value; }; /** diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index ae8f406bf..a8126ac01 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -1671,7 +1671,7 @@ void prim_getAttr(EvalState & state, const PosIdx pos, Value * * args, Value & v "in the attribute set under consideration" ); // !!! add to stack trace? - if (state.countCalls && i->pos) state.attrSelects[i->pos]++; + if (state.stats.countCalls && i->pos) state.stats.attrSelects[i->pos]++; state.forceValue(*i->value, pos); v = *i->value; }