From 0b305c534f989dbc3645ff03e070b0e4665fdeb7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 24 Oct 2010 00:41:29 +0000 Subject: [PATCH] * Store attribute sets as a vector instead of a map (i.e. a red-black tree). This saves a lot of memory. The vector should be sorted so that names can be looked up using binary search, but this is not the case yet. (Surprisingly, looking up attributes using linear search doesn't have a big impact on performance.) Memory consumption for $ nix-instantiate /etc/nixos/nixos/tests -A bittorrent.test --readonly-mode on x86_64-linux with GC enabled is now 185 MiB (compared to 946 MiB on the trunk). --- src/libexpr/attr-path.cc | 2 +- src/libexpr/eval.cc | 84 +++++++++++++++++++++---------------- src/libexpr/eval.hh | 18 +++++++- src/libexpr/get-drvs.cc | 34 +++++++-------- src/libexpr/primops.cc | 65 +++++++++++++++------------- src/libexpr/symbol-table.hh | 2 + src/libexpr/value-to-xml.cc | 14 +++---- 7 files changed, 129 insertions(+), 90 deletions(-) diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc index 365b03c0bf..49c08339ae 100644 --- a/src/libexpr/attr-path.cc +++ b/src/libexpr/attr-path.cc @@ -49,7 +49,7 @@ void findAlongAttrPath(EvalState & state, const string & attrPath, Bindings::iterator a = v.attrs->find(state.symbols.create(attr)); if (a == v.attrs->end()) throw Error(format("attribute `%1%' in selection path `%2%' not found") % attr % curPath); - v = *a->second.value; + v = *a->value; } else if (apType == apIndex) { diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index d73f99a156..85394b768d 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -13,7 +13,7 @@ #include #include -#define NEW (UseGC) +#define NEW new (UseGC) #else @@ -30,6 +30,23 @@ namespace nix { + + +Bindings::iterator Bindings::find(const Symbol & name) +{ + iterator i = begin(); + for ( ; i != end() && i->name != name; ++i) ; + return i; +} + + +Attr & Bindings::operator [] (const Symbol & name) +{ + iterator i = find(name); + if (i != end()) return *i; + push_back(Attr(name, 0)); + return back(); +} std::ostream & operator << (std::ostream & str, const Value & v) @@ -62,7 +79,7 @@ std::ostream & operator << (std::ostream & str, const Value & v) typedef std::map Sorted; Sorted sorted; foreach (Bindings::iterator, i, *v.attrs) - sorted[i->first] = i->second.value; + sorted[i->name] = i->value; foreach (Sorted::iterator, i, sorted) str << i->first << " = " << *i->second << "; "; str << "}"; @@ -152,7 +169,7 @@ void EvalState::addConstant(const string & name, Value & v) staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl; baseEnv.values[baseEnvDispl++] = v2; string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name; - (*baseEnv.values[0]->attrs)[symbols.create(name2)].value = v2; + baseEnv.values[0]->attrs->push_back(Attr(symbols.create(name2), v2)); } @@ -166,7 +183,7 @@ void EvalState::addPrimOp(const string & name, v->primOp = NEW PrimOp(primOp, arity, sym); staticBaseEnv.vars[sym] = baseEnvDispl; baseEnv.values[baseEnvDispl++] = v; - (*baseEnv.values[0]->attrs)[symbols.create(name2)].value = v; + baseEnv.values[0]->attrs->push_back(Attr(symbols.create(name2), v)); } @@ -277,7 +294,7 @@ Value * EvalState::lookupVar(Env * env, const VarRef & var) while (1) { Bindings::iterator j = env->values[0]->attrs->find(var.name); if (j != env->values[0]->attrs->end()) - return j->second.value; + return j->value; if (env->prevWith == 0) throwEvalError("undefined variable `%1%'", var.name); for (unsigned int l = env->prevWith; l; --l, env = env->up) ; @@ -305,9 +322,9 @@ Env & EvalState::allocEnv(unsigned int size) Value * EvalState::allocAttr(Value & vAttrs, const Symbol & name) { - Attr & a = (*vAttrs.attrs)[name]; - a.value = allocValue(); - return a.value; + Value * v = allocValue(); + vAttrs.attrs->push_back(Attr(name, v)); + return v; } @@ -338,8 +355,7 @@ void EvalState::mkThunk_(Value & v, Expr * expr) void EvalState::cloneAttrs(Value & src, Value & dst) { mkAttrs(dst); - foreach (Bindings::iterator, i, *src.attrs) - (*dst.attrs)[i->first] = i->second; + *dst.attrs = *src.attrs; } @@ -462,21 +478,18 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) /* The recursive attributes are evaluated in the new environment. */ foreach (Attrs::iterator, i, attrs) { - nix::Attr & a = (*v.attrs)[i->first]; - a.value = state.allocValue(); - mkThunk(*a.value, env2, i->second.first); - env2.values[displ++] = a.value; - a.pos = &i->second.second; + Value * vAttr = state.allocValue(); + mkThunk(*vAttr, env2, i->second.first); + 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::iterator, i, inherited) { - nix::Attr & a = (*v.attrs)[i->first.name]; - Value * v2 = state.lookupVar(&env, i->first); - a.value = v2; - env2.values[displ++] = v2; - a.pos = &i->second; + Value * vAttr = state.lookupVar(&env, i->first); + env2.values[displ++] = vAttr; + v.attrs->push_back(nix::Attr(i->first.name, vAttr, &i->second)); } /* If the rec contains an attribute called `__overrides', then @@ -489,12 +502,12 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) Hence we need __overrides.) */ Bindings::iterator overrides = v.attrs->find(state.sOverrides); if (overrides != v.attrs->end()) { - state.forceAttrs(*overrides->second.value); - foreach (Bindings::iterator, i, *overrides->second.value->attrs) { - nix::Attr & a = (*v.attrs)[i->first]; + 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->first]] = i->second.value; - a = i->second; + env2.values[displs[i->name]] = i->value; + a = *i; } } } @@ -565,13 +578,13 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) if (i == v2.attrs->end()) throwEvalError("attribute `%1%' missing", name); try { - state.forceValue(*i->second.value); + state.forceValue(*i->value); } catch (Error & e) { addErrorPrefix(e, "while evaluating the attribute `%1%' at %2%:\n", - name, *i->second.pos); + name, *i->pos); throw; } - v = *i->second.value; + v = *i->value; } @@ -676,7 +689,7 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v) mkThunk(*env2.values[displ++], env2, i->def); } else { attrsUsed++; - env2.values[displ++] = j->second.value; + env2.values[displ++] = j->value; } } @@ -712,7 +725,7 @@ 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->second; + (*actualArgs.attrs)[i->name] = *j; else if (!i->def) throwTypeError("cannot auto-call a function that has an argument without a default value (`%1%')", i->name); } @@ -801,8 +814,9 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v) state.cloneAttrs(v1, v); + /* !!! fix */ foreach (Bindings::iterator, i, *v2.attrs) - (*v.attrs)[i->first] = i->second; + (*v.attrs)[i->name] = *i; state.nrOpUpdateValuesCopied += v.attrs->size(); } @@ -880,7 +894,7 @@ void EvalState::strictForceValue(Value & v) if (v.type == tAttrs) { foreach (Bindings::iterator, i, *v.attrs) - strictForceValue(*i->second.value); + strictForceValue(*i->value); } else if (v.type == tList) { @@ -971,7 +985,7 @@ bool EvalState::isDerivation(Value & v) { if (v.type != tAttrs) return false; Bindings::iterator i = v.attrs->find(sType); - return i != v.attrs->end() && forceStringNoCtx(*i->second.value) == "derivation"; + return i != v.attrs->end() && forceStringNoCtx(*i->value) == "derivation"; } @@ -1015,7 +1029,7 @@ string EvalState::coerceToString(Value & v, PathSet & context, Bindings::iterator i = v.attrs->find(sOutPath); if (i == v.attrs->end()) throwTypeError("cannot coerce an attribute set (except a derivation) to a string"); - return coerceToString(*i->second.value, context, coerceMore, copyToStore); + return coerceToString(*i->value, context, coerceMore, copyToStore); } if (coerceMore) { @@ -1103,7 +1117,7 @@ bool EvalState::eqValues(Value & v1, Value & v2) if (v1.attrs->size() != v2.attrs->size()) return false; Bindings::iterator i = v1.attrs->begin(), j = v2.attrs->begin(); for ( ; i != v1.attrs->end(); ++i, ++j) - if (i->first != j->first || !eqValues(*i->second.value, *j->second.value)) + if (i->name != j->name || !eqValues(*i->value, *j->value)) return false; return true; } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index ec49394425..2f1b3fa459 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -20,13 +20,24 @@ struct Env; struct Value; struct Attr; + +/* Attribute sets are represented as a vector of attributes, sorted by + symbol (i.e. pointer to the attribute name in the symbol table). */ #if HAVE_BOEHMGC -typedef std::map, gc_allocator > > Bindings; +typedef std::vector > BindingsBase; #else -typedef std::map Bindings; +typedef std::vector BindingsBase; #endif +class Bindings : public BindingsBase +{ +public: + iterator find(const Symbol & name); + Attr & operator [] (const Symbol & name); +}; + + typedef enum { tInt = 1, tBool, @@ -125,8 +136,11 @@ struct Env struct Attr { + Symbol name; Value * value; Pos * pos; + Attr(Symbol name, Value * value, Pos * pos = &noPos) + : name(name), value(value), pos(pos) { }; Attr() : pos(&noPos) { }; }; diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 63106b87b5..312c2cd405 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -10,7 +10,7 @@ string DrvInfo::queryDrvPath(EvalState & state) const if (drvPath == "" && attrs) { Bindings::iterator i = attrs->find(state.sDrvPath); PathSet context; - (string &) drvPath = i != attrs->end() ? state.coerceToPath(*i->second.value, context) : ""; + (string &) drvPath = i != attrs->end() ? state.coerceToPath(*i->value, context) : ""; } return drvPath; } @@ -21,7 +21,7 @@ string DrvInfo::queryOutPath(EvalState & state) const if (outPath == "" && attrs) { Bindings::iterator i = attrs->find(state.sOutPath); PathSet context; - (string &) outPath = i != attrs->end() ? state.coerceToPath(*i->second.value, context) : ""; + (string &) outPath = i != attrs->end() ? state.coerceToPath(*i->value, context) : ""; } return outPath; } @@ -36,23 +36,23 @@ MetaInfo DrvInfo::queryMetaInfo(EvalState & state) const Bindings::iterator a = attrs->find(state.sMeta); if (a == attrs->end()) return meta; /* fine, empty meta information */ - state.forceAttrs(*a->second.value); + state.forceAttrs(*a->value); - foreach (Bindings::iterator, i, *a->second.value->attrs) { + foreach (Bindings::iterator, i, *a->value->attrs) { MetaValue value; - state.forceValue(*i->second.value); - if (i->second.value->type == tString) { + state.forceValue(*i->value); + if (i->value->type == tString) { value.type = MetaValue::tpString; - value.stringValue = i->second.value->string.s; - } else if (i->second.value->type == tInt) { + value.stringValue = i->value->string.s; + } else if (i->value->type == tInt) { value.type = MetaValue::tpInt; - value.intValue = i->second.value->integer; - } else if (i->second.value->type == tList) { + value.intValue = i->value->integer; + } else if (i->value->type == tList) { value.type = MetaValue::tpStrings; - for (unsigned int j = 0; j < i->second.value->list.length; ++j) - value.stringValues.push_back(state.forceStringNoCtx(*i->second.value->list.elems[j])); + for (unsigned int j = 0; j < i->value->list.length; ++j) + value.stringValues.push_back(state.forceStringNoCtx(*i->value->list.elems[j])); } else continue; - ((MetaInfo &) meta)[i->first] = value; + ((MetaInfo &) meta)[i->name] = value; } return meta; @@ -99,13 +99,13 @@ static bool getDerivation(EvalState & state, Value & v, Bindings::iterator i = v.attrs->find(state.sName); /* !!! We really would like to have a decent back trace here. */ if (i == v.attrs->end()) throw TypeError("derivation name missing"); - drv.name = state.forceStringNoCtx(*i->second.value); + drv.name = state.forceStringNoCtx(*i->value); Bindings::iterator i2 = v.attrs->find(state.sSystem); if (i2 == v.attrs->end()) drv.system = "unknown"; else - drv.system = state.forceStringNoCtx(*i2->second.value); + drv.system = state.forceStringNoCtx(*i2->value); drv.attrs = v.attrs; @@ -163,7 +163,7 @@ static void getDerivations(EvalState & state, Value & vIn, typedef std::map SortedSymbols; SortedSymbols attrs; foreach (Bindings::iterator, i, *v.attrs) - attrs.insert(std::pair(i->first, i->first)); + attrs.insert(std::pair(i->name, i->name)); foreach (SortedSymbols::iterator, i, attrs) { startNest(nest, lvlDebug, format("evaluating attribute `%1%'") % i->first); @@ -178,7 +178,7 @@ static void getDerivations(EvalState & state, Value & vIn, attribute. */ if (v2.type == tAttrs) { Bindings::iterator j = v2.attrs->find(state.symbols.create("recurseForDerivations")); - if (j != v2.attrs->end() && state.forceBool(*j->second.value)) + if (j != v2.attrs->end() && state.forceBool(*j->value)) getDerivations(state, v2, pathPrefix2, autoArgs, drvs, done); } } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 01cbf7a7c2..20b8395088 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -119,18 +119,18 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v) args[0]->attrs->find(state.symbols.create("startSet")); if (startSet == args[0]->attrs->end()) throw EvalError("attribute `startSet' required"); - state.forceList(*startSet->second.value); + state.forceList(*startSet->value); list workSet; - for (unsigned int n = 0; n < startSet->second.value->list.length; ++n) - workSet.push_back(startSet->second.value->list.elems[n]); + for (unsigned int n = 0; n < startSet->value->list.length; ++n) + workSet.push_back(startSet->value->list.elems[n]); /* Get the operator. */ Bindings::iterator op = args[0]->attrs->find(state.symbols.create("operator")); if (op == args[0]->attrs->end()) throw EvalError("attribute `operator' required"); - state.forceValue(*op->second.value); + state.forceValue(*op->value); /* Construct the closure by applying the operator to element of `workSet', adding the result to `workSet', continuing until @@ -147,15 +147,15 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v) e->attrs->find(state.symbols.create("key")); if (key == e->attrs->end()) throw EvalError("attribute `key' required"); - state.forceValue(*key->second.value); + state.forceValue(*key->value); - if (doneKeys.find(*key->second.value) != doneKeys.end()) continue; - doneKeys.insert(*key->second.value); + if (doneKeys.find(*key->value) != doneKeys.end()) continue; + doneKeys.insert(*key->value); res.push_back(*e); /* Call the `operator' function with `e' as argument. */ Value call; - mkApp(call, *op->second.value, *e); + mkApp(call, *op->value, *e); state.forceList(call); /* Add the values returned by the operator to the work set. */ @@ -322,9 +322,9 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v) if (attr == args[0]->attrs->end()) throw EvalError("required attribute `name' missing"); string drvName; - Pos & posDrvName(*attr->second.pos); + Pos & posDrvName(*attr->pos); try { - drvName = state.forceStringNoCtx(*attr->second.value); + drvName = state.forceStringNoCtx(*attr->value); } catch (Error & e) { e.addPrefix(format("while evaluating the derivation attribute `name' at %1%:\n") % posDrvName); throw; @@ -339,7 +339,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v) bool outputHashRecursive = false; foreach (Bindings::iterator, i, *args[0]->attrs) { - string key = i->first; + string key = i->name; startNest(nest, lvlVomit, format("processing attribute `%1%'") % key); try { @@ -347,9 +347,9 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v) /* The `args' attribute is special: it supplies the command-line arguments to the builder. */ if (key == "args") { - state.forceList(*i->second.value); - for (unsigned int n = 0; n < i->second.value->list.length; ++n) { - string s = state.coerceToString(*i->second.value->list.elems[n], context, true); + state.forceList(*i->value); + for (unsigned int n = 0; n < i->value->list.length; ++n) { + string s = state.coerceToString(*i->value->list.elems[n], context, true); drv.args.push_back(s); } } @@ -357,11 +357,11 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v) /* All other attributes are passed to the builder through the environment. */ else { - string s = state.coerceToString(*i->second.value, context, true); + string s = state.coerceToString(*i->value, context, true); drv.env[key] = s; if (key == "builder") drv.builder = s; - else if (i->first == state.sSystem) drv.platform = s; - else if (i->first == state.sName) drvName = s; + else if (i->name == state.sSystem) drv.platform = s; + else if (i->name == state.sName) drvName = s; else if (key == "outputHash") outputHash = s; else if (key == "outputHashAlgo") outputHashAlgo = s; else if (key == "outputHashMode") { @@ -373,7 +373,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v) } catch (Error & e) { e.addPrefix(format("while evaluating the derivation attribute `%1%' at %2%:\n") - % key % *i->second.pos); + % key % *i->pos); e.addPrefix(format("while instantiating the derivation named `%1%' at %2%:\n") % drvName % posDrvName); throw; @@ -690,7 +690,7 @@ static void prim_attrNames(EvalState & state, Value * * args, Value & v) StringSet names; foreach (Bindings::iterator, i, *args[0]->attrs) - names.insert(i->first); + names.insert(i->name); unsigned int n = 0; foreach (StringSet::iterator, i, names) @@ -708,8 +708,8 @@ static void prim_getAttr(EvalState & state, Value * * args, Value & v) if (i == args[1]->attrs->end()) throw EvalError(format("attribute `%1%' missing") % attr); // !!! add to stack trace? - state.forceValue(*i->second.value); - v = *i->second.value; + state.forceValue(*i->value); + v = *i->value; } @@ -735,11 +735,18 @@ static void prim_removeAttrs(EvalState & state, Value * * args, Value & v) state.forceAttrs(*args[0]); state.forceList(*args[1]); - state.cloneAttrs(*args[0], v); - + /* Get the attribute names to be removed. */ + std::set names; for (unsigned int i = 0; i < args[1]->list.length; ++i) { state.forceStringNoCtx(*args[1]->list.elems[i]); - v.attrs->erase(state.symbols.create(args[1]->list.elems[i]->string.s)); + names.insert(state.symbols.create(args[1]->list.elems[i]->string.s)); + } + + /* Copy all attributes not in that set. */ + state.mkAttrs(v); + foreach (Bindings::iterator, i, *args[0]->attrs) { + if (names.find(i->name) == names.end()) + v.attrs->push_back(*i); } } @@ -761,13 +768,15 @@ static void prim_listToAttrs(EvalState & state, Value * * args, Value & v) Bindings::iterator j = v2.attrs->find(state.sName); if (j == v2.attrs->end()) throw TypeError("`name' attribute missing in a call to `listToAttrs'"); - string name = state.forceStringNoCtx(*j->second.value); + string name = state.forceStringNoCtx(*j->value); Bindings::iterator j2 = v2.attrs->find(state.symbols.create("value")); if (j2 == v2.attrs->end()) throw TypeError("`value' attribute missing in a call to `listToAttrs'"); - (*v.attrs)[state.symbols.create(name)] = j2->second; + Attr & a = (*v.attrs)[state.symbols.create(name)]; + a.value = j2->value; + a.pos = j2->pos; } } @@ -783,9 +792,9 @@ static void prim_intersectAttrs(EvalState & state, Value * * args, Value & v) state.mkAttrs(v); foreach (Bindings::iterator, i, *args[0]->attrs) { - Bindings::iterator j = args[1]->attrs->find(i->first); + Bindings::iterator j = args[1]->attrs->find(i->name); if (j != args[1]->attrs->end()) - (*v.attrs)[j->first] = j->second; + v.attrs->push_back(*j); } } diff --git a/src/libexpr/symbol-table.hh b/src/libexpr/symbol-table.hh index 20ebe5fedb..976117a20a 100644 --- a/src/libexpr/symbol-table.hh +++ b/src/libexpr/symbol-table.hh @@ -28,6 +28,8 @@ private: friend class SymbolTable; public: + Symbol() : s(0) { }; + bool operator == (const Symbol & s2) const { return s == s2.s; diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index 1bb200fb85..4f9b62d5f1 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -34,7 +34,7 @@ static void showAttrs(EvalState & state, bool strict, bool location, StringSet names; foreach (Bindings::iterator, i, attrs) - names.insert(i->first); + names.insert(i->name); foreach (StringSet::iterator, i, names) { Attr & a(attrs[state.symbols.create(*i)]); @@ -90,16 +90,16 @@ static void printValueAsXML(EvalState & state, bool strict, bool location, Path drvPath; a = v.attrs->find(state.sDrvPath); if (a != v.attrs->end()) { - if (strict) state.forceValue(*a->second.value); - if (a->second.value->type == tString) - xmlAttrs["drvPath"] = drvPath = a->second.value->string.s; + if (strict) state.forceValue(*a->value); + if (a->value->type == tString) + xmlAttrs["drvPath"] = drvPath = a->value->string.s; } a = v.attrs->find(state.sOutPath); if (a != v.attrs->end()) { - if (strict) state.forceValue(*a->second.value); - if (a->second.value->type == tString) - xmlAttrs["outPath"] = a->second.value->string.s; + if (strict) state.forceValue(*a->value); + if (a->value->type == tString) + xmlAttrs["outPath"] = a->value->string.s; } XMLOpenElement _(doc, "derivation", xmlAttrs);