Skip to content

Conversation

@Radvendii
Copy link
Contributor

@Radvendii Radvendii commented Sep 30, 2025

Motivation

For big picture motivation, see the tracking issue

This PR moves the data from the vector ExprSelect::attrPath into our Exprs arena, and then refers to it by an AttrName * and a uint32_t length.

  • Removes 12 bytes per ExprSelect by going from std::vector (24 bytes) to AttrName * (8 bytes) and uint32_t (length)
  • Removes additional 4 byte per ExprSelect of padding (PosIdx was followed by 4 bytes of padding)

This change (and several to follow) are not required for indexification of Expr *s or cleaning up parsing leaks, but I thought it would be nice to finish up these changes first so Exprs are not left in a half-way changed state.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@Radvendii Radvendii force-pushed the exprselect-alloc branch 2 times, most recently from 112b667 to e5c50bf Compare October 3, 2025 15:48
@Radvendii Radvendii changed the title libexpr: allocate ExprSelect's AttrName vector in Expr::alloc libexpr: allocate ExprSelect's AttrPath in Expr::alloc Oct 3, 2025
Copy link
Contributor

@xokdvium xokdvium left a comment

Choose a reason for hiding this comment

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

Some nitpicks. With those fixed and squashed looks will be good to go

@xokdvium xokdvium merged commit 7582682 into NixOS:master Oct 3, 2025
15 checks passed
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.

3 participants