From 6feba520085c78f105cc9ebce4f81ed8cdaed085 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 4 Mar 2024 04:08:28 +0100 Subject: [PATCH] Merge pull request #8895 from hercules-ci/gc-before-stats eval: Run a full GC before printing stats (cherry picked from commit aeea49609be014b1928c95b7ec28dbedeb4f032a) Change-Id: I47a23d3a7a47ea61d9a2b5727b638f879f3aaf1e --- src/libcmd/command.cc | 2 +- src/libexpr/eval.cc | 212 ++++++++++++++----------- src/libexpr/eval.hh | 20 ++- src/nix-build/nix-build.cc | 2 +- src/nix-env/nix-env.cc | 2 +- src/nix-instantiate/nix-instantiate.cc | 2 +- 6 files changed, 141 insertions(+), 99 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 6d445fd96..e03a0e615 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 } } -void EvalState::printStats() +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::maybePrintStats() { 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,105 +2521,104 @@ 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["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["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 553cbe4fd..42dd73d11 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -709,9 +709,25 @@ 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(); + void maybePrintStats(); + + /** + * 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 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 b112e8cb3..d0ec206a9 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; }