Skip to content

perf(minifier): allocate AST nodes in arena directly#23710

Merged
graphite-app[bot] merged 1 commit into
mainfrom
om/06-21-perf_minifier_allocate_ast_nodes_in_arena_directly
Jun 23, 2026
Merged

perf(minifier): allocate AST nodes in arena directly#23710
graphite-app[bot] merged 1 commit into
mainfrom
om/06-21-perf_minifier_allocate_ast_nodes_in_arena_directly

Conversation

@overlookmotel

@overlookmotel overlookmotel commented Jun 22, 2026

Copy link
Copy Markdown
Member

Same as #23709.

Small perf optimizations around AST builder calls.

  • AST nodes which end up in Boxes, allocate into the Box as early as possible, to increase chance compiler sees the type can be built directly in arena, rather than built on the stack, and then copied from stack into arena.
  • Functions return Box<T> rather than T where the value needs to be boxed anyway. If function is not inlined, this avoids stack allocation + copy.

Also, shorten code where possible by using more specific AstBuilder methods.

overlookmotel commented Jun 22, 2026

Copy link
Copy Markdown
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 changes, fast-track this PR to the front of the merge queue

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

codspeed-hq Bot commented Jun 22, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 52 untouched benchmarks
⏩ 19 skipped benchmarks1


Comparing om/06-21-perf_minifier_allocate_ast_nodes_in_arena_directly (86b4184) with om/06-21-perf_isolated_declarations_allocate_ast_nodes_in_arena_directly (2e9ab4d)

Open in CodSpeed

Footnotes

  1. 19 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@overlookmotel overlookmotel marked this pull request as ready for review June 22, 2026 12:40
@overlookmotel overlookmotel requested a review from Dunqing as a code owner June 22, 2026 12:40
Copilot AI review requested due to automatic review settings June 22, 2026 12:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR applies small performance-oriented refactors in oxc_minifier to build AST nodes directly into arena-allocated Boxes earlier and to use more specific AstBuilder convenience helpers, reducing stack construction/copies and shortening call sites.

Changes:

  • Update AST construction call sites to use alloc_* builder methods directly (e.g., literals, object/block expressions).
  • Replace manual BlockStatement boxing patterns with statement_block_with_scope_id / alloc_block_statement helpers.
  • Simplify a minifier test to avoid an unnecessary clone_in when the vector is consumed.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
crates/oxc_minifier/tests/ecmascript/array_join.rs Uses arena-allocating literal/object builders and removes an unnecessary clone_in when constructing the test array AST.
crates/oxc_minifier/src/peephole/remove_dead_code.rs Swaps Box<BlockStatement> directly using alloc_block_statement, avoiding stack construction + re-allocation for the extracted finalizer block.
crates/oxc_minifier/src/peephole/minimize_statements.rs Replaces manual block boxing with statement_block_with_scope_id helper for cleaner/faster block statement creation.
crates/oxc_minifier/src/peephole/minimize_if_statement.rs Uses statement_block_with_scope_id to wrap a consequent in a scoped block without manual allocation/boxing.

@graphite-app graphite-app Bot force-pushed the om/06-21-perf_isolated_declarations_allocate_ast_nodes_in_arena_directly branch 2 times, most recently from 23c2d47 to 3e5ef15 Compare June 22, 2026 12:55
@graphite-app graphite-app Bot force-pushed the om/06-21-perf_minifier_allocate_ast_nodes_in_arena_directly branch from 25120f4 to 644b3d0 Compare June 22, 2026 12:55
@graphite-app

graphite-app Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Merge activity

Same as #23709.

Small perf optimizations around AST builder calls.

- AST nodes which end up in `Box`es, allocate into the `Box` as early as possible, to increase chance compiler sees the type can be built directly in arena, rather than built on the stack, and then copied from stack into arena.
- Functions return `Box<T>` rather than `T` where the value needs to be boxed anyway. If function is not inlined, this avoids stack allocation + copy.

Also, shorten code where possible by using more specific `AstBuilder` methods.
@graphite-app graphite-app Bot force-pushed the om/06-21-perf_isolated_declarations_allocate_ast_nodes_in_arena_directly branch from 2e9ab4d to d025887 Compare June 23, 2026 01:00
@graphite-app graphite-app Bot force-pushed the om/06-21-perf_minifier_allocate_ast_nodes_in_arena_directly branch from 86b4184 to 3855f0c Compare June 23, 2026 01:00
Base automatically changed from om/06-21-perf_isolated_declarations_allocate_ast_nodes_in_arena_directly to main June 23, 2026 01:04
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jun 23, 2026
@graphite-app graphite-app Bot merged commit 3855f0c into main Jun 23, 2026
31 checks passed
@graphite-app graphite-app Bot deleted the om/06-21-perf_minifier_allocate_ast_nodes_in_arena_directly branch June 23, 2026 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants