* Keep attribute sets in sorted order to speed up attribute lookups.

* Simplify the representation of attributes in the AST.
* Change the behaviour of listToAttrs() in case of duplicate names.
This commit is contained in:
Eelco Dolstra 2010-10-24 19:52:33 +00:00
parent 2dc6d50941
commit e0b7fb8f27
12 changed files with 186 additions and 148 deletions

View file

@ -20,14 +20,17 @@ bool parseOptionArg(const string & arg, Strings::iterator & i,
if (i == argsEnd) throw error;
string value = *i++;
/* !!! check for duplicates! */
Value * v = state.allocValue();
autoArgs[state.symbols.create(name)].value = v;
autoArgs.push_back(Attr(state.symbols.create(name), v));
if (arg == "--arg")
state.mkThunk_(*v, parseExprFromString(state, value, absPath(".")));
else
mkString(*v, value);
autoArgs.sort(); // !!! inefficient
return true;
}

View file

@ -34,18 +34,16 @@ namespace nix {
Bindings::iterator Bindings::find(const Symbol & name)
{
iterator i = begin();
for ( ; i != end() && i->name != name; ++i) ;
return i;
Attr key(name, 0);
iterator i = lower_bound(begin(), end(), key);
if (i != end() && i->name == name) return i;
return end();
}
Attr & Bindings::operator [] (const Symbol & name)
void Bindings::sort()
{
iterator i = find(name);
if (i != end()) return *i;
push_back(Attr(name, 0));
return back();
std::sort(begin(), end());
}
@ -178,12 +176,12 @@ void EvalState::addPrimOp(const string & name,
{
Value * v = allocValue();
string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name;
Symbol sym = symbols.create(name);
Symbol sym = symbols.create(name2);
v->type = tPrimOp;
v->primOp = NEW PrimOp(primOp, arity, sym);
staticBaseEnv.vars[sym] = baseEnvDispl;
staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl;
baseEnv.values[baseEnvDispl++] = v;
baseEnv.values[0]->attrs->push_back(Attr(symbols.create(name2), v));
baseEnv.values[0]->attrs->push_back(Attr(sym, v));
}
@ -506,30 +504,36 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v)
void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
{
state.mkAttrs(v);
state.mkAttrs(v); // !!! reserve size
if (recursive) {
/* Create a new environment that contains the attributes in
this `rec'. */
Env & env2(state.allocEnv(attrs.size() + inherited.size()));
Env & env2(state.allocEnv(attrs.size()));
env2.up = &env;
unsigned int displ = 0;
AttrDefs::iterator overrides = attrs.find(state.sOverrides);
bool hasOverrides = overrides != attrs.end();
/* The recursive attributes are evaluated in the new
environment. */
foreach (Attrs::iterator, i, attrs) {
Value * vAttr = state.maybeThunk(env2, i->second.first);
environment, while the inherited attributes are evaluated
in the original environment. */
unsigned int displ = 0;
foreach (AttrDefs::iterator, i, attrs)
if (i->second.inherited) {
/* !!! handle overrides? */
Value * vAttr = state.lookupVar(&env, i->second.var);
env2.values[displ++] = vAttr;
v.attrs->push_back(nix::Attr(i->first, vAttr, &i->second.second));
}
/* The inherited attributes, on the other hand, are
evaluated in the original environment. */
foreach (list<Inherited>::iterator, i, inherited) {
Value * vAttr = state.lookupVar(&env, i->first);
v.attrs->push_back(Attr(i->first, vAttr, &i->second.pos));
} else {
Value * vAttr;
if (hasOverrides) {
vAttr = state.allocValue();
mkThunk(*vAttr, env2, i->second.e);
} else
vAttr = state.maybeThunk(env2, i->second.e);
env2.values[displ++] = vAttr;
v.attrs->push_back(nix::Attr(i->first.name, vAttr, &i->second));
v.attrs->push_back(Attr(i->first, vAttr, &i->second.pos));
}
/* If the rec contains an attribute called `__overrides', then
@ -540,30 +544,27 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
still reference the original value, because that value has
been substituted into the bodies of the other attributes.
Hence we need __overrides.) */
Bindings::iterator overrides = v.attrs->find(state.sOverrides);
if (overrides != v.attrs->end()) {
state.forceAttrs(*overrides->value);
foreach (Bindings::iterator, i, *overrides->value->attrs) {
nix::Attr & a = (*v.attrs)[i->name];
if (a.value)
env2.values[displs[i->name]] = i->value;
a = *i;
if (hasOverrides) {
Value * vOverrides = (*v.attrs)[overrides->second.displ].value;
state.forceAttrs(*vOverrides);
foreach (Bindings::iterator, i, *vOverrides->attrs) {
AttrDefs::iterator j = attrs.find(i->name);
if (j != attrs.end()) {
(*v.attrs)[j->second.displ] = *i;
env2.values[j->second.displ] = i->value;
} else
v.attrs->push_back(*i);
}
v.attrs->sort();
}
}
else {
foreach (Attrs::iterator, i, attrs) {
nix::Attr & a = (*v.attrs)[i->first];
a.value = state.maybeThunk(env, i->second.first);
a.pos = &i->second.second;
}
foreach (list<Inherited>::iterator, i, inherited) {
nix::Attr & a = (*v.attrs)[i->first.name];
a.value = state.lookupVar(&env, i->first);
a.pos = &i->second;
}
foreach (AttrDefs::iterator, i, attrs)
if (i->second.inherited)
v.attrs->push_back(Attr(i->first, state.lookupVar(&env, i->second.var), &i->second.pos));
else
v.attrs->push_back(Attr(i->first, state.maybeThunk(env, i->second.e), &i->second.pos));
}
}
@ -572,20 +573,18 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
{
/* Create a new environment that contains the attributes in this
`let'. */
Env & env2(state.allocEnv(attrs->attrs.size() + attrs->inherited.size()));
Env & env2(state.allocEnv(attrs->attrs.size()));
env2.up = &env;
unsigned int displ = 0;
/* The recursive attributes are evaluated in the new
/* The recursive attributes are evaluated in the new environment,
while the inherited attributes are evaluated in the original
environment. */
foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs)
env2.values[displ++] = state.maybeThunk(env2, i->second.first);
/* The inherited attributes, on the other hand, are evaluated in
the original environment. */
foreach (list<ExprAttrs::Inherited>::iterator, i, attrs->inherited)
env2.values[displ++] = state.lookupVar(&env, i->first);
unsigned int displ = 0;
foreach (ExprAttrs::AttrDefs::iterator, i, attrs->attrs)
if (i->second.inherited)
env2.values[displ++] = state.lookupVar(&env, i->second.var);
else
env2.values[displ++] = state.maybeThunk(env2, i->second.e);
state.eval(env2, body, v);
}
@ -764,11 +763,13 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res)
foreach (Formals::Formals_::iterator, i, fun.lambda.fun->formals->formals) {
Bindings::iterator j = args.find(i->name);
if (j != args.end())
(*actualArgs.attrs)[i->name] = *j;
actualArgs.attrs->push_back(*j);
else if (!i->def)
throwTypeError("cannot auto-call a function that has an argument without a default value (`%1%')", i->name);
}
actualArgs.attrs->sort();
callFunction(fun, actualArgs, res);
}
@ -851,11 +852,27 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
if (v1.attrs->size() == 0) { v = v2; return; }
if (v2.attrs->size() == 0) { v = v1; return; }
state.cloneAttrs(v1, v);
state.mkAttrs(v);
/* !!! fix */
foreach (Bindings::iterator, i, *v2.attrs)
(*v.attrs)[i->name] = *i;
/* Merge the attribute sets, preferring values from the second
set. Make sure to keep the resulting vector in sorted
order. */
Bindings::iterator i = v1.attrs->begin();
Bindings::iterator j = v2.attrs->begin();
while (i != v1.attrs->end() && j != v2.attrs->end()) {
if (i->name == j->name) {
v.attrs->push_back(*j);
++i; ++j;
}
else if (i->name < j->name)
v.attrs->push_back(*i++);
else
v.attrs->push_back(*j++);
}
while (i != v1.attrs->end()) v.attrs->push_back(*i++);
while (j != v2.attrs->end()) v.attrs->push_back(*j++);
state.nrOpUpdateValuesCopied += v.attrs->size();
}

View file

@ -34,7 +34,7 @@ class Bindings : public BindingsBase
{
public:
iterator find(const Symbol & name);
Attr & operator [] (const Symbol & name);
void sort();
};
@ -142,6 +142,10 @@ struct Attr
Attr(Symbol name, Value * value, Pos * pos = &noPos)
: name(name), value(value), pos(pos) { };
Attr() : pos(&noPos) { };
bool operator < (const Attr & a) const
{
return name < a.name;
}
};

View file

@ -168,7 +168,7 @@ static void getDerivations(EvalState & state, Value & vIn,
foreach (SortedSymbols::iterator, i, attrs) {
startNest(nest, lvlDebug, format("evaluating attribute `%1%'") % i->first);
string pathPrefix2 = addToPath(pathPrefix, i->first);
Value & v2(*(*v.attrs)[i->second].value);
Value & v2(*v.attrs->find(i->second)->value);
if (combineChannels)
getDerivations(state, v2, pathPrefix2, autoArgs, drvs, done);
else if (getDerivation(state, v2, pathPrefix2, drvs, done)) {

View file

@ -55,10 +55,11 @@ void ExprAttrs::show(std::ostream & str)
{
if (recursive) str << "rec ";
str << "{ ";
foreach (list<Inherited>::iterator, i, inherited)
str << "inherit " << i->first.name << "; ";
foreach (Attrs::iterator, i, attrs)
str << i->first << " = " << *i->second.first << "; ";
foreach (AttrDefs::iterator, i, attrs)
if (i->second.inherited)
str << "inherit " << i->first << " " << "; ";
else
str << i->first << " = " << *i->second.e << "; ";
str << "}";
}
@ -91,10 +92,11 @@ void ExprLambda::show(std::ostream & str)
void ExprLet::show(std::ostream & str)
{
str << "let ";
foreach (list<ExprAttrs::Inherited>::iterator, i, attrs->inherited)
str << "inherit " << i->first.name << "; ";
foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs)
str << i->first << " = " << *i->second.first << "; ";
foreach (ExprAttrs::AttrDefs::iterator, i, attrs->attrs)
if (i->second.inherited)
str << "inherit " << i->first << "; ";
else
str << i->first << " = " << *i->second.e << "; ";
str << "in " << *body;
}
@ -211,26 +213,18 @@ void ExprAttrs::bindVars(const StaticEnv & env)
StaticEnv newEnv(false, &env);
unsigned int displ = 0;
foreach (AttrDefs::iterator, i, attrs)
newEnv.vars[i->first] = i->second.displ = displ++;
foreach (ExprAttrs::Attrs::iterator, i, attrs)
displs[i->first] = newEnv.vars[i->first] = displ++;
foreach (list<Inherited>::iterator, i, inherited) {
displs[i->first.name] = newEnv.vars[i->first.name] = displ++;
i->first.bind(env);
foreach (AttrDefs::iterator, i, attrs)
if (i->second.inherited) i->second.var.bind(env);
else i->second.e->bindVars(newEnv);
}
foreach (ExprAttrs::Attrs::iterator, i, attrs)
i->second.first->bindVars(newEnv);
}
else {
foreach (ExprAttrs::Attrs::iterator, i, attrs)
i->second.first->bindVars(env);
foreach (list<Inherited>::iterator, i, inherited)
i->first.bind(env);
}
else
foreach (AttrDefs::iterator, i, attrs)
if (i->second.inherited) i->second.var.bind(env);
else i->second.e->bindVars(env);
}
void ExprList::bindVars(const StaticEnv & env)
@ -263,17 +257,12 @@ void ExprLet::bindVars(const StaticEnv & env)
StaticEnv newEnv(false, &env);
unsigned int displ = 0;
foreach (ExprAttrs::AttrDefs::iterator, i, attrs->attrs)
newEnv.vars[i->first] = i->second.displ = displ++;
foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs)
newEnv.vars[i->first] = displ++;
foreach (list<ExprAttrs::Inherited>::iterator, i, attrs->inherited) {
newEnv.vars[i->first.name] = displ++;
i->first.bind(env);
}
foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs)
i->second.first->bindVars(newEnv);
foreach (ExprAttrs::AttrDefs::iterator, i, attrs->attrs)
if (i->second.inherited) i->second.var.bind(env);
else i->second.e->bindVars(newEnv);
body->bindVars(newEnv);
}

View file

@ -101,6 +101,7 @@ struct VarRef
unsigned int level;
unsigned int displ;
VarRef() { };
VarRef(const Symbol & name) : name(name) { };
void bind(const StaticEnv & env);
};
@ -131,13 +132,18 @@ struct ExprOpHasAttr : Expr
struct ExprAttrs : Expr
{
bool recursive;
typedef std::pair<Expr *, Pos> Attr;
typedef std::pair<VarRef, Pos> Inherited;
typedef std::map<Symbol, Attr> Attrs;
Attrs attrs;
list<Inherited> inherited;
std::map<Symbol, Pos> attrNames; // used during parsing
std::map<Symbol, unsigned int> displs;
struct AttrDef {
bool inherited;
Expr * e; // if not inherited
VarRef var; // if inherited
Pos pos;
unsigned int displ; // displacement
AttrDef(Expr * e, const Pos & pos) : inherited(false), e(e), pos(pos) { };
AttrDef(const Symbol & name, const Pos & pos) : inherited(true), var(name), pos(pos) { };
AttrDef() { };
};
typedef std::map<Symbol, AttrDef> AttrDefs;
AttrDefs attrs;
ExprAttrs() : recursive(false) { };
COMMON_METHODS
};

View file

@ -93,20 +93,20 @@ static void addAttr(ExprAttrs * attrs, const vector<Symbol> & attrPath,
unsigned int n = 0;
foreach (vector<Symbol>::const_iterator, i, attrPath) {
n++;
ExprAttrs::Attrs::iterator j = attrs->attrs.find(*i);
ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(*i);
if (j != attrs->attrs.end()) {
ExprAttrs * attrs2 = dynamic_cast<ExprAttrs *>(j->second.first);
if (!attrs2 || n == attrPath.size()) dupAttr(attrPath, pos, j->second.second);
if (!j->second.inherited) {
ExprAttrs * attrs2 = dynamic_cast<ExprAttrs *>(j->second.e);
if (!attrs2 || n == attrPath.size()) dupAttr(attrPath, pos, j->second.pos);
attrs = attrs2;
} else
dupAttr(attrPath, pos, j->second.pos);
} else {
if (attrs->attrNames.find(*i) != attrs->attrNames.end())
dupAttr(attrPath, pos, attrs->attrNames[*i]);
attrs->attrNames[*i] = pos;
if (n == attrPath.size())
attrs->attrs[*i] = ExprAttrs::Attr(e, pos);
attrs->attrs[*i] = ExprAttrs::AttrDef(e, pos);
else {
ExprAttrs * nested = new ExprAttrs;
attrs->attrs[*i] = ExprAttrs::Attr(nested, pos);
attrs->attrs[*i] = ExprAttrs::AttrDef(nested, pos);
attrs = nested;
}
}
@ -383,21 +383,19 @@ binds
| binds INHERIT ids ';'
{ $$ = $1;
foreach (vector<Symbol>::iterator, i, *$3) {
if ($$->attrNames.find(*i) != $$->attrNames.end())
dupAttr(*i, makeCurPos(@3, data), $$->attrNames[*i]);
if ($$->attrs.find(*i) != $$->attrs.end())
dupAttr(*i, makeCurPos(@3, data), $$->attrs[*i].pos);
Pos pos = makeCurPos(@3, data);
$$->inherited.push_back(ExprAttrs::Inherited(*i, pos));
$$->attrNames[*i] = pos;
$$->attrs[*i] = ExprAttrs::AttrDef(*i, pos);
}
}
| binds INHERIT '(' expr ')' ids ';'
{ $$ = $1;
/* !!! Should ensure sharing of the expression in $4. */
foreach (vector<Symbol>::iterator, i, *$6) {
if ($$->attrNames.find(*i) != $$->attrNames.end())
dupAttr(*i, makeCurPos(@6, data), $$->attrNames[*i]);
$$->attrs[*i] = ExprAttrs::Attr(new ExprSelect($4, *i), makeCurPos(@6, data));
$$->attrNames[*i] = makeCurPos(@6, data);
if ($$->attrs.find(*i) != $$->attrs.end())
dupAttr(*i, makeCurPos(@6, data), $$->attrs[*i].pos);
$$->attrs[*i] = ExprAttrs::AttrDef(new ExprSelect($4, *i), makeCurPos(@6, data));
}}
| { $$ = new ExprAttrs; }

View file

@ -209,14 +209,13 @@ static void prim_tryEval(EvalState & state, Value * * args, Value & v)
state.mkAttrs(v);
try {
state.forceValue(*args[0]);
printMsg(lvlError, format("%1%") % *args[0]);
(*v.attrs)[state.symbols.create("value")].value = args[0];
v.attrs->push_back(Attr(state.symbols.create("value"), args[0]));
mkBool(*state.allocAttr(v, state.symbols.create("success")), true);
printMsg(lvlError, format("%1%") % v);
} catch (AssertionError & e) {
mkBool(*state.allocAttr(v, state.symbols.create("value")), false);
mkBool(*state.allocAttr(v, state.symbols.create("success")), false);
}
v.attrs->sort();
}
@ -488,6 +487,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
state.mkAttrs(v);
mkString(*state.allocAttr(v, state.sOutPath), outPath, singleton<PathSet>(drvPath));
mkString(*state.allocAttr(v, state.sDrvPath), drvPath, singleton<PathSet>("=" + drvPath));
v.attrs->sort();
}
@ -742,7 +742,9 @@ static void prim_removeAttrs(EvalState & state, Value * * args, Value & v)
names.insert(state.symbols.create(args[1]->list.elems[i]->string.s));
}
/* Copy all attributes not in that set. */
/* Copy all attributes not in that set. Note that we don't need
to sort v.attrs because it's a subset of an already sorted
vector. */
state.mkAttrs(v);
foreach (Bindings::iterator, i, *args[0]->attrs) {
if (names.find(i->name) == names.end())
@ -761,6 +763,8 @@ static void prim_listToAttrs(EvalState & state, Value * * args, Value & v)
state.mkAttrs(v);
std::set<Symbol> seen;
for (unsigned int i = 0; i < args[0]->list.length; ++i) {
Value & v2(*args[0]->list.elems[i]);
state.forceAttrs(v2);
@ -774,10 +778,15 @@ static void prim_listToAttrs(EvalState & state, Value * * args, Value & v)
if (j2 == v2.attrs->end())
throw TypeError("`value' attribute missing in a call to `listToAttrs'");
Attr & a = (*v.attrs)[state.symbols.create(name)];
a.value = j2->value;
a.pos = j2->pos;
Symbol sym = state.symbols.create(name);
if (seen.find(sym) == seen.end()) {
v.attrs->push_back(Attr(sym, j2->value, j2->pos));
seen.insert(sym);
}
/* !!! Throw an error if `name' already exists? */
}
v.attrs->sort();
}
@ -825,6 +834,8 @@ static void prim_functionArgs(EvalState & state, Value * * args, Value & v)
foreach (Formals::Formals_::iterator, i, args[0]->lambda.fun->formals->formals)
// !!! should optimise booleans (allocate only once)
mkBool(*state.allocAttr(v, i->name), i->def);
v.attrs->sort();
}
@ -1007,6 +1018,7 @@ static void prim_parseDrvName(EvalState & state, Value * * args, Value & v)
state.mkAttrs(v);
mkString(*state.allocAttr(v, state.sName), parsed.name);
mkString(*state.allocAttr(v, state.symbols.create("version")), parsed.version);
v.attrs->sort();
}
@ -1120,6 +1132,10 @@ void EvalState::createBaseEnv()
// Versions
addPrimOp("__parseDrvName", 1, prim_parseDrvName);
addPrimOp("__compareVersions", 2, prim_compareVersions);
/* Now that we've added all primops, sort the `builtins' attribute
set, because attribute lookups expect it to be sorted. */
baseEnv.values[0]->attrs->sort();
}

View file

@ -37,7 +37,7 @@ static void showAttrs(EvalState & state, bool strict, bool location,
names.insert(i->name);
foreach (StringSet::iterator, i, names) {
Attr & a(attrs[state.symbols.create(*i)]);
Attr & a(*attrs.find(state.symbols.create(*i)));
XMLAttrs xmlAttrs;
xmlAttrs["name"] = *i;

View file

@ -132,7 +132,7 @@ static void getAllExprs(EvalState & state,
if (hasSuffix(attrName, ".nix"))
attrName = string(attrName, 0, attrName.size() - 4);
attrs.attrs[state.symbols.create(attrName)] =
ExprAttrs::Attr(parseExprFromFile(state, absPath(path2)), noPos);
ExprAttrs::AttrDef(parseExprFromFile(state, absPath(path2)), noPos);
}
else
/* `path2' is a directory (with no default.nix in it);
@ -154,7 +154,7 @@ static Expr * loadSourceExpr(EvalState & state, const Path & path)
some system-wide directory). */
ExprAttrs * attrs = new ExprAttrs;
attrs->attrs[state.symbols.create("_combineChannels")] =
ExprAttrs::Attr(new ExprList(), noPos);
ExprAttrs::AttrDef(new ExprList(), noPos);
getAllExprs(state, path, *attrs);
return attrs;
}

View file

@ -70,12 +70,13 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
if (drvPath != "")
mkString(*state.allocAttr(v, state.sDrvPath), i->queryDrvPath(state));
state.mkAttrs(*state.allocAttr(v, state.sMeta));
Value & vMeta = *state.allocAttr(v, state.sMeta);
state.mkAttrs(vMeta);
MetaInfo meta = i->queryMetaInfo(state);
foreach (MetaInfo::const_iterator, j, meta) {
Value & v2(*state.allocAttr(*(*v.attrs)[state.sMeta].value, state.symbols.create(j->first)));
Value & v2(*state.allocAttr(vMeta, state.symbols.create(j->first)));
switch (j->second.type) {
case MetaValue::tpInt: mkInt(v2, j->second.intValue); break;
case MetaValue::tpString: mkString(v2, j->second.stringValue); break;
@ -92,6 +93,9 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
}
}
vMeta.attrs->sort();
v.attrs->sort();
/* This is only necessary when installing store paths, e.g.,
`nix-env -i /nix/store/abcd...-foo'. */
store->addTempRoot(i->queryOutPath(state));
@ -118,7 +122,8 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
mkString(*state.allocAttr(args, state.sSystem), thisSystem);
mkString(*state.allocAttr(args, state.symbols.create("manifest")),
manifestFile, singleton<PathSet>(manifestFile));
(*args.attrs)[state.symbols.create("derivations")].value = &manifest;
args.attrs->push_back(Attr(state.symbols.create("derivations"), &manifest));
args.attrs->sort();
mkApp(topLevel, envBuilder, args);
/* Evaluate it. */

View file

@ -7,5 +7,5 @@ let
a = builtins.listToAttrs list;
b = builtins.listToAttrs ( list ++ list );
r = builtins.listToAttrs [ (asi "result" [ a b ]) ( asi "throw" (throw "this should not be thrown")) ];
x = builtins.listToAttrs [ (asi "foo" "bla") (asi "foo" "bar") ];
x = builtins.listToAttrs [ (asi "foo" "bar") (asi "foo" "bla") ];
in concat (map (x: x.a) r.result) + x.foo