From f67a7007a230c84015793794277c0449e682ab54 Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Tue, 13 Feb 2018 01:32:29 +0200 Subject: [PATCH 1/6] libexpr: Pre-reserve space in string in unescapeStr() Avoids some malloc() traffic. --- src/libexpr/lexer.l | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libexpr/lexer.l b/src/libexpr/lexer.l index 28a0a6a87..e5e01fb58 100644 --- a/src/libexpr/lexer.l +++ b/src/libexpr/lexer.l @@ -49,9 +49,10 @@ static void adjustLoc(YYLTYPE * loc, const char * s, size_t len) } -static Expr * unescapeStr(SymbolTable & symbols, const char * s) +static Expr * unescapeStr(SymbolTable & symbols, const char * s, size_t length) { string t; + t.reserve(length); char c; while ((c = *s++)) { if (c == '\\') { @@ -150,7 +151,7 @@ or { return OR_KW; } /* It is impossible to match strings ending with '$' with one regex because trailing contexts are only valid at the end of a rule. (A sane but undocumented limitation.) */ - yylval->e = unescapeStr(data->symbols, yytext); + yylval->e = unescapeStr(data->symbols, yytext, yyleng); return STR; } \$\{ { PUSH_STATE(INSIDE_DOLLAR_CURLY); return DOLLAR_CURLY; } @@ -178,7 +179,7 @@ or { return OR_KW; } return IND_STR; } \'\'\\. { - yylval->e = unescapeStr(data->symbols, yytext + 2); + yylval->e = unescapeStr(data->symbols, yytext + 2, yyleng - 2); return IND_STR; } \$\{ { PUSH_STATE(INSIDE_DOLLAR_CURLY); return DOLLAR_CURLY; } From b8bed7da14b26dcc328075522842dd16aa71b434 Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Fri, 16 Feb 2018 05:14:35 +0200 Subject: [PATCH 2/6] libexpr: Optimize prim_attrNames a bit Instead of having lexicographicOrder() create a temporary sorted array of Attr*:s and copying attr names from that, copy the attr names first and then sort that. --- src/libexpr/primops.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index ca97b2b28..89e984d2e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1138,8 +1138,11 @@ static void prim_attrNames(EvalState & state, const Pos & pos, Value * * args, V state.mkList(v, args[0]->attrs->size()); size_t n = 0; - for (auto & i : args[0]->attrs->lexicographicOrder()) - mkString(*(v.listElems()[n++] = state.allocValue()), i->name); + for (auto & i : *args[0]->attrs) + mkString(*(v.listElems()[n++] = state.allocValue()), i.name); + + std::sort(v.listElems(), v.listElems() + n, + [](Value * v1, Value * v2) { return strcmp(v1->string.s, v2->string.s) < 0; }); } From 0845cdf9443a6b304c1bcec304a462ae4995c744 Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Tue, 13 Feb 2018 05:00:17 +0200 Subject: [PATCH 3/6] libexpr: Rely on Boehm returning zeroed memory in EvalState::allocEnv() Boehm guarantees that memory returned by GC_malloc() is zeroed, so take advantage of that. --- src/libexpr/attr-set.cc | 3 ++- src/libexpr/eval.cc | 7 +++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libexpr/attr-set.cc b/src/libexpr/attr-set.cc index 910428c02..b284daa3c 100644 --- a/src/libexpr/attr-set.cc +++ b/src/libexpr/attr-set.cc @@ -7,13 +7,14 @@ namespace nix { +/* Note: Various places expect the allocated memory to be zeroed. */ static void * allocBytes(size_t n) { void * p; #if HAVE_BOEHMGC p = GC_malloc(n); #else - p = malloc(n); + p = calloc(n, 1); #endif if (!p) throw std::bad_alloc(); return p; diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b94bc597b..48542d8e5 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -43,13 +43,14 @@ static char * dupString(const char * s) } +/* Note: Various places expect the allocated memory to be zeroed. */ static void * allocBytes(size_t n) { void * p; #if HAVE_BOEHMGC p = GC_malloc(n); #else - p = malloc(n); + p = calloc(n, 1); #endif if (!p) throw std::bad_alloc(); return p; @@ -582,9 +583,7 @@ Env & EvalState::allocEnv(unsigned int size) Env * env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *)); env->size = size; - /* Clear the values because maybeThunk() and lookupVar fromWith expect this. */ - for (unsigned i = 0; i < size; ++i) - env->values[i] = 0; + /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */ return *env; } From 7e0360504d1a964ad5bd0da996045bc3868d0d7d Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Sat, 4 Mar 2017 15:24:06 +0200 Subject: [PATCH 4/6] libexpr: Optimize prim_derivationStrict by using more symbol comparisons --- src/libexpr/eval.cc | 4 ++++ src/libexpr/eval.hh | 3 ++- src/libexpr/primops.cc | 18 +++++++++--------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 48542d8e5..2144d3452 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -294,6 +294,10 @@ EvalState::EvalState(const Strings & _searchPath, ref store) , sWrong(symbols.create("wrong")) , sStructuredAttrs(symbols.create("__structuredAttrs")) , sBuilder(symbols.create("builder")) + , sArgs(symbols.create("args")) + , sOutputHash(symbols.create("outputHash")) + , sOutputHashAlgo(symbols.create("outputHashAlgo")) + , sOutputHashMode(symbols.create("outputHashMode")) , repair(NoRepair) , store(store) , baseEnv(allocEnv(128)) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 51905d7e1..9d8799b79 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -69,7 +69,8 @@ public: const Symbol sWith, sOutPath, sDrvPath, sType, sMeta, sName, sValue, sSystem, sOverrides, sOutputs, sOutputName, sIgnoreNulls, sFile, sLine, sColumn, sFunctor, sToString, - sRight, sWrong, sStructuredAttrs, sBuilder; + sRight, sWrong, sStructuredAttrs, sBuilder, sArgs, + sOutputHash, sOutputHashAlgo, sOutputHashMode; Symbol sDerivationNix; /* If set, force copying files to the Nix store even if they diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 89e984d2e..317623b22 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -589,7 +589,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * /* The `args' attribute is special: it supplies the command-line arguments to the builder. */ - if (key == "args") { + if (i->name == state.sArgs) { state.forceList(*i->value, pos); for (unsigned int n = 0; n < i->value->listSize(); ++n) { string s = state.coerceToString(posDrvName, *i->value->listElems()[n], context, true); @@ -614,13 +614,13 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * drv.platform = state.forceStringNoCtx(*i->value, posDrvName); else if (i->name == state.sName) drvName = state.forceStringNoCtx(*i->value, posDrvName); - else if (key == "outputHash") + else if (i->name == state.sOutputHash) outputHash = state.forceStringNoCtx(*i->value, posDrvName); - else if (key == "outputHashAlgo") + else if (i->name == state.sOutputHashAlgo) outputHashAlgo = state.forceStringNoCtx(*i->value, posDrvName); - else if (key == "outputHashMode") + else if (i->name == state.sOutputHashMode) handleHashMode(state.forceStringNoCtx(*i->value, posDrvName)); - else if (key == "outputs") { + else if (i->name == state.sOutputs) { /* Require ‘outputs’ to be a list of strings. */ state.forceList(*i->value, posDrvName); Strings ss; @@ -638,10 +638,10 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * drvName = s; printMsg(lvlVomit, format("derivation name is '%1%'") % drvName); } - else if (key == "outputHash") outputHash = s; - else if (key == "outputHashAlgo") outputHashAlgo = s; - else if (key == "outputHashMode") handleHashMode(s); - else if (key == "outputs") + else if (i->name == state.sOutputHash) outputHash = s; + else if (i->name == state.sOutputHashAlgo) outputHashAlgo = s; + else if (i->name == state.sOutputHashMode) handleHashMode(s); + else if (i->name == state.sOutputs) handleOutputs(tokenizeString(s)); } From 66eeff33456d544e5852e580d8cac21f0c38c11f Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Sat, 17 Feb 2018 16:51:10 +0200 Subject: [PATCH 5/6] libexpr: Remove unnecessary drvName assignment in prim_derivationStrict drvName is already assigned to the same value right at the start of the function. --- src/libexpr/primops.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 317623b22..f6c34d525 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -612,8 +612,6 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * drv.builder = state.forceString(*i->value, context, posDrvName); else if (i->name == state.sSystem) drv.platform = state.forceStringNoCtx(*i->value, posDrvName); - else if (i->name == state.sName) - drvName = state.forceStringNoCtx(*i->value, posDrvName); else if (i->name == state.sOutputHash) outputHash = state.forceStringNoCtx(*i->value, posDrvName); else if (i->name == state.sOutputHashAlgo) @@ -634,10 +632,6 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * drv.env.emplace(key, s); if (i->name == state.sBuilder) drv.builder = s; else if (i->name == state.sSystem) drv.platform = s; - else if (i->name == state.sName) { - drvName = s; - printMsg(lvlVomit, format("derivation name is '%1%'") % drvName); - } else if (i->name == state.sOutputHash) outputHash = s; else if (i->name == state.sOutputHashAlgo) outputHashAlgo = s; else if (i->name == state.sOutputHashMode) handleHashMode(s); From 37264ed0ad898cdd6880de8ce6e5dda7977eed5f Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Fri, 16 Feb 2018 05:13:39 +0200 Subject: [PATCH 6/6] libexpr: Avoid an unnecessary string copy in prim_derivationStrict --- src/libexpr/primops.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index f6c34d525..a800d2429 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -553,7 +553,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * for (auto & i : args[0]->attrs->lexicographicOrder()) { if (i->name == state.sIgnoreNulls) continue; - string key = i->name; + const string & key = i->name; vomit("processing attribute '%1%'", key); auto handleHashMode = [&](const std::string & s) {