Expr stats #14102
Draft
Expr stats #14102
+253
−60
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Saves 24-32 bytes per string (size of std::string) 2. Saves additional bytes by not over-allocating strings (in total we save ~1% memory) 3. Sets us up to perform a similar transformation on the other Expr subclasses 4. Makes ExprString trivially moveable (before the string data might move, causing the Value's pointer to become invalid). This is important so we can put ExprStrings in an std::vector and refer to them by index We have introduced a string copy in ParserState::stripIndentation(). This could be removed by pre-allocating the right sized string in the arena, but this adds complexity and doesn't seem to improve performance, so for now we've left the copy in.
Radvendii
force-pushed
the
expr-stats
branch
from
d3497b6 to
fb6da3c
Compare
xokdvium
reviewed
Sep 28, 2025
Comment on lines
+2959
to
+2985
| {"total", Expr::nrCreated.load()}, | ||
| {"ExprInt", ExprInt::nrCreated.load()}, | ||
| {"ExprFloat", ExprFloat::nrCreated.load()}, | ||
| {"ExprString", ExprString::nrCreated.load()}, | ||
| {"ExprPath", ExprPath::nrCreated.load()}, | ||
| {"ExprVar", ExprVar::nrCreated.load()}, | ||
| {"ExprInheritFrom", ExprInheritFrom::nrCreated.load()}, | ||
| {"ExprSelect", ExprSelect::nrCreated.load()}, | ||
| {"ExprOpHasAttr", ExprOpHasAttr::nrCreated.load()}, | ||
| {"ExprAttr", ExprAttrs::nrCreated.load()}, | ||
| {"ExprList", ExprList::nrCreated.load()}, | ||
| {"ExprLambda", ExprLambda::nrCreated.load()}, | ||
| {"ExprCall", ExprCall::nrCreated.load()}, | ||
| {"ExprLet", ExprLet::nrCreated.load()}, | ||
| {"ExprWith", ExprWith::nrCreated.load()}, | ||
| {"ExprIf", ExprIf::nrCreated.load()}, | ||
| {"ExprAssert", ExprAssert::nrCreated.load()}, | ||
| {"ExprOpNot", ExprOpNot::nrCreated.load()}, | ||
| {"ExprConcatStrings", ExprConcatStrings::nrCreated.load()}, | ||
| {"ExprPos", ExprPos::nrCreated.load()}, | ||
| {"ExprOpEq", ExprOpEq::nrCreated.load()}, | ||
| {"ExprOpNEq", ExprOpNEq::nrCreated.load()}, | ||
| {"ExprOpAnd", ExprOpAnd::nrCreated.load()}, | ||
| {"ExprOpOr", ExprOpOr::nrCreated.load()}, | ||
| {"ExprOpImpl", ExprOpImpl::nrCreated.load()}, | ||
| {"ExprOpConcatLists", ExprOpConcatLists::nrCreated.load()}, | ||
| {"ExprOpUpdate", ExprOpUpdate::nrCreated.load()}, |
There was a problem hiding this comment.
Sounds like this could really benefit from some macro like NIX_VALUE_STORAGE_FOR_EACH_FIELD to make listing all the expr types less error-prone.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
It's useful to know which Exprs are getting made, to know where to focus attention, and to sanity-check how much memory we save by different optimizations. It turns out for a NixOS config evaluation, the top-used Exprs are:
Exprconstructor and therefore increment the total.I know ExprInheritFroms also count as ExprVars, but ExprInheritFroms didn't even make it into the top 5 list, so that doesn't explain the discrepancy.
Context
Relevant to my work tracked in #14088
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.