1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2025-06-25 02:21:16 +02:00

{libexpr,libcmd}: Make debugger significantly faster

The underlying issue is that debugger code path was
calling PosTable::operator[] in each eval method.
This has become incredibly expensive since 5d9fdab3de.

While we are it it, I've reworked the code to
not use std::shared_ptr where it really isn't necessary.

As I've documented in previous commits, this is actually
more a workaround for recursive header dependencies now
and is only necessary in `error.hh` code.

Some ad-hoc benchmarking:

After this commit:

```
Benchmark 1: nix eval nixpkgs#hello --impure --ignore-try --no-eval-cache --debugger
  Time (mean ± σ):     784.2 ms ±   7.1 ms    [User: 561.4 ms, System: 147.7 ms]
  Range (min … max):   773.5 ms … 792.6 ms    10 runs
```

On master 3604c7c51:

```
Benchmark 1: nix eval nixpkgs#hello --impure --ignore-try --no-eval-cache --debugger
  Time (mean ± σ):     22.914 s ±  0.178 s    [User: 18.524 s, System: 4.151 s]
  Range (min … max):   22.738 s … 23.290 s    10 runs
```

(cherry picked from commit adbd08399c)
This commit is contained in:
Sergei Zimmerman 2025-03-13 16:24:30 +00:00 committed by Mergify
parent 6faf66d2f7
commit aaf86cc0d5
4 changed files with 47 additions and 29 deletions

View file

@ -157,16 +157,13 @@ static std::ostream & showDebugTrace(std::ostream & out, const PosTable & positi
out << ANSI_RED "error: " << ANSI_NORMAL;
out << dt.hint.str() << "\n";
// prefer direct pos, but if noPos then try the expr.
auto pos = dt.pos
? dt.pos
: positions[dt.expr.getPos() ? dt.expr.getPos() : noPos];
auto pos = dt.getPos(positions);
if (pos) {
out << *pos;
if (auto loc = pos->getCodeLines()) {
out << pos;
if (auto loc = pos.getCodeLines()) {
out << "\n";
printCodeLines(out, "", *pos, *loc);
printCodeLines(out, "", pos, *loc);
out << "\n";
}
}

View file

@ -45,7 +45,7 @@ EvalErrorBuilder<T> & EvalErrorBuilder<T>::withFrame(const Env & env, const Expr
// TODO: check compatibility with nested debugger calls.
// TODO: What side-effects??
error.state.debugTraces.push_front(DebugTrace{
.pos = error.state.positions[expr.getPos()],
.pos = expr.getPos(),
.expr = expr,
.env = env,
.hint = HintFmt("Fake frame for debugging purposes"),

View file

@ -756,18 +756,26 @@ void EvalState::runDebugRepl(const Error * error, const Env & env, const Expr &
if (!debugRepl || inDebugger)
return;
auto dts =
error && expr.getPos()
? std::make_unique<DebugTraceStacker>(
*this,
DebugTrace {
.pos = error->info().pos ? error->info().pos : positions[expr.getPos()],
auto dts = [&]() -> std::unique_ptr<DebugTraceStacker> {
if (error && expr.getPos()) {
auto trace = DebugTrace{
.pos = [&]() -> std::variant<Pos, PosIdx> {
if (error->info().pos) {
if (auto * pos = error->info().pos.get())
return *pos;
return noPos;
}
return expr.getPos();
}(),
.expr = expr,
.env = env,
.hint = error->info().msg,
.isError = true
})
: nullptr;
.isError = true};
return std::make_unique<DebugTraceStacker>(*this, std::move(trace));
}
return nullptr;
}();
if (error)
{
@ -812,7 +820,7 @@ static std::unique_ptr<DebugTraceStacker> makeDebugTraceStacker(
EvalState & state,
Expr & expr,
Env & env,
std::shared_ptr<Pos> && pos,
std::variant<Pos, PosIdx> pos,
const Args & ... formatArgs)
{
return std::make_unique<DebugTraceStacker>(state,
@ -1088,7 +1096,7 @@ void EvalState::evalFile(const SourcePath & path, Value & v, bool mustBeTrivial)
*this,
*e,
this->baseEnv,
e->getPos() ? std::make_shared<Pos>(positions[e->getPos()]) : nullptr,
e->getPos(),
"while evaluating the file '%1%':", resolvedPath.to_string())
: nullptr;
@ -1314,9 +1322,7 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
state,
*this,
env2,
getPos()
? std::make_shared<Pos>(state.positions[getPos()])
: nullptr,
getPos(),
"while evaluating a '%1%' expression",
"let"
)
@ -1385,7 +1391,7 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v)
state,
*this,
env,
state.positions[getPos()],
getPos(),
"while evaluating the attribute '%1%'",
showAttrPath(state, env, attrPath))
: nullptr;
@ -1603,7 +1609,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value &
try {
auto dts = debugRepl
? makeDebugTraceStacker(
*this, *lambda.body, env2, positions[lambda.pos],
*this, *lambda.body, env2, lambda.pos,
"while calling %s",
lambda.name
? concatStrings("'", symbols[lambda.name], "'")
@ -1742,9 +1748,7 @@ void ExprCall::eval(EvalState & state, Env & env, Value & v)
state,
*this,
env,
getPos()
? std::make_shared<Pos>(state.positions[getPos()])
: nullptr,
getPos(),
"while calling a function"
)
: nullptr;
@ -2122,7 +2126,7 @@ void EvalState::forceValueDeep(Value & v)
try {
// If the value is a thunk, we're evaling. Otherwise no trace necessary.
auto dts = debugRepl && i.value->isThunk()
? makeDebugTraceStacker(*this, *i.value->payload.thunk.expr, *i.value->payload.thunk.env, positions[i.pos],
? makeDebugTraceStacker(*this, *i.value->payload.thunk.expr, *i.value->payload.thunk.env, i.pos,
"while evaluating the attribute '%1%'", symbols[i.name])
: nullptr;

View file

@ -155,11 +155,28 @@ struct RegexCache;
std::shared_ptr<RegexCache> makeRegexCache();
struct DebugTrace {
std::shared_ptr<Pos> pos;
/* WARNING: Converting PosIdx -> Pos should be done with extra care. This is
due to the fact that operator[] of PosTable is incredibly expensive. */
std::variant<Pos, PosIdx> pos;
const Expr & expr;
const Env & env;
HintFmt hint;
bool isError;
Pos getPos(const PosTable & table) const
{
return std::visit(
overloaded{
[&](PosIdx idx) {
// Prefer direct pos, but if noPos then try the expr.
if (!idx)
idx = expr.getPos();
return table[idx];
},
[&](Pos pos) { return pos; },
},
pos);
}
};
class EvalState : public std::enable_shared_from_this<EvalState>