nordic-dev.net/nix
nordic-dev.net/nix
2
0
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2024-10-30 05:40:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #9847 from pennae/inherit-from-dedup

Browse Source 
deduplicate inherit-from source expr work
This commit is contained in:
Robert Hensing 2024-02-26 20:25:58 +01:00 committed by GitHub
commit 4c7f0ef6ca
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 229 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
doc/manual/rl-next/inherit-from-by-need.md Normal file
View File

@ -0,0 +1,7 @@
---
synopsis: "`inherit (x) ...` evaluates `x` only once"
prs: 9847
---
`inherit (x) a b ...` now evaluates the expression `x` only once for all inherited attributes rather than once for each inherited attribute.
This does not usually have a measurable impact, but side-effects (such as `builtins.trace`) would be duplicated and expensive expressions (such as derivations) could cause a measurable slowdown.

40
src/libexpr/eval.cc
View File

@ -1198,6 +1198,18 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v)
}
Env * ExprAttrs::buildInheritFromEnv(EvalState & state, Env & up)
{
Env & inheritEnv = state.allocEnv(inheritFromExprs->size());
inheritEnv.up = &up;
Displacement displ = 0;
for (auto from : *inheritFromExprs)
inheritEnv.values[displ++] = from->maybeThunk(state, up);
return &inheritEnv;
}
void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
{
v.mkAttrs(state.buildBindings(attrs.size() + dynamicAttrs.size()).finish());
@ -1209,6 +1221,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
Env & env2(state.allocEnv(attrs.size()));
env2.up = &env;
dynamicEnv = &env2;
Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr;
AttrDefs::iterator overrides = attrs.find(state.sOverrides);
bool hasOverrides = overrides != attrs.end();
@ -1219,11 +1232,11 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
Displacement displ = 0;
for (auto & i : attrs) {
Value * vAttr;
if (hasOverrides && !i.second.inherited) {
if (hasOverrides && i.second.kind != AttrDef::Kind::Inherited) {
vAttr = state.allocValue();
mkThunk(*vAttr, env2, i.second.e);
mkThunk(*vAttr, *i.second.chooseByKind(&env2, &env, inheritEnv), i.second.e);
} else
vAttr = i.second.e->maybeThunk(state, i.second.inherited ? env : env2);
vAttr = i.second.e->maybeThunk(state, *i.second.chooseByKind(&env2, &env, inheritEnv));
env2.values[displ++] = vAttr;
v.attrs->push_back(Attr(i.first, vAttr, i.second.pos));
}
@ -1255,9 +1268,15 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
}
}
else
for (auto & i : attrs)
v.attrs->push_back(Attr(i.first, i.second.e->maybeThunk(state, env), i.second.pos));
else {
Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env) : nullptr;
for (auto & i : attrs) {
v.attrs->push_back(Attr(
i.first,
i.second.e->maybeThunk(state, *i.second.chooseByKind(&env, &env, inheritEnv)),
i.second.pos));
}
}
/* Dynamic attrs apply *after* rec and __overrides. */
for (auto & i : dynamicAttrs) {
@ -1289,12 +1308,17 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
Env & env2(state.allocEnv(attrs->attrs.size()));
env2.up = &env;
Env * inheritEnv = attrs->inheritFromExprs ? attrs->buildInheritFromEnv(state, env2) : nullptr;
/* The recursive attributes are evaluated in the new environment,
while the inherited attributes are evaluated in the original
environment. */
Displacement displ = 0;
for (auto & i : attrs->attrs)
env2.values[displ++] = i.second.e->maybeThunk(state, i.second.inherited ? env : env2);
for (auto & i : attrs->attrs) {
env2.values[displ++] = i.second.e->maybeThunk(
state,
*i.second.chooseByKind(&env2, &env, inheritEnv));
}
auto dts = state.debugRepl
? makeDebugTraceStacker(

113
src/libexpr/nixexpr.cc
View File

@ -70,10 +70,8 @@ void ExprOpHasAttr::show(const SymbolTable & symbols, std::ostream & str) const
str << ") ? " << showAttrPath(symbols, attrPath) << ")";
}
void ExprAttrs::show(const SymbolTable & symbols, std::ostream & str) const
void ExprAttrs::showBindings(const SymbolTable & symbols, std::ostream & str) const
{
if (recursive) str << "rec ";
str << "{ ";
typedef const decltype(attrs)::value_type * Attr;
std::vector<Attr> sorted;
for (auto & i : attrs) sorted.push_back(&i);
@ -81,10 +79,37 @@ void ExprAttrs::show(const SymbolTable & symbols, std::ostream & str) const
std::string_view sa = symbols[a->first], sb = symbols[b->first];
return sa < sb;
});
std::vector<Symbol> inherits;
std::map<ExprInheritFrom *, std::vector<Symbol>> inheritsFrom;
for (auto & i : sorted) {
if (i->second.inherited)
str << "inherit " << symbols[i->first] << " " << "; ";
else {
switch (i->second.kind) {
case AttrDef::Kind::Plain:
break;
case AttrDef::Kind::Inherited:
inherits.push_back(i->first);
break;
case AttrDef::Kind::InheritedFrom: {
auto & select = dynamic_cast<ExprSelect &>(*i->second.e);
auto & from = dynamic_cast<ExprInheritFrom &>(*select.e);
inheritsFrom[&from].push_back(i->first);
break;
}
}
}
if (!inherits.empty()) {
str << "inherit";
for (auto sym : inherits) str << " " << symbols[sym];
str << "; ";
}
for (const auto & [from, syms] : inheritsFrom) {
str << "inherit (";
(*inheritFromExprs)[from->displ]->show(symbols, str);
str << ")";
for (auto sym : syms) str << " " << symbols[sym];
str << "; ";
}
for (auto & i : sorted) {
if (i->second.kind == AttrDef::Kind::Plain) {
str << symbols[i->first] << " = ";
i->second.e->show(symbols, str);
str << "; ";
@ -97,6 +122,13 @@ void ExprAttrs::show(const SymbolTable & symbols, std::ostream & str) const
i.valueExpr->show(symbols, str);
str << "; ";
}
}
void ExprAttrs::show(const SymbolTable & symbols, std::ostream & str) const
{
if (recursive) str << "rec ";
str << "{ ";
showBindings(symbols, str);
str << "}";
}
@ -152,15 +184,7 @@ void ExprCall::show(const SymbolTable & symbols, std::ostream & str) const
void ExprLet::show(const SymbolTable & symbols, std::ostream & str) const
{
str << "(let ";
for (auto & i : attrs->attrs)
if (i.second.inherited) {
str << "inherit " << symbols[i.first] << "; ";
}
else {
str << symbols[i.first] << " = ";
i.second.e->show(symbols, str);
str << "; ";
}
attrs->showBindings(symbols, str);
str << "in ";
body->show(symbols, str);
str << ")";
@ -305,6 +329,12 @@ void ExprVar::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> &
this->level = withLevel;
}
void ExprInheritFrom::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> & env)
{
if (es.debugRepl)
es.exprEnvs.insert(std::make_pair(this, env));
}
void ExprSelect::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> & env)
{
if (es.debugRepl)
@ -328,22 +358,47 @@ void ExprOpHasAttr::bindVars(EvalState & es, const std::shared_ptr<const StaticE
i.expr->bindVars(es, env);
}
std::shared_ptr<const StaticEnv> ExprAttrs::bindInheritSources(
EvalState & es, const std::shared_ptr<const StaticEnv> & env)
{
if (!inheritFromExprs)
return nullptr;
// the inherit (from) source values are inserted into an env of its own, which
// does not introduce any variable names.
// analysis must see an empty env, or an env that contains only entries with
// otherwise unused names to not interfere with regular names. the parser
// has already filled all exprs that access this env with appropriate level
// and displacement, and nothing else is allowed to access it. ideally we'd
// not even *have* an expr that grabs anything from this env since it's fully
// invisible, but the evaluator does not allow for this yet.
auto inner = std::make_shared<StaticEnv>(nullptr, env.get(), 0);
for (auto from : *inheritFromExprs)
from->bindVars(es, env);
return inner;
}
void ExprAttrs::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> & env)
{
if (es.debugRepl)
es.exprEnvs.insert(std::make_pair(this, env));
if (recursive) {
auto newEnv = std::make_shared<StaticEnv>(nullptr, env.get(), recursive ? attrs.size() : 0);
auto newEnv = [&] () -> std::shared_ptr<const StaticEnv> {
auto newEnv = std::make_shared<StaticEnv>(nullptr, env.get(), attrs.size());
Displacement displ = 0;
for (auto & i : attrs)
newEnv->vars.emplace_back(i.first, i.second.displ = displ++);
Displacement displ = 0;
for (auto & i : attrs)
newEnv->vars.emplace_back(i.first, i.second.displ = displ++);
return newEnv;
}();
// No need to sort newEnv since attrs is in sorted order.
auto inheritFromEnv = bindInheritSources(es, newEnv);
for (auto & i : attrs)
i.second.e->bindVars(es, i.second.inherited ? env : newEnv);
i.second.e->bindVars(es, i.second.chooseByKind(newEnv, env, inheritFromEnv));
for (auto & i : dynamicAttrs) {
i.nameExpr->bindVars(es, newEnv);
@ -351,8 +406,10 @@ void ExprAttrs::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv>
}
}
else {
auto inheritFromEnv = bindInheritSources(es, env);
for (auto & i : attrs)
i.second.e->bindVars(es, env);
i.second.e->bindVars(es, i.second.chooseByKind(env, env, inheritFromEnv));
for (auto & i : dynamicAttrs) {
i.nameExpr->bindVars(es, env);
@ -409,16 +466,20 @@ void ExprCall::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> &
void ExprLet::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> & env)
{
auto newEnv = std::make_shared<StaticEnv>(nullptr, env.get(), attrs->attrs.size());
auto newEnv = [&] () -> std::shared_ptr<const StaticEnv> {
auto newEnv = std::make_shared<StaticEnv>(nullptr, env.get(), attrs->attrs.size());
Displacement displ = 0;
for (auto & i : attrs->attrs)
newEnv->vars.emplace_back(i.first, i.second.displ = displ++);
Displacement displ = 0;
for (auto & i : attrs->attrs)
newEnv->vars.emplace_back(i.first, i.second.displ = displ++);
return newEnv;
}();
// No need to sort newEnv since attrs->attrs is in sorted order.
auto inheritFromEnv = attrs->bindInheritSources(es, newEnv);
for (auto & i : attrs->attrs)
i.second.e->bindVars(es, i.second.inherited ? env : newEnv);
i.second.e->bindVars(es, i.second.chooseByKind(newEnv, env, inheritFromEnv));
if (es.debugRepl)
es.exprEnvs.insert(std::make_pair(this, newEnv));

52
src/libexpr/nixexpr.hh
View File

@ -135,6 +135,23 @@ struct ExprVar : Expr
COMMON_METHODS
};
/**
* A pseudo-expression for the purpose of evaluating the `from` expression in `inherit (from)` syntax.
* Unlike normal variable references, the displacement is set during parsing, and always refers to
* `ExprAttrs::inheritFromExprs` (by itself or in `ExprLet`), whose values are put into their own `Env`.
*/
struct ExprInheritFrom : ExprVar
{
ExprInheritFrom(PosIdx pos, Displacement displ): ExprVar(pos, {})
{
this->level = 0;
this->displ = displ;
this->fromWith = nullptr;
}
void bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> & env);
};
struct ExprSelect : Expr
{
PosIdx pos;
@ -160,16 +177,40 @@ struct ExprAttrs : Expr
bool recursive;
PosIdx pos;
struct AttrDef {
bool inherited;
enum class Kind {
/** `attr = expr;` */
Plain,
/** `inherit attr1 attrn;` */
Inherited,
/** `inherit (expr) attr1 attrn;` */
InheritedFrom,
};
Kind kind;
Expr * e;
PosIdx pos;
Displacement displ; // displacement
AttrDef(Expr * e, const PosIdx & pos, bool inherited=false)
: inherited(inherited), e(e), pos(pos) { };
AttrDef(Expr * e, const PosIdx & pos, Kind kind = Kind::Plain)
: kind(kind), e(e), pos(pos) { };
AttrDef() { };
template<typename T>
const T & chooseByKind(const T & plain, const T & inherited, const T & inheritedFrom) const
{
switch (kind) {
case Kind::Plain:
return plain;
case Kind::Inherited:
return inherited;
default:
case Kind::InheritedFrom:
return inheritedFrom;
}
}
};
typedef std::map<Symbol, AttrDef> AttrDefs;
AttrDefs attrs;
std::unique_ptr<std::vector<Expr *>> inheritFromExprs;
struct DynamicAttrDef {
Expr * nameExpr, * valueExpr;
PosIdx pos;
@ -182,6 +223,11 @@ struct ExprAttrs : Expr
ExprAttrs() : recursive(false) { };
PosIdx getPos() const override { return pos; }
COMMON_METHODS
std::shared_ptr<const StaticEnv> bindInheritSources(
EvalState & es, const std::shared_ptr<const StaticEnv> & env);
Env * buildInheritFromEnv(EvalState & state, Env & up);
void showBindings(const SymbolTable & symbols, std::ostream & str) const;
};
struct ExprList : Expr

13
src/libexpr/parser-state.hh
View File

@ -89,7 +89,7 @@ inline void ParserState::addAttr(ExprAttrs * attrs, AttrPath && attrPath, Expr *
if (i->symbol) {
ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol);
if (j != attrs->attrs.end()) {
if (!j->second.inherited) {
if (j->second.kind != ExprAttrs::AttrDef::Kind::Inherited) {
ExprAttrs * attrs2 = dynamic_cast<ExprAttrs *>(j->second.e);
if (!attrs2) dupAttr(attrPath, pos, j->second.pos);
attrs = attrs2;
@ -118,13 +118,24 @@ inline void ParserState::addAttr(ExprAttrs * attrs, AttrPath && attrPath, Expr *
auto ae = dynamic_cast<ExprAttrs *>(e);
auto jAttrs = dynamic_cast<ExprAttrs *>(j->second.e);
if (jAttrs && ae) {
if (ae->inheritFromExprs && !jAttrs->inheritFromExprs)
jAttrs->inheritFromExprs = std::make_unique<std::vector<Expr *>>();
for (auto & ad : ae->attrs) {
auto j2 = jAttrs->attrs.find(ad.first);
if (j2 != jAttrs->attrs.end()) // Attr already defined in iAttrs, error.
dupAttr(ad.first, j2->second.pos, ad.second.pos);
jAttrs->attrs.emplace(ad.first, ad.second);
if (ad.second.kind == ExprAttrs::AttrDef::Kind::InheritedFrom) {
auto & sel = dynamic_cast<ExprSelect &>(*ad.second.e);
auto & from = dynamic_cast<ExprInheritFrom &>(*sel.e);
from.displ += jAttrs->inheritFromExprs->size();
}
}
jAttrs->dynamicAttrs.insert(jAttrs->dynamicAttrs.end(), ae->dynamicAttrs.begin(), ae->dynamicAttrs.end());
if (ae->inheritFromExprs) {
jAttrs->inheritFromExprs->insert(jAttrs->inheritFromExprs->end(),
ae->inheritFromExprs->begin(), ae->inheritFromExprs->end());
}
} else {
dupAttr(attrPath, pos, j->second.pos);
}

16
src/libexpr/parser.y
View File

@ -313,17 +313,27 @@ binds
if ($$->attrs.find(i.symbol) != $$->attrs.end())
state->dupAttr(i.symbol, state->at(@3), $$->attrs[i.symbol].pos);
auto pos = state->at(@3);
$$->attrs.emplace(i.symbol, ExprAttrs::AttrDef(new ExprVar(CUR_POS, i.symbol), pos, true));
$$->attrs.emplace(
i.symbol,
ExprAttrs::AttrDef(new ExprVar(CUR_POS, i.symbol), pos, ExprAttrs::AttrDef::Kind::Inherited));
}
delete $3;
}
| binds INHERIT '(' expr ')' attrs ';'
{ $$ = $1;
/* !!! Should ensure sharing of the expression in $4. */
if (!$$->inheritFromExprs)
$$->inheritFromExprs = std::make_unique<std::vector<Expr *>>();
$$->inheritFromExprs->push_back($4);
auto from = new nix::ExprInheritFrom(state->at(@4), $$->inheritFromExprs->size() - 1);
for (auto & i : *$6) {
if ($$->attrs.find(i.symbol) != $$->attrs.end())
state->dupAttr(i.symbol, state->at(@6), $$->attrs[i.symbol].pos);
$$->attrs.emplace(i.symbol, ExprAttrs::AttrDef(new ExprSelect(CUR_POS, $4, i.symbol), state->at(@6)));
$$->attrs.emplace(
i.symbol,
ExprAttrs::AttrDef(
new ExprSelect(CUR_POS, from, i.symbol),
state->at(@6),
ExprAttrs::AttrDef::Kind::InheritedFrom));
}
delete $6;
}

1
tests/functional/lang/eval-okay-inherit-from.err.exp Normal file
View File

@ -0,0 +1 @@
trace: used

1
tests/functional/lang/eval-okay-inherit-from.exp Normal file
View File

@ -0,0 +1 @@
[ 1 2 { __overrides = { y = { d = [ ]; }; }; c = [ ]; d = 4; x = { c = [ ]; }; y = «repeated»; } { inner = { c = 3; d = 4; }; } ]

16
tests/functional/lang/eval-okay-inherit-from.nix Normal file
View File

@ -0,0 +1,16 @@
let
inherit (builtins.trace "used" { a = 1; b = 2; }) a b;
x.c = 3;
y.d = 4;
merged = {
inner = {
inherit (y) d;
};
inner = {
inherit (x) c;
};
};
in
[ a b rec { x.c = []; inherit (x) c; inherit (y) d; __overrides.y.d = []; } merged ]

1
tests/functional/lang/parse-okay-inherits.exp Normal file
View File

@ -0,0 +1 @@
(let b = 2; c = { }; in { inherit b; inherit (c) d e; a = 1; f = 3; })

9
tests/functional/lang/parse-okay-inherits.nix Normal file
View File

@ -0,0 +1,9 @@
let
c = {};
b = 2;
in {
a = 1;
inherit b;
inherit (c) d e;
f = 3;
}

2
tests/functional/lang/parse-okay-subversion.exp
View File

@ -1 +1 @@
({ fetchurl, localServer ? false, httpServer ? false, sslSupport ? false, pythonBindings ? false, javaSwigBindings ? false, javahlBindings ? false, stdenv, openssl ? null, httpd ? null, db4 ? null, expat, swig ? null, j2sdk ? null }: assert (expat != null); assert (localServer -> (db4 != null)); assert (httpServer -> ((httpd != null) && ((httpd).expat == expat))); assert (sslSupport -> ((openssl != null) && (httpServer -> ((httpd).openssl == openssl)))); assert (pythonBindings -> ((swig != null) && (swig).pythonSupport)); assert (javaSwigBindings -> ((swig != null) && (swig).javaSupport)); assert (javahlBindings -> (j2sdk != null)); ((stdenv).mkDerivation { builder = /foo/bar; db4 = (if localServer then db4 else null); inherit expat ; inherit httpServer ; httpd = (if httpServer then httpd else null); j2sdk = (if javaSwigBindings then (swig).j2sdk else (if javahlBindings then j2sdk else null)); inherit javaSwigBindings ; inherit javahlBindings ; inherit localServer ; name = "subversion-1.1.1"; openssl = (if sslSupport then openssl else null); patches = (if javahlBindings then [ (/javahl.patch) ] else [ ]); python = (if pythonBindings then (swig).python else null); inherit pythonBindings ; src = (fetchurl { md5 = "a180c3fe91680389c210c99def54d9e0"; url = "http://subversion.tigris.org/tarballs/subversion-1.1.1.tar.bz2"; }); inherit sslSupport ; swig = (if (pythonBindings || javaSwigBindings) then swig else null); }))
({ fetchurl, localServer ? false, httpServer ? false, sslSupport ? false, pythonBindings ? false, javaSwigBindings ? false, javahlBindings ? false, stdenv, openssl ? null, httpd ? null, db4 ? null, expat, swig ? null, j2sdk ? null }: assert (expat != null); assert (localServer -> (db4 != null)); assert (httpServer -> ((httpd != null) && ((httpd).expat == expat))); assert (sslSupport -> ((openssl != null) && (httpServer -> ((httpd).openssl == openssl)))); assert (pythonBindings -> ((swig != null) && (swig).pythonSupport)); assert (javaSwigBindings -> ((swig != null) && (swig).javaSupport)); assert (javahlBindings -> (j2sdk != null)); ((stdenv).mkDerivation { inherit expat httpServer javaSwigBindings javahlBindings localServer pythonBindings sslSupport; builder = /foo/bar; db4 = (if localServer then db4 else null); httpd = (if httpServer then httpd else null); j2sdk = (if javaSwigBindings then (swig).j2sdk else (if javahlBindings then j2sdk else null)); name = "subversion-1.1.1"; openssl = (if sslSupport then openssl else null); patches = (if javahlBindings then [ (/javahl.patch) ] else [ ]); python = (if pythonBindings then (swig).python else null); src = (fetchurl { md5 = "a180c3fe91680389c210c99def54d9e0"; url = "http://subversion.tigris.org/tarballs/subversion-1.1.1.tar.bz2"; }); swig = (if (pythonBindings || javaSwigBindings) then swig else null); }))