From 2160258cc4b58a379c31616f9be91973694cdf43 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 23 Apr 2019 11:07:47 +0200 Subject: [PATCH] Store contexts as symbols This provides some deduplication since most contexts are used multiple times. Also, store singleton contexts directly in the Value object. This saves a 16-byte Context object. This is useful because the vast majority of contexts are singletons, e.g. 23723 out of 26138 in a NixOS 19.03 system configuration. --- src/libexpr/eval.cc | 33 ++++++---------------- src/libexpr/eval.hh | 4 +-- src/libexpr/gc.cc | 15 ++++++++-- src/libexpr/gc.hh | 1 + src/libexpr/symbol-table.hh | 3 ++ src/libexpr/value-to-json.cc | 2 +- src/libexpr/value-to-xml.cc | 2 +- src/libexpr/value.hh | 55 ++++++++++++++++++++++++++++++++---- 8 files changed, 78 insertions(+), 37 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index e39b2c513..cbc06bc1b 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -21,6 +21,8 @@ namespace nix { +SymbolTable symbols; + // FIXME static char * dupString(const char * s) @@ -500,15 +502,7 @@ void mkString(Value & v, const char * s) Value & mkString(Value & v, const string & s, const PathSet & context) { mkString(v, s.c_str()); - if (!context.empty()) { - size_t n = 0; - // FIXME: store as Symbols? - v.string.context = (const char * *) - malloc((context.size() + 1) * sizeof(char *)); - for (auto & i : context) - v.string.context[n++] = dupString(i.c_str()); - v.string.context[n] = 0; - } + v.setContext(context); return v; } @@ -1464,18 +1458,10 @@ string EvalState::forceString(Value & v, const Pos & pos) } -void copyContext(const Value & v, PathSet & context) -{ - if (v.string.context) - for (const char * * p = v.string.context; *p; ++p) - context.insert(*p); -} - - string EvalState::forceString(Value & v, PathSet & context, const Pos & pos) { string s = forceString(v, pos); - copyContext(v, context); + v.getContext(context); return s; } @@ -1484,12 +1470,14 @@ string EvalState::forceStringNoCtx(Value & v, const Pos & pos) { string s = forceString(v, pos); if (v.string.context) { + PathSet context; + v.getContext(context); if (pos) throwEvalError("the string '%1%' is not allowed to refer to a store path (such as '%2%'), at %3%", - v.string.s, v.string.context[0], pos); + v.string.s, *context.begin(), pos); else throwEvalError("the string '%1%' is not allowed to refer to a store path (such as '%2%')", - v.string.s, v.string.context[0]); + v.string.s, *context.begin()); } return s; } @@ -1514,7 +1502,7 @@ string EvalState::coerceToString(const Pos & pos, Value & v, PathSet & context, string s; if (v.type == tString) { - copyContext(v, context); + v.getContext(context); return v.string.s; } @@ -1813,9 +1801,6 @@ size_t valueSize(Value & v) switch (v.type) { case tString: sz += doString(v.string.s); - if (v.string.context) - for (const char * * p = v.string.context; *p; ++p) - sz += doString(*p); break; case tPath: sz += doString(v.path); diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index ee37a6a68..4ed8fc541 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -84,8 +84,6 @@ public: Value & mkString(Value & v, const string & s, const PathSet & context = PathSet()); -void copyContext(const Value & v, PathSet & context); - /* Cache for calls to addToStore(); maps source paths to the store paths. */ @@ -106,7 +104,7 @@ void initGC(); class EvalState { public: - SymbolTable symbols; + SymbolTable & symbols{nix::symbols}; // FIXME: remove const Symbol sWith, sOutPath, sDrvPath, sType, sMeta, sName, sValue, sSystem, sOverrides, sOutputs, sOutputName, sIgnoreNulls, diff --git a/src/libexpr/gc.cc b/src/libexpr/gc.cc index 3f3999133..89c79b6d1 100644 --- a/src/libexpr/gc.cc +++ b/src/libexpr/gc.cc @@ -100,6 +100,7 @@ void GC::gc() break; } + case tContext: case tInt: case tBool: case tNull: @@ -107,9 +108,14 @@ void GC::gc() case tFloat: break; - case tString: - // FIXME + case tString: { + auto obj2 = (Value *) obj; + // FIXME: GC string + // See setContext(). + if (!(((ptrdiff_t) obj2->string.context) & 1)) + push(obj2->string.context); break; + } case tPath: // FIXME @@ -245,9 +251,12 @@ std::pair GC::Arena::freeUnmarked() case tWithAttrsEnv: objSize = ((Env *) obj)->words(); break; + case tContext: + objSize = ((Context *) obj)->getSize() + 1; + break; default: + printError("GC encountered invalid object with tag %d", tag); abort(); - //throw Error("GC encountered invalid object with tag %d", tag); } } diff --git a/src/libexpr/gc.hh b/src/libexpr/gc.hh index 99fee8bf8..5684b4ed5 100644 --- a/src/libexpr/gc.hh +++ b/src/libexpr/gc.hh @@ -20,6 +20,7 @@ enum Tag { tEnv, tWithExprEnv, tWithAttrsEnv, + tContext, // Value tags tInt, diff --git a/src/libexpr/symbol-table.hh b/src/libexpr/symbol-table.hh index 91faea122..f3b07ff2a 100644 --- a/src/libexpr/symbol-table.hh +++ b/src/libexpr/symbol-table.hh @@ -19,6 +19,7 @@ private: const string * s; // pointer into SymbolTable Symbol(const string * s) : s(s) { }; friend class SymbolTable; + friend class Value; public: Symbol() : s(0) { }; @@ -84,4 +85,6 @@ public: } }; +extern SymbolTable symbols; + } diff --git a/src/libexpr/value-to-json.cc b/src/libexpr/value-to-json.cc index 73f2843aa..1f168b8e3 100644 --- a/src/libexpr/value-to-json.cc +++ b/src/libexpr/value-to-json.cc @@ -27,7 +27,7 @@ void printValueAsJSON(EvalState & state, bool strict, break; case tString: - copyContext(v, context); + v.getContext(context); out.write(v.string.s); break; diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index d00b1c621..4128cd455 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -70,7 +70,7 @@ static void printValueAsXML(EvalState & state, bool strict, bool location, case tString: /* !!! show the context? */ - copyContext(v, context); + v.getContext(context); doc.writeEmptyElement("string", singletonAttrs("value", v.string.s)); break; diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 187ddc0fb..50e50e913 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -10,7 +10,7 @@ struct Env; struct Expr; struct ExprLambda; struct PrimOp; -struct PrimOp; +struct Context; class Symbol; struct Pos; class EvalState; @@ -70,6 +70,24 @@ std::ostream & operator << (std::ostream & str, const ExternalValueBase & v); #endif +class Context : Object +{ + friend class Value; + friend class GC; + + Symbol members[0]; + + Context(const PathSet & context) : Object(tContext, context.size()) + { + size_t n = 0; + for (auto & i : context) + members[n++] = symbols.create(i); + } + + size_t getSize() { return getMisc(); } +}; + + struct Value : Object { union @@ -94,12 +112,10 @@ struct Value : Object with context C is used as a derivation attribute, then the derivations in C will be added to the inputDrvs of the derivation, and the other store paths in C will be added to - the inputSrcs of the derivations. - - For canonicity, the store paths should be in sorted order. */ + the inputSrcs of the derivations. */ struct { const char * s; - const char * * context; // must be in sorted order + Context * context; } string; const char * path; @@ -152,6 +168,35 @@ public: } constexpr static Size words() { return 3; } // FIXME + + void setContext(const PathSet & context) + { + if (context.size() == 0) + string.context = nullptr; + else if (context.size() == 1) { + // If we have a single context, then store it + // directly. This saves allocating a Context object (16 + // bytes). + auto symbol = symbols.create(*context.begin()); + string.context = (Context *) (((ptrdiff_t) symbol.s) | 1); + } else { + string.context = gc.alloc(1 + context.size(), context); + } + } + + void getContext(PathSet & context) + { + if (string.context) { + if (((ptrdiff_t) string.context) & 1) { + auto s = (const std::string *) (((ptrdiff_t) string.context) & ~1UL); + context.insert(*s); + } else { + auto size = string.context->getSize(); + for (size_t i = 0; i < size; ++i) + context.insert(string.context->members[i]); + } + } + } };