Skip to content

perf(ast): re-order struct fields to reduce padding#11056

Merged
graphite-app[bot] merged 1 commit intomainfrom
05-15-perf_ast_re-order_struct_fields_to_reduce_padding
May 16, 2025
Merged

perf(ast): re-order struct fields to reduce padding#11056
graphite-app[bot] merged 1 commit intomainfrom
05-15-perf_ast_re-order_struct_fields_to_reduce_padding

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented May 15, 2025

All our AST structs are #[repr(C)], which gives us guaranteed stable memory layouts. This is essential for raw transfer, and helpful for other purposes too.

However, the downside is that we lose out on the automatic field-reordering that default #[repr(Rust)] provides, where structs are packed to minimize padding bytes.

This PR implements a similar field-reordering scheme, to get the same effect.

  • oxc_ast_tools determines optimal field order for compact memory layout.
  • oxc_ast_tools generates a file (structs.rs) which details the desired field order for each type.
  • #[ast] proc macro follows the instructions in structs.rs and re-orders the fields.

This reduces the size of various AST types by 8 bytes. No type now has more than 7 padding bytes (previously e.g. Function had 15 bytes of padding).

Possible future improvements:

  1. Get Rust Analyser to display original field order.
  2. Remove syn dependency from oxc_ast_macros crate, to improve compile times. It should be possible to do all the AST manipulation using just proc_macro crate, by generating more code in oxc_ast_tools, so the macro's logic can be simpler.

Note: This PR doesn't add syn dependency, it was already there. But it makes it easier to remove it.

@github-actions github-actions bot added A-ast Area - AST A-ast-tools Area - AST tools C-performance Category - Solution not expected to change functional behavior, only performance labels May 15, 2025
Copy link
Member Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codspeed-hq
Copy link

codspeed-hq bot commented May 15, 2025

CodSpeed Instrumentation Performance Report

Merging #11056 will not alter performance

Comparing 05-15-perf_ast_re-order_struct_fields_to_reduce_padding (2b0a69f) with main (5dcd0f1)

Summary

✅ 36 untouched benchmarks

@overlookmotel overlookmotel force-pushed the 05-15-perf_ast_re-order_struct_fields_to_reduce_padding branch from 791fbbb to f54800c Compare May 15, 2025 14:01
@overlookmotel
Copy link
Member Author

overlookmotel commented May 15, 2025

Perf impact is much less than I'd hoped. Only sub-% movement on parser benchmarks, and 1% on a couple of transformer/minifier benchmarks. Meh!

Still, why not? It's crazy to be using memory for padding bytes. And now we don't risk perf regressions if we rearrange fields or add more fields in future.

There is one quite large annoyance. Rustdoc seems to work from the types after proc macros have run, so now the fields in docs for oxc_ast crate are no longer listed in original order, but in memory layout order. I'll see if I can fix that by injecting #[cfg(doc)] attributes.

@overlookmotel overlookmotel force-pushed the 05-15-perf_ast_re-order_struct_fields_to_reduce_padding branch from f54800c to 00d5da8 Compare May 15, 2025 15:30
@overlookmotel
Copy link
Member Author

The Rustdoc issue is now fixed. Rustdoc now shows struct fields in original source order.

@overlookmotel overlookmotel marked this pull request as ready for review May 15, 2025 16:17
@overlookmotel overlookmotel requested a review from leaysgur as a code owner May 15, 2025 16:17
@overlookmotel overlookmotel requested review from Boshen and removed request for leaysgur May 15, 2025 16:17
@overlookmotel overlookmotel force-pushed the 05-15-perf_ast_re-order_struct_fields_to_reduce_padding branch from 00d5da8 to 7e0b283 Compare May 15, 2025 22:36
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label May 16, 2025
Copy link
Member

Boshen commented May 16, 2025

Merge activity

graphite-app bot pushed a commit that referenced this pull request May 16, 2025
All our AST structs are `#[repr(C)]`, which gives us guaranteed stable memory layouts. This is essential for raw transfer, and helpful for other purposes too.

However, the downside is that we lose out on the automatic field-reordering that default `#[repr(Rust)]` provides, where structs are packed to minimize padding bytes.

This PR implements a similar field-reordering scheme, to get the same effect.

* `oxc_ast_tools` determines optimal field order for compact memory layout.
* `oxc_ast_tools` generates a file (`structs.rs`) which details the desired field order for each type.
* `#[ast]` proc macro follows the instructions in `structs.rs` and re-orders the fields.

This reduces the size of various AST types by 8 bytes. No type now has more than 7 padding bytes (previously e.g. `Function` had 15 bytes of padding).

Possible future improvements:

1. Get Rust Analyser to display original field order.
2. Remove `syn` dependency from `oxc_ast_macros` crate, to improve compile times. It should be possible to do all the AST manipulation using just `proc_macro` crate, by generating more code in `oxc_ast_tools`, so the macro's logic can be simpler.

Note: This PR doesn't add `syn` dependency, it was already there. But it makes it easier to remove it.
@graphite-app graphite-app bot force-pushed the 05-15-perf_ast_re-order_struct_fields_to_reduce_padding branch from 7e0b283 to e3e60eb Compare May 16, 2025 03:42
All our AST structs are `#[repr(C)]`, which gives us guaranteed stable memory layouts. This is essential for raw transfer, and helpful for other purposes too.

However, the downside is that we lose out on the automatic field-reordering that default `#[repr(Rust)]` provides, where structs are packed to minimize padding bytes.

This PR implements a similar field-reordering scheme, to get the same effect.

* `oxc_ast_tools` determines optimal field order for compact memory layout.
* `oxc_ast_tools` generates a file (`structs.rs`) which details the desired field order for each type.
* `#[ast]` proc macro follows the instructions in `structs.rs` and re-orders the fields.

This reduces the size of various AST types by 8 bytes. No type now has more than 7 padding bytes (previously e.g. `Function` had 15 bytes of padding).

Possible future improvements:

1. Get Rust Analyser to display original field order.
2. Remove `syn` dependency from `oxc_ast_macros` crate, to improve compile times. It should be possible to do all the AST manipulation using just `proc_macro` crate, by generating more code in `oxc_ast_tools`, so the macro's logic can be simpler.

Note: This PR doesn't add `syn` dependency, it was already there. But it makes it easier to remove it.
@graphite-app graphite-app bot force-pushed the 05-15-perf_ast_re-order_struct_fields_to_reduce_padding branch from e3e60eb to 2b0a69f Compare May 16, 2025 03:43
@graphite-app graphite-app bot merged commit 2b0a69f into main May 16, 2025
26 checks passed
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label May 16, 2025
@graphite-app graphite-app bot deleted the 05-15-perf_ast_re-order_struct_fields_to_reduce_padding branch May 16, 2025 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-ast-tools Area - AST tools C-performance Category - Solution not expected to change functional behavior, only performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants