perf(ast): place NodeId field after Span in structs#20584
Merged
graphite-app[bot] merged 1 commit intomainfrom Mar 20, 2026
Merged
Conversation
This was referenced Mar 20, 2026
Member
Author
This was referenced Mar 20, 2026
Merging this PR will not alter performance
Comparing Footnotes
|
2dfbe32 to
cc4867c
Compare
b258884 to
2064ade
Compare
Contributor
Merge activity
|
Alter the code in `ast_tools` which re-orders struct fields, to make `NodeId` always occupy a consistent "slot" in every AST node - at byte position 8, just after `Span`. This will be a sizeable gain once we start utilizing the `NodeId` fields more. Because, for example, every variant of `Expression` has its `NodeId` stored in same location as all the other variants, `Expression::node_id()` is just 1 operation - a pointer read - rather than a nest of branches, or a lookup table. Also alter the algorithm for ordering struct fields to fill in the 4-byte gap after `NodeId` field with other field(s), to avoid excess padding. The new algorithm also prioritizes keeping fields in definition order as much as possible, rather than sorting them strictly in order of size and alignment. This is mildly advantageous because field definition order is the order the AST is walked in, so it avoids bouncing between cache lines while iterating through the fields of a struct when visiting the AST. No types change size in the process. Fields remain packed to keep type sizes the minimum they can be - they just change order.
cc4867c to
e984666
Compare
2064ade to
c6ea0a0
Compare
graphite-app bot
pushed a commit
that referenced
this pull request
Mar 20, 2026
Now that `AstKind`'s variants are all structs (no enums any more), and #20584 has ensured that `Span` is in a consistent position in all structs, `AstKind::span` should boil down to a single operation - a pointer read. Therefore, mark it `#[inline]`. Note: `Span` was already in a consistent position prior to #20584, but that was more by accident than anything else. Now it's guaranteed. But this change won't make a difference to perf, it just guards against a perf regression in future.
graphite-app bot
pushed a commit
that referenced
this pull request
Mar 20, 2026
Follow-on after #20584. Simplify implementation of struct field ordering in `ast_tools`. This change does not alter any layouts, just shortens the code by taking a simpler approach to handling ZST fields.
Base automatically changed from
om/02-15-refactor_ast_re-order_layout_assertions_in_memory_order
to
main
March 20, 2026 22:28
costajohnt
pushed a commit
to costajohnt/oxc
that referenced
this pull request
Mar 22, 2026
…ods (oxc-project#20582) Pure refactor. Memory layout calculation was implemented as a series of free functions. Convert them to a `struct` with methods, so they can pass data between them in `self` rather than many function params. This prepares the way for the changes in oxc-project#20584.
costajohnt
pushed a commit
to costajohnt/oxc
that referenced
this pull request
Mar 22, 2026
…20584) Alter the code in `ast_tools` which re-orders struct fields, to make `NodeId` always occupy a consistent "slot" in every AST node - at byte position 8, just after `Span`. This will be a sizeable gain once we start utilizing the `NodeId` fields more. Because, for example, every variant of `Expression` has its `NodeId` stored in same location as all the other variants, `Expression::node_id()` is just 1 operation - a pointer read - rather than a nest of branches, or a lookup table. Also alter the algorithm for ordering struct fields to fill in the 4-byte gap after `NodeId` field with other field(s), to avoid excess padding. The new algorithm also prioritizes keeping fields in definition order as much as possible, rather than sorting them strictly in order of size and alignment. This is mildly advantageous because field definition order is the order the AST is walked in, so it avoids bouncing between cache lines while iterating through the fields of a struct when visiting the AST. No types change size in the process. Fields remain packed to keep type sizes the minimum they can be - they just change order.
costajohnt
pushed a commit
to costajohnt/oxc
that referenced
this pull request
Mar 22, 2026
Now that `AstKind`'s variants are all structs (no enums any more), and oxc-project#20584 has ensured that `Span` is in a consistent position in all structs, `AstKind::span` should boil down to a single operation - a pointer read. Therefore, mark it `#[inline]`. Note: `Span` was already in a consistent position prior to oxc-project#20584, but that was more by accident than anything else. Now it's guaranteed. But this change won't make a difference to perf, it just guards against a perf regression in future.
costajohnt
pushed a commit
to costajohnt/oxc
that referenced
this pull request
Mar 22, 2026
) Follow-on after oxc-project#20584. Simplify implementation of struct field ordering in `ast_tools`. This change does not alter any layouts, just shortens the code by taking a simpler approach to handling ZST fields.
This was referenced Mar 23, 2026
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
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.

Alter the code in
ast_toolswhich re-orders struct fields, to makeNodeIdalways occupy a consistent "slot" in every AST node - at byte position 8, just afterSpan.This will be a sizeable gain once we start utilizing the
NodeIdfields more. Because, for example, every variant ofExpressionhas itsNodeIdstored in same location as all the other variants,Expression::node_id()is just 1 operation - a pointer read - rather than a nest of branches, or a lookup table.Also alter the algorithm for ordering struct fields to fill in the 4-byte gap after
NodeIdfield with other field(s), to avoid excess padding.The new algorithm also prioritizes keeping fields in definition order as much as possible, rather than sorting them strictly in order of size and alignment. This is mildly advantageous because field definition order is the order the AST is walked in, so it avoids bouncing between cache lines while iterating through the fields of a struct when visiting the AST.
No types change size in the process. Fields remain packed to keep type sizes the minimum they can be - they just change order.