From 9ac836d1d6d7e3480ad3bc4dbe23286d9052052b Mon Sep 17 00:00:00 2001 From: pennae Date: Wed, 19 Jan 2022 14:31:30 +0100 Subject: [PATCH] don't use Symbols for strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit string expressions by and large do not need the benefits a Symbol gives us, instead they pollute the symbol table and cause unnecessary overhead for almost all strings. the one place we can think of that benefits from them (attrpaths with expressions) extracts the benefit in the parser, which we'll have to touch anyway when changing ExprString to hold strings. this gives a sizeable improvement on of 3-5% on all benchmarks we've run. before nix search --no-eval-cache --offline ../nixpkgs hello Time (mean ± σ): 8.844 s ± 0.045 s [User: 6.750 s, System: 1.663 s] Range (min … max): 8.758 s … 8.922 s 20 runs nix eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix Time (mean ± σ): 367.4 ms ± 3.3 ms [User: 332.3 ms, System: 35.2 ms] Range (min … max): 364.0 ms … 375.2 ms 20 runs nix eval --raw --impure --expr 'with import {}; system' Time (mean ± σ): 2.810 s ± 0.030 s [User: 2.517 s, System: 0.225 s] Range (min … max): 2.742 s … 2.854 s 20 runs after nix search --no-eval-cache --offline ../nixpkgs hello Time (mean ± σ): 8.533 s ± 0.068 s [User: 6.485 s, System: 1.642 s] Range (min … max): 8.404 s … 8.657 s 20 runs nix eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix Time (mean ± σ): 347.6 ms ± 3.1 ms [User: 313.1 ms, System: 34.5 ms] Range (min … max): 343.3 ms … 354.6 ms 20 runs nix eval --raw --impure --expr 'with import {}; system' Time (mean ± σ): 2.709 s ± 0.032 s [User: 2.414 s, System: 0.232 s] Range (min … max): 2.655 s … 2.788 s 20 runs --- src/libexpr/nixexpr.hh | 4 ++-- src/libexpr/parser.y | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index ae11560ea..a2070ff69 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -110,9 +110,9 @@ struct ExprFloat : Expr struct ExprString : Expr { - Symbol s; + string s; Value v; - ExprString(const Symbol & s) : s(s) { v.mkString(s); }; + ExprString(std::string s) : s(std::move(s)) { v.mkString(this->s.data()); }; COMMON_METHODS Value * maybeThunk(EvalState & state, Env & env); }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 38b218156..88bdebcd6 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -166,7 +166,7 @@ static void addFormal(const Pos & pos, Formals * formals, const Formal & formal) static Expr * stripIndentation(const Pos & pos, SymbolTable & symbols, vector > > & es) { - if (es.empty()) return new ExprString(symbols.create("")); + if (es.empty()) return new ExprString(""); /* Figure out the minimum indentation. Note that by design whitespace-only final lines are not taken into account. (So @@ -244,7 +244,7 @@ static Expr * stripIndentation(const Pos & pos, SymbolTable & symbols, s2 = string(s2, 0, p + 1); } - es2->emplace_back(i->first, new ExprString(symbols.create(s2))); + es2->emplace_back(i->first, new ExprString(s2)); }; for (; i != es.end(); ++i, --n) { std::visit(overloaded { trimExpr, trimString }, i->second); @@ -434,7 +434,7 @@ expr_simple $$ = new ExprCall(CUR_POS, new ExprVar(data->symbols.create("__findFile")), {new ExprVar(data->symbols.create("__nixPath")), - new ExprString(data->symbols.create(path))}); + new ExprString(path)}); } | URI { static bool noURLLiterals = settings.isExperimentalFeatureEnabled(Xp::NoUrlLiterals); @@ -443,7 +443,7 @@ expr_simple .msg = hintfmt("URL literals are disabled"), .errPos = CUR_POS }); - $$ = new ExprString(data->symbols.create($1)); + $$ = new ExprString(string($1)); } | '(' expr ')' { $$ = $2; } /* Let expressions `let {..., body = ...}' are just desugared @@ -458,19 +458,19 @@ expr_simple ; string_parts - : STR { $$ = new ExprString(data->symbols.create($1)); } + : STR { $$ = new ExprString(string($1)); } | string_parts_interpolated { $$ = new ExprConcatStrings(CUR_POS, true, $1); } - | { $$ = new ExprString(data->symbols.create("")); } + | { $$ = new ExprString(""); } ; string_parts_interpolated : string_parts_interpolated STR - { $$ = $1; $1->emplace_back(makeCurPos(@2, data), new ExprString(data->symbols.create($2))); } + { $$ = $1; $1->emplace_back(makeCurPos(@2, data), new ExprString(string($2))); } | string_parts_interpolated DOLLAR_CURLY expr '}' { $$ = $1; $1->emplace_back(makeCurPos(@2, data), $3); } | DOLLAR_CURLY expr '}' { $$ = new vector >; $$->emplace_back(makeCurPos(@1, data), $2); } | STR DOLLAR_CURLY expr '}' { $$ = new vector >; - $$->emplace_back(makeCurPos(@1, data), new ExprString(data->symbols.create($1))); + $$->emplace_back(makeCurPos(@1, data), new ExprString(string($1))); $$->emplace_back(makeCurPos(@2, data), $3); } ; @@ -524,7 +524,7 @@ attrs { $$ = $1; ExprString * str = dynamic_cast($2); if (str) { - $$->push_back(AttrName(str->s)); + $$->push_back(AttrName(data->symbols.create(str->s))); delete str; } else throw ParseError({ @@ -541,7 +541,7 @@ attrpath { $$ = $1; ExprString * str = dynamic_cast($3); if (str) { - $$->push_back(AttrName(str->s)); + $$->push_back(AttrName(data->symbols.create(str->s))); delete str; } else $$->push_back(AttrName($3)); @@ -551,7 +551,7 @@ attrpath { $$ = new vector; ExprString *str = dynamic_cast($1); if (str) { - $$->push_back(AttrName(str->s)); + $$->push_back(AttrName(data->symbols.create(str->s))); delete str; } else $$->push_back(AttrName($1));