From f95364a803ddf694e4d7a7f6841101b93d9741fe Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 1 Sep 2023 12:20:54 +0200 Subject: [PATCH 1/2] eval: Run a full GC before printing stats This makes the numbers more deterministic, especially when it comes to the final heap size. --- src/libexpr/eval.cc | 212 +++++++++++++++++++++++++------------------- src/libexpr/eval.hh | 18 +++- 2 files changed, 136 insertions(+), 94 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index a78c0afb1..600444955 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2477,10 +2477,37 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v } } +bool EvalState::fullGC() { +#if HAVE_BOEHMGC + GC_gcollect(); + // Check that it ran. We might replace this with a version that uses more + // of the boehm API to get this reliably, at a maintenance cost. + // We use a 1K margin because technically this has a race condtion, but we + // probably won't encounter it in practice, because the CLI isn't concurrent + // like that. + return GC_get_bytes_since_gc() < 1024; +#else + return false; +#endif +} + void EvalState::printStats() { bool showStats = getEnv("NIX_SHOW_STATS").value_or("0") != "0"; + if (showStats) { + // Make the final heap size more deterministic. +#if HAVE_BOEHMGC + if (!fullGC()) { + warn("failed to perform a full GC before reporting stats"); + } +#endif + printStatistics(); + } +} + +void EvalState::printStatistics() +{ struct rusage buf; getrusage(RUSAGE_SELF, &buf); float cpuTime = buf.ru_utime.tv_sec + ((float) buf.ru_utime.tv_usec / 1000000); @@ -2494,106 +2521,105 @@ void EvalState::printStats() GC_word heapSize, totalBytes; GC_get_heap_usage_safe(&heapSize, 0, 0, 0, &totalBytes); #endif - if (showStats) { - auto outPath = getEnv("NIX_SHOW_STATS_PATH").value_or("-"); - std::fstream fs; - if (outPath != "-") - fs.open(outPath, std::fstream::out); - json topObj = json::object(); - topObj["cpuTime"] = cpuTime; - topObj["envs"] = { - {"number", nrEnvs}, - {"elements", nrValuesInEnvs}, - {"bytes", bEnvs}, - }; - topObj["nrExprs"] = Expr::nrExprs; - topObj["list"] = { - {"elements", nrListElems}, - {"bytes", bLists}, - {"concats", nrListConcats}, - }; - topObj["values"] = { - {"number", nrValues}, - {"bytes", bValues}, - }; - topObj["symbols"] = { - {"number", symbols.size()}, - {"bytes", symbols.totalSize()}, - }; - topObj["sets"] = { - {"number", nrAttrsets}, - {"bytes", bAttrsets}, - {"elements", nrAttrsInAttrsets}, - }; - topObj["sizes"] = { - {"Env", sizeof(Env)}, - {"Value", sizeof(Value)}, - {"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; + + auto outPath = getEnv("NIX_SHOW_STATS_PATH").value_or("-"); + std::fstream fs; + if (outPath != "-") + fs.open(outPath, std::fstream::out); + json topObj = json::object(); + topObj["cpuTime"] = cpuTime; + topObj["envs"] = { + {"number", nrEnvs}, + {"elements", nrValuesInEnvs}, + {"bytes", bEnvs}, + }; + topObj["nrExprs"] = Expr::nrExprs; + topObj["list"] = { + {"elements", nrListElems}, + {"bytes", bLists}, + {"concats", nrListConcats}, + }; + topObj["values"] = { + {"number", nrValues}, + {"bytes", bValues}, + }; + topObj["symbols"] = { + {"number", symbols.size()}, + {"bytes", symbols.totalSize()}, + }; + topObj["sets"] = { + {"number", nrAttrsets}, + {"bytes", bAttrsets}, + {"elements", nrAttrsInAttrsets}, + }; + topObj["sizes"] = { + {"Env", sizeof(Env)}, + {"Value", sizeof(Value)}, + {"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; #if HAVE_BOEHMGC - topObj["gc"] = { - {"heapSize", heapSize}, - {"totalBytes", totalBytes}, - }; + topObj["gc"] = { + {"heapSize", heapSize}, + {"totalBytes", totalBytes}, + }; #endif - if (countCalls) { - topObj["primops"] = primOpCalls; - { - auto& list = topObj["functions"]; - list = json::array(); - for (auto & [fun, count] : functionCalls) { - json obj = json::object(); - if (fun->name) - obj["name"] = (std::string_view) symbols[fun->name]; - else - obj["name"] = nullptr; - if (auto pos = positions[fun->pos]) { - if (auto path = std::get_if(&pos.origin)) - obj["file"] = path->to_string(); - obj["line"] = pos.line; - obj["column"] = pos.column; - } - obj["count"] = count; - list.push_back(obj); - } - } - { - auto list = topObj["attributes"]; - list = json::array(); - for (auto & i : attrSelects) { - json obj = json::object(); - if (auto pos = positions[i.first]) { - if (auto path = std::get_if(&pos.origin)) - obj["file"] = path->to_string(); - obj["line"] = pos.line; - obj["column"] = pos.column; - } - obj["count"] = i.second; - list.push_back(obj); + if (countCalls) { + topObj["primops"] = primOpCalls; + { + auto& list = topObj["functions"]; + list = json::array(); + for (auto & [fun, count] : functionCalls) { + json obj = json::object(); + if (fun->name) + obj["name"] = (std::string_view) symbols[fun->name]; + else + obj["name"] = nullptr; + if (auto pos = positions[fun->pos]) { + if (auto path = std::get_if(&pos.origin)) + obj["file"] = path->to_string(); + obj["line"] = pos.line; + obj["column"] = pos.column; } + obj["count"] = count; + list.push_back(obj); } } + { + auto list = topObj["attributes"]; + list = json::array(); + for (auto & i : attrSelects) { + json obj = json::object(); + if (auto pos = positions[i.first]) { + if (auto path = std::get_if(&pos.origin)) + obj["file"] = path->to_string(); + obj["line"] = pos.line; + obj["column"] = pos.column; + } + obj["count"] = i.second; + list.push_back(obj); + } + } + } - if (getEnv("NIX_SHOW_SYMBOLS").value_or("0") != "0") { - // XXX: overrides earlier assignment - topObj["symbols"] = json::array(); - auto &list = topObj["symbols"]; - symbols.dump([&](const std::string & s) { list.emplace_back(s); }); - } - if (outPath == "-") { - std::cerr << topObj.dump(2) << std::endl; - } else { - fs << topObj.dump(2) << std::endl; - } + if (getEnv("NIX_SHOW_SYMBOLS").value_or("0") != "0") { + // XXX: overrides earlier assignment + topObj["symbols"] = json::array(); + auto &list = topObj["symbols"]; + symbols.dump([&](const std::string & s) { list.emplace_back(s); }); + } + if (outPath == "-") { + std::cerr << topObj.dump(2) << std::endl; + } else { + fs << topObj.dump(2) << std::endl; } } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index fa8fa462b..3bfa34ef6 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -709,10 +709,26 @@ public: void concatLists(Value & v, size_t nrLists, Value * * lists, const PosIdx pos, std::string_view errorCtx); /** - * Print statistics. + * Print statistics, if enabled. + * + * Performs a full memory GC before printing the statistics, so that the + * GC statistics are more accurate. */ void printStats(); + /** + * Print statistics, unconditionally, cheaply, without performing a GC first. + */ + void printStatistics(); + + /** + * Perform a full memory garbage collection - not incremental. + * + * @return true if Nix was built with GC and a GC was performed, false if not. + * The return value is currently not thread safe - just the return value. + */ + bool fullGC(); + /** * Realise the given context, and return a mapping from the placeholders * used to construct the associated value to their final store path From c32084a12cb922f54829b329fa1527f865e5b8ca Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 9 Oct 2023 16:25:53 +0200 Subject: [PATCH 2/2] printStats -> maybePrintStats --- src/libcmd/command.cc | 2 +- src/libexpr/eval.cc | 2 +- src/libexpr/eval.hh | 2 +- src/nix-build/nix-build.cc | 2 +- src/nix-env/nix-env.cc | 2 +- src/nix-instantiate/nix-instantiate.cc | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index 4fc197956..a88ba8134 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -98,7 +98,7 @@ EvalCommand::EvalCommand() EvalCommand::~EvalCommand() { if (evalState) - evalState->printStats(); + evalState->maybePrintStats(); } ref EvalCommand::getEvalStore() diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 600444955..b18eb5266 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2491,7 +2491,7 @@ bool EvalState::fullGC() { #endif } -void EvalState::printStats() +void EvalState::maybePrintStats() { bool showStats = getEnv("NIX_SHOW_STATS").value_or("0") != "0"; diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 3bfa34ef6..c95558952 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -714,7 +714,7 @@ public: * Performs a full memory GC before printing the statistics, so that the * GC statistics are more accurate. */ - void printStats(); + void maybePrintStats(); /** * Print statistics, unconditionally, cheaply, without performing a GC first. diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index e2189fc66..2895c5e3c 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -344,7 +344,7 @@ static void main_nix_build(int argc, char * * argv) } } - state->printStats(); + state->maybePrintStats(); auto buildPaths = [&](const std::vector & paths) { /* Note: we do this even when !printMissing to efficiently diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index e455a50fd..01742daa8 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -1531,7 +1531,7 @@ static int main_nix_env(int argc, char * * argv) op(globals, std::move(opFlags), std::move(opArgs)); - globals.state->printStats(); + globals.state->maybePrintStats(); return 0; } diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index 446b27e66..d40196497 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -189,7 +189,7 @@ static int main_nix_instantiate(int argc, char * * argv) evalOnly, outputKind, xmlOutputSourceLocation, e); } - state->printStats(); + state->maybePrintStats(); return 0; }