Skip to content

Conversation

Radvendii
Copy link
Contributor

@Radvendii Radvendii commented Sep 28, 2025

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:

  • ExprVar (700K) (37%)
  • ExprAttrs (670K) (35%)
  • ExprString (370K) (19%)
  • ExprSelect (200K) (10%)
  • ExprCall (170K) (9%)
  • Total (1.89M)

⚠️ This... adds up to 110%, so we must be double-counting somewhere, but I can't find it. I'm only incrementing the counters in constructors, which should also call the Expr constructor 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.

@Radvendii Radvendii requested a review from edolstra as a code owner September 28, 2025 17:51
@Radvendii Radvendii marked this pull request as draft September 28, 2025 17:51
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()},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose... although there's only 2 places where this happens and macros are error-prone in their own ways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants