mirror of
https://github.com/NixOS/nix.git
synced 2024-11-22 14:52:55 +00:00
defer formals duplicate check for incresed efficiency all round
if we defer the duplicate argument check for lambda formals we can use more efficient data structures for the formals set, and we can get rid of the duplication of formals names to boot. instead of a list of formals we've seen and a set of names we'll keep a vector instead and run a sort+dupcheck step before moving the parsed formals into a newly created lambda. this improves performance on search and rebuild by ~1%, pure parsing gains more (about 4%). this does reorder lambda arguments in the xml output, but the output is still stable. this shouldn't be a problem since argument order is not semantically important anyway. before nix search --no-eval-cache --offline ../nixpkgs hello Time (mean ± σ): 8.550 s ± 0.060 s [User: 6.470 s, System: 1.664 s] Range (min … max): 8.435 s … 8.666 s 20 runs nix eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix Time (mean ± σ): 346.7 ms ± 2.1 ms [User: 312.4 ms, System: 34.2 ms] Range (min … max): 343.8 ms … 353.4 ms 20 runs nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system' Time (mean ± σ): 2.720 s ± 0.031 s [User: 2.415 s, System: 0.231 s] Range (min … max): 2.662 s … 2.780 s 20 runs after nix search --no-eval-cache --offline ../nixpkgs hello Time (mean ± σ): 8.462 s ± 0.063 s [User: 6.398 s, System: 1.661 s] Range (min … max): 8.339 s … 8.542 s 20 runs nix eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix Time (mean ± σ): 329.1 ms ± 1.4 ms [User: 296.8 ms, System: 32.3 ms] Range (min … max): 326.1 ms … 330.8 ms 20 runs nix eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system' Time (mean ± σ): 2.687 s ± 0.035 s [User: 2.392 s, System: 0.228 s] Range (min … max): 2.626 s … 2.754 s 20 runs
This commit is contained in:
parent
9ac836d1d6
commit
7d4cc5515c
@ -1368,7 +1368,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value &
|
||||
/* Nope, so show the first unexpected argument to the
|
||||
user. */
|
||||
for (auto & i : *args[0]->attrs)
|
||||
if (!lambda.formals->argNames.count(i.name))
|
||||
if (!lambda.formals->has(i.name))
|
||||
throwTypeError(pos, "%1% called with unexpected argument '%2%'", lambda, i.name);
|
||||
abort(); // can't happen
|
||||
}
|
||||
|
@ -216,10 +216,25 @@ struct Formal
|
||||
|
||||
struct Formals
|
||||
{
|
||||
typedef std::list<Formal> Formals_;
|
||||
typedef std::vector<Formal> Formals_;
|
||||
Formals_ formals;
|
||||
std::set<Symbol> argNames; // used during parsing
|
||||
bool ellipsis;
|
||||
|
||||
bool has(Symbol arg) const {
|
||||
auto it = std::lower_bound(formals.begin(), formals.end(), arg,
|
||||
[] (const Formal & f, const Symbol & sym) { return f.name < sym; });
|
||||
return it != formals.end() && it->name == arg;
|
||||
}
|
||||
|
||||
std::vector<Formal> lexicographicOrder() const
|
||||
{
|
||||
std::vector<Formal> result(formals.begin(), formals.end());
|
||||
std::sort(result.begin(), result.end(),
|
||||
[] (const Formal & a, const Formal & b) {
|
||||
return std::string_view(a.name) < std::string_view(b.name);
|
||||
});
|
||||
return result;
|
||||
}
|
||||
};
|
||||
|
||||
struct ExprLambda : Expr
|
||||
@ -232,11 +247,6 @@ struct ExprLambda : Expr
|
||||
ExprLambda(const Pos & pos, const Symbol & arg, Formals * formals, Expr * body)
|
||||
: pos(pos), arg(arg), formals(formals), body(body)
|
||||
{
|
||||
if (!arg.empty() && formals && formals->argNames.find(arg) != formals->argNames.end())
|
||||
throw ParseError({
|
||||
.msg = hintfmt("duplicate formal function argument '%1%'", arg),
|
||||
.errPos = pos
|
||||
});
|
||||
};
|
||||
void setName(Symbol & name);
|
||||
string showNamePos() const;
|
||||
|
@ -41,6 +41,11 @@ namespace nix {
|
||||
{ };
|
||||
};
|
||||
|
||||
struct ParserFormals {
|
||||
std::vector<Formal> formals;
|
||||
bool ellipsis = false;
|
||||
};
|
||||
|
||||
}
|
||||
|
||||
// using C a struct allows us to avoid having to define the special
|
||||
@ -151,15 +156,39 @@ static void addAttr(ExprAttrs * attrs, AttrPath & attrPath,
|
||||
}
|
||||
|
||||
|
||||
static void addFormal(const Pos & pos, Formals * formals, const Formal & formal)
|
||||
static Formals * toFormals(ParseData & data, ParserFormals * formals,
|
||||
Pos pos = noPos, Symbol arg = {})
|
||||
{
|
||||
if (!formals->argNames.insert(formal.name).second)
|
||||
std::sort(formals->formals.begin(), formals->formals.end(),
|
||||
[] (const auto & a, const auto & b) {
|
||||
return std::tie(a.name, a.pos) < std::tie(b.name, b.pos);
|
||||
});
|
||||
|
||||
std::optional<std::pair<Symbol, Pos>> duplicate;
|
||||
for (size_t i = 0; i + 1 < formals->formals.size(); i++) {
|
||||
if (formals->formals[i].name != formals->formals[i + 1].name)
|
||||
continue;
|
||||
std::pair thisDup{formals->formals[i].name, formals->formals[i + 1].pos};
|
||||
duplicate = std::min(thisDup, duplicate.value_or(thisDup));
|
||||
}
|
||||
if (duplicate)
|
||||
throw ParseError({
|
||||
.msg = hintfmt("duplicate formal function argument '%1%'",
|
||||
formal.name),
|
||||
.msg = hintfmt("duplicate formal function argument '%1%'", duplicate->first),
|
||||
.errPos = duplicate->second
|
||||
});
|
||||
|
||||
Formals result;
|
||||
result.ellipsis = formals->ellipsis;
|
||||
result.formals = std::move(formals->formals);
|
||||
|
||||
if (arg.set() && result.has(arg))
|
||||
throw ParseError({
|
||||
.msg = hintfmt("duplicate formal function argument '%1%'", arg),
|
||||
.errPos = pos
|
||||
});
|
||||
formals->formals.push_front(formal);
|
||||
|
||||
delete formals;
|
||||
return new Formals(std::move(result));
|
||||
}
|
||||
|
||||
|
||||
@ -282,7 +311,7 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * err
|
||||
nix::Expr * e;
|
||||
nix::ExprList * list;
|
||||
nix::ExprAttrs * attrs;
|
||||
nix::Formals * formals;
|
||||
nix::ParserFormals * formals;
|
||||
nix::Formal * formal;
|
||||
nix::NixInt n;
|
||||
nix::NixFloat nf;
|
||||
@ -340,11 +369,17 @@ expr_function
|
||||
: ID ':' expr_function
|
||||
{ $$ = new ExprLambda(CUR_POS, data->symbols.create($1), 0, $3); }
|
||||
| '{' formals '}' ':' expr_function
|
||||
{ $$ = new ExprLambda(CUR_POS, data->symbols.create(""), $2, $5); }
|
||||
{ $$ = new ExprLambda(CUR_POS, data->symbols.create(""), toFormals(*data, $2), $5); }
|
||||
| '{' formals '}' '@' ID ':' expr_function
|
||||
{ $$ = new ExprLambda(CUR_POS, data->symbols.create($5), $2, $7); }
|
||||
{
|
||||
Symbol arg = data->symbols.create($5);
|
||||
$$ = new ExprLambda(CUR_POS, arg, toFormals(*data, $2, CUR_POS, arg), $7);
|
||||
}
|
||||
| ID '@' '{' formals '}' ':' expr_function
|
||||
{ $$ = new ExprLambda(CUR_POS, data->symbols.create($1), $4, $7); }
|
||||
{
|
||||
Symbol arg = data->symbols.create($1);
|
||||
$$ = new ExprLambda(CUR_POS, arg, toFormals(*data, $4, CUR_POS, arg), $7);
|
||||
}
|
||||
| ASSERT expr ';' expr_function
|
||||
{ $$ = new ExprAssert(CUR_POS, $2, $4); }
|
||||
| WITH expr ';' expr_function
|
||||
@ -575,13 +610,13 @@ expr_list
|
||||
|
||||
formals
|
||||
: formal ',' formals
|
||||
{ $$ = $3; addFormal(CUR_POS, $$, *$1); }
|
||||
{ $$ = $3; $$->formals.push_back(*$1); }
|
||||
| formal
|
||||
{ $$ = new Formals; addFormal(CUR_POS, $$, *$1); $$->ellipsis = false; }
|
||||
{ $$ = new ParserFormals; $$->formals.push_back(*$1); $$->ellipsis = false; }
|
||||
|
|
||||
{ $$ = new Formals; $$->ellipsis = false; }
|
||||
{ $$ = new ParserFormals; $$->ellipsis = false; }
|
||||
| ELLIPSIS
|
||||
{ $$ = new Formals; $$->ellipsis = true; }
|
||||
{ $$ = new ParserFormals; $$->ellipsis = true; }
|
||||
;
|
||||
|
||||
formal
|
||||
|
@ -142,7 +142,7 @@ static void printValueAsXML(EvalState & state, bool strict, bool location,
|
||||
if (!v.lambda.fun->arg.empty()) attrs["name"] = v.lambda.fun->arg;
|
||||
if (v.lambda.fun->formals->ellipsis) attrs["ellipsis"] = "1";
|
||||
XMLOpenElement _(doc, "attrspat", attrs);
|
||||
for (auto & i : v.lambda.fun->formals->formals)
|
||||
for (auto & i : v.lambda.fun->formals->lexicographicOrder())
|
||||
doc.writeEmptyElement("attr", singletonAttrs("name", i.name));
|
||||
} else
|
||||
doc.writeEmptyElement("varpat", singletonAttrs("name", v.lambda.fun->arg));
|
||||
|
@ -476,8 +476,8 @@ struct CmdFlakeCheck : FlakeCommand
|
||||
if (!v.isLambda())
|
||||
throw Error("bundler must be a function");
|
||||
if (!v.lambda.fun->formals ||
|
||||
!v.lambda.fun->formals->argNames.count(state->symbols.create("program")) ||
|
||||
!v.lambda.fun->formals->argNames.count(state->symbols.create("system")))
|
||||
!v.lambda.fun->formals->has(state->symbols.create("program")) ||
|
||||
!v.lambda.fun->formals->has(state->symbols.create("system")))
|
||||
throw Error("bundler must take formal arguments 'program' and 'system'");
|
||||
} catch (Error & e) {
|
||||
e.addTrace(pos, hintfmt("while checking the template '%s'", attrPath));
|
||||
|
@ -50,6 +50,6 @@ with import ./config.nix;
|
||||
|
||||
fetchurl = import <nix/fetchurl.nix> {
|
||||
url = "file://" + toString ./lang/eval-okay-xml.exp.xml;
|
||||
sha256 = "0kg4sla7ihm8ijr8cb3117fhl99zrc2bwy1jrngsfmkh8bav4m0v";
|
||||
sha256 = "sha256-behBlX+DQK/Pjvkuc8Tx68Jwi4E5v86wDq+ZLaHyhQE=";
|
||||
};
|
||||
}
|
||||
|
@ -31,9 +31,9 @@
|
||||
<attr name="f">
|
||||
<function>
|
||||
<attrspat>
|
||||
<attr name="z" />
|
||||
<attr name="x" />
|
||||
<attr name="y" />
|
||||
<attr name="z" />
|
||||
</attrspat>
|
||||
</function>
|
||||
</attr>
|
||||
|
Loading…
Reference in New Issue
Block a user