From ab35cbd675610a52513f746051e98d0a50815bc1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 21 Feb 2020 18:31:16 +0100 Subject: [PATCH] StaticEnv: Use std::vector instead of std::map --- src/libexpr/eval.cc | 6 +++--- src/libexpr/nixexpr.cc | 25 +++++++++++++++++-------- src/libexpr/nixexpr.hh | 25 +++++++++++++++++++++---- src/libexpr/parser.y | 8 ++++---- src/libexpr/primops.cc | 10 ++++++++-- src/nix/repl.cc | 3 ++- 6 files changed, 55 insertions(+), 22 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index db1e7e56d..062d190b6 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -583,7 +583,7 @@ Value * EvalState::addConstant(const string & name, Value & v) { Value * v2 = allocValue(); *v2 = v; - staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl; + staticBaseEnv.vars.emplace_back(symbols.create(name), baseEnvDispl); baseEnv.values[baseEnvDispl++] = v2; string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name; baseEnv.values[0]->attrs->push_back(Attr(symbols.create(name2), v2)); @@ -609,7 +609,7 @@ Value * EvalState::addPrimOp(const string & name, Value * v = allocValue(); v->mkPrimOp(new PrimOp { .fun = primOp, .arity = arity, .name = sym }); - staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl; + staticBaseEnv.vars.emplace_back(symbols.create(name), baseEnvDispl); baseEnv.values[baseEnvDispl++] = v; baseEnv.values[0]->attrs->push_back(Attr(sym, v)); return v; @@ -635,7 +635,7 @@ Value * EvalState::addPrimOp(PrimOp && primOp) Value * v = allocValue(); v->mkPrimOp(new PrimOp(std::move(primOp))); - staticBaseEnv.vars[envName] = baseEnvDispl; + staticBaseEnv.vars.emplace_back(envName, baseEnvDispl); baseEnv.values[baseEnvDispl++] = v; baseEnv.values[0]->attrs->push_back(Attr(primOp.name, v)); return v; diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 0d0f3e469..95a353a40 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -269,7 +269,7 @@ void ExprVar::bindVars(const StaticEnv & env) if (curEnv->isWith) { if (withLevel == -1) withLevel = level; } else { - StaticEnv::Vars::const_iterator i = curEnv->vars.find(name); + auto i = curEnv->find(name); if (i != curEnv->vars.end()) { fromWith = false; this->level = level; @@ -311,14 +311,16 @@ void ExprOpHasAttr::bindVars(const StaticEnv & env) void ExprAttrs::bindVars(const StaticEnv & env) { const StaticEnv * dynamicEnv = &env; - StaticEnv newEnv(false, &env); + StaticEnv newEnv(false, &env, recursive ? attrs.size() : 0); if (recursive) { dynamicEnv = &newEnv; unsigned int displ = 0; for (auto & i : attrs) - newEnv.vars[i.first] = i.second.displ = displ++; + newEnv.vars.emplace_back(i.first, i.second.displ = displ++); + + // No need to sort newEnv since attrs is in sorted order. for (auto & i : attrs) i.second.e->bindVars(i.second.inherited ? env : newEnv); @@ -342,15 +344,20 @@ void ExprList::bindVars(const StaticEnv & env) void ExprLambda::bindVars(const StaticEnv & env) { - StaticEnv newEnv(false, &env); + StaticEnv newEnv( + false, &env, + (hasFormals() ? formals->formals.size() : 0) + + (arg.empty() ? 0 : 1)); unsigned int displ = 0; - if (!arg.empty()) newEnv.vars[arg] = displ++; + if (!arg.empty()) newEnv.vars.emplace_back(arg, displ++); if (hasFormals()) { for (auto & i : formals->formals) - newEnv.vars[i.name] = displ++; + newEnv.vars.emplace_back(i.name, displ++); + + newEnv.sort(); for (auto & i : formals->formals) if (i.def) i.def->bindVars(newEnv); @@ -361,11 +368,13 @@ void ExprLambda::bindVars(const StaticEnv & env) void ExprLet::bindVars(const StaticEnv & env) { - StaticEnv newEnv(false, &env); + StaticEnv newEnv(false, &env, attrs->attrs.size()); unsigned int displ = 0; for (auto & i : attrs->attrs) - newEnv.vars[i.first] = i.second.displ = displ++; + newEnv.vars.emplace_back(i.first, i.second.displ = displ++); + + // No need to sort newEnv since attrs->attrs is in sorted order. for (auto & i : attrs->attrs) i.second.e->bindVars(i.second.inherited ? env : newEnv); diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 851e875bd..3bfa5e0d3 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -4,8 +4,6 @@ #include "symbol-table.hh" #include "error.hh" -#include - namespace nix { @@ -342,9 +340,28 @@ struct StaticEnv { bool isWith; const StaticEnv * up; - typedef std::map Vars; + + // Note: these must be in sorted order. + typedef std::vector> Vars; Vars vars; - StaticEnv(bool isWith, const StaticEnv * up) : isWith(isWith), up(up) { }; + + StaticEnv(bool isWith, const StaticEnv * up, size_t expectedSize = 0) : isWith(isWith), up(up) { + vars.reserve(expectedSize); + }; + + void sort() + { + std::sort(vars.begin(), vars.end(), + [](const Vars::value_type & a, const Vars::value_type & b) { return a.first < b.first; }); + } + + Vars::const_iterator find(const Symbol & name) const + { + Vars::value_type key(name, 0); + auto i = std::lower_bound(vars.begin(), vars.end(), key); + if (i != vars.end() && i->first == name) return i; + return vars.end(); + } }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 813ff2fc3..5d0f05206 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -126,14 +126,14 @@ static void addAttr(ExprAttrs * attrs, AttrPath & attrPath, 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[ad.first] = ad.second; + jAttrs->attrs.emplace(ad.first, ad.second); } } else { dupAttr(attrPath, pos, j->second.pos); } } else { // This attr path is not defined. Let's create it. - attrs->attrs[i->symbol] = ExprAttrs::AttrDef(e, pos); + attrs->attrs.emplace(i->symbol, ExprAttrs::AttrDef(e, pos)); e->setName(i->symbol); } } else { @@ -483,7 +483,7 @@ binds if ($$->attrs.find(i.symbol) != $$->attrs.end()) dupAttr(i.symbol, makeCurPos(@3, data), $$->attrs[i.symbol].pos); Pos pos = makeCurPos(@3, data); - $$->attrs[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, true)); } } | binds INHERIT '(' expr ')' attrs ';' @@ -492,7 +492,7 @@ binds for (auto & i : *$6) { if ($$->attrs.find(i.symbol) != $$->attrs.end()) dupAttr(i.symbol, makeCurPos(@6, data), $$->attrs[i.symbol].pos); - $$->attrs[i.symbol] = ExprAttrs::AttrDef(new ExprSelect(CUR_POS, $4, i.symbol), makeCurPos(@6, data)); + $$->attrs.emplace(i.symbol, ExprAttrs::AttrDef(new ExprSelect(CUR_POS, $4, i.symbol), makeCurPos(@6, data))); } } | { $$ = new ExprAttrs(makeCurPos(@0, data)); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 6b3cafec8..8a573f35c 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -184,14 +184,17 @@ static void import(EvalState & state, const Pos & pos, Value & vPath, Value * vS Env * env = &state.allocEnv(vScope->attrs->size()); env->up = &state.baseEnv; - StaticEnv staticEnv(false, &state.staticBaseEnv); + StaticEnv staticEnv(false, &state.staticBaseEnv, vScope->attrs->size()); unsigned int displ = 0; for (auto & attr : *vScope->attrs) { - staticEnv.vars[attr.name] = displ; + staticEnv.vars.emplace_back(attr.name, displ); env->values[displ++] = attr.value; } + // No need to call staticEnv.sort(), because + // args[0]->attrs is already sorted. + printTalkative("evaluating file '%1%'", realPath); Expr * e = state.parseExprFromFile(resolveExprPath(realPath), staticEnv); @@ -3726,6 +3729,7 @@ void EvalState::createBaseEnv() /* Add a wrapper around the derivation primop that computes the `drvPath' and `outPath' attributes lazily. */ + staticBaseEnv.sort(); sDerivationNix = symbols.create("//builtin/derivation.nix"); eval(parse( #include "primops/derivation.nix.gen.hh" @@ -3735,6 +3739,8 @@ void EvalState::createBaseEnv() /* Now that we've added all primops, sort the `builtins' set, because attribute lookups expect it to be sorted. */ baseEnv.values[0]->attrs->sort(); + + staticBaseEnv.sort(); } diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 9c0d22438..07bf32a0a 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -644,7 +644,8 @@ void NixRepl::addVarToScope(const Symbol & name, Value & v) { if (displ >= envSize) throw Error("environment full; cannot add more variables"); - staticEnv.vars[name] = displ; + staticEnv.vars.emplace_back(name, displ); + staticEnv.sort(); env->values[displ++] = &v; varNames.insert((string) name); }