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
```
This commit is contained in:
parent
50123f2a56
commit
adbd08399c
4 changed files with 47 additions and 29 deletions
|
@ -140,16 +140,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";
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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"),
|
||||
|
|
|
@ -771,18 +771,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)
|
||||
{
|
||||
|
@ -827,7 +835,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,
|
||||
|
@ -1104,7 +1112,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;
|
||||
|
||||
|
@ -1330,9 +1338,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"
|
||||
)
|
||||
|
@ -1401,7 +1407,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;
|
||||
|
@ -1602,7 +1608,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
|
|||
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], "'")
|
||||
|
@ -1737,9 +1743,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;
|
||||
|
@ -2123,7 +2127,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;
|
||||
|
||||
|
|
|
@ -171,11 +171,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>
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue