From f7b2476415906097404532c7485622eb37cf17e0 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 8 Mar 2024 05:37:19 +0100 Subject: [PATCH] don't repeatedly look up ast internal symbols these symbols are used a *lot*, so it makes sense to cache them. this mostly increases clarity of the code (however clear one may wish to call the parser desugaring here), but it also provides a small performance benefit. (cherry picked from commit 09a1128d9e2ff0ae6176784938047350d6f8a782) Change-Id: I73d9f66be4555168e048cb2d542277251580c2d1 --- src/libexpr/eval.cc | 12 +++++++++++- src/libexpr/eval.hh | 2 ++ src/libexpr/nixexpr.hh | 5 +++++ src/libexpr/parser-state.hh | 1 + src/libexpr/parser.y | 31 +++++++++++++++++-------------- 5 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 2a6a89ddb..055c0ca3f 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -414,6 +414,16 @@ EvalState::EvalState( , sPath(symbols.create("path")) , sPrefix(symbols.create("prefix")) , sOutputSpecified(symbols.create("outputSpecified")) + , exprSymbols{ + .sub = symbols.create("__sub"), + .lessThan = symbols.create("__lessThan"), + .mul = symbols.create("__mul"), + .div = symbols.create("__div"), + .or_ = symbols.create("or"), + .findFile = symbols.create("__findFile"), + .nixPath = symbols.create("__nixPath"), + .body = symbols.create("body") + } , repair(NoRepair) , emptyBindings(0) , derivationInternal(rootPath(CanonPath("/builtin/derivation.nix"))) @@ -2815,7 +2825,7 @@ Expr * EvalState::parse( const SourcePath & basePath, std::shared_ptr & staticEnv) { - auto result = parseExprFromBuf(text, length, origin, basePath, symbols, positions); + auto result = parseExprFromBuf(text, length, origin, basePath, symbols, positions, exprSymbols); result->bindVars(*this, staticEnv); diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 8355e443e..3f661e73e 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -206,6 +206,8 @@ public: sPrefix, sOutputSpecified; + const Expr::AstSymbols exprSymbols; + /** * If set, force copying files to the Nix store even if they * already exist there. diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 6f2a4c0f9..4b41fe1fc 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -141,6 +141,11 @@ std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath) struct Expr { + struct AstSymbols { + Symbol sub, lessThan, mul, div, or_, findFile, nixPath, body; + }; + + virtual ~Expr() { }; virtual void show(const SymbolTable & symbols, std::ostream & str) const; virtual void bindVars(EvalState & es, const std::shared_ptr & env); diff --git a/src/libexpr/parser-state.hh b/src/libexpr/parser-state.hh index 34863c001..b0fb93767 100644 --- a/src/libexpr/parser-state.hh +++ b/src/libexpr/parser-state.hh @@ -42,6 +42,7 @@ struct ParserState { Expr * result; SourcePath basePath; PosTable::Origin origin; + const Expr::AstSymbols & s; void dupAttr(const AttrPath & attrPath, const PosIdx pos, const PosIdx prevPos); void dupAttr(Symbol attr, const PosIdx pos, const PosIdx prevPos); diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 5027b7b99..982d112dc 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -39,7 +39,8 @@ Expr * parseExprFromBuf( Pos::Origin origin, const SourcePath & basePath, SymbolTable & symbols, - PosTable & positions); + PosTable & positions, + const Expr::AstSymbols & astSymbols); } @@ -166,13 +167,13 @@ expr_if expr_op : '!' expr_op %prec NOT { $$ = new ExprOpNot($2); } - | '-' expr_op %prec NEGATE { $$ = new ExprCall(CUR_POS, new ExprVar(state->symbols.create("__sub")), {new ExprInt(0), $2}); } + | '-' expr_op %prec NEGATE { $$ = new ExprCall(CUR_POS, new ExprVar(state->s.sub), {new ExprInt(0), $2}); } | expr_op EQ expr_op { $$ = new ExprOpEq($1, $3); } | expr_op NEQ expr_op { $$ = new ExprOpNEq($1, $3); } - | expr_op '<' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->symbols.create("__lessThan")), {$1, $3}); } - | expr_op LEQ expr_op { $$ = new ExprOpNot(new ExprCall(state->at(@2), new ExprVar(state->symbols.create("__lessThan")), {$3, $1})); } - | expr_op '>' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->symbols.create("__lessThan")), {$3, $1}); } - | expr_op GEQ expr_op { $$ = new ExprOpNot(new ExprCall(state->at(@2), new ExprVar(state->symbols.create("__lessThan")), {$1, $3})); } + | expr_op '<' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.lessThan), {$1, $3}); } + | expr_op LEQ expr_op { $$ = new ExprOpNot(new ExprCall(state->at(@2), new ExprVar(state->s.lessThan), {$3, $1})); } + | expr_op '>' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.lessThan), {$3, $1}); } + | expr_op GEQ expr_op { $$ = new ExprOpNot(new ExprCall(state->at(@2), new ExprVar(state->s.lessThan), {$1, $3})); } | expr_op AND expr_op { $$ = new ExprOpAnd(state->at(@2), $1, $3); } | expr_op OR expr_op { $$ = new ExprOpOr(state->at(@2), $1, $3); } | expr_op IMPL expr_op { $$ = new ExprOpImpl(state->at(@2), $1, $3); } @@ -180,9 +181,9 @@ expr_op | expr_op '?' attrpath { $$ = new ExprOpHasAttr($1, std::move(*$3)); delete $3; } | expr_op '+' expr_op { $$ = new ExprConcatStrings(state->at(@2), false, new std::vector >({{state->at(@1), $1}, {state->at(@3), $3}})); } - | expr_op '-' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->symbols.create("__sub")), {$1, $3}); } - | expr_op '*' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->symbols.create("__mul")), {$1, $3}); } - | expr_op '/' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->symbols.create("__div")), {$1, $3}); } + | expr_op '-' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.sub), {$1, $3}); } + | expr_op '*' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.mul), {$1, $3}); } + | expr_op '/' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.div), {$1, $3}); } | expr_op CONCAT expr_op { $$ = new ExprOpConcatLists(state->at(@2), $1, $3); } | expr_app ; @@ -206,7 +207,7 @@ expr_select | /* Backwards compatibility: because Nixpkgs has a rarely used function named ‘or’, allow stuff like ‘map or [...]’. */ expr_simple OR_KW - { $$ = new ExprCall(CUR_POS, $1, {new ExprVar(CUR_POS, state->symbols.create("or"))}); } + { $$ = new ExprCall(CUR_POS, $1, {new ExprVar(CUR_POS, state->s.or_)}); } | expr_simple ; @@ -233,8 +234,8 @@ expr_simple | SPATH { std::string path($1.p + 1, $1.l - 2); $$ = new ExprCall(CUR_POS, - new ExprVar(state->symbols.create("__findFile")), - {new ExprVar(state->symbols.create("__nixPath")), + new ExprVar(state->s.findFile), + {new ExprVar(state->s.nixPath), new ExprString(std::move(path))}); } | URI { @@ -250,7 +251,7 @@ expr_simple /* Let expressions `let {..., body = ...}' are just desugared into `(rec {..., body = ...}).body'. */ | LET '{' binds '}' - { $3->recursive = true; $$ = new ExprSelect(noPos, $3, state->symbols.create("body")); } + { $3->recursive = true; $$ = new ExprSelect(noPos, $3, state->s.body); } | REC '{' binds '}' { $3->recursive = true; $$ = $3; } | '{' binds '}' @@ -411,7 +412,8 @@ Expr * parseExprFromBuf( Pos::Origin origin, const SourcePath & basePath, SymbolTable & symbols, - PosTable & positions) + PosTable & positions, + const Expr::AstSymbols & astSymbols) { yyscan_t scanner; ParserState state { @@ -419,6 +421,7 @@ Expr * parseExprFromBuf( .positions = positions, .basePath = basePath, .origin = {origin}, + .s = astSymbols, }; yylex_init(&scanner);