Skip to content

perf(parser): allocate TriviaBuilder comments in the arena#21512

Merged
graphite-app[bot] merged 1 commit intomainfrom
perf/trivia-builder-arena-vec
Apr 23, 2026
Merged

perf(parser): allocate TriviaBuilder comments in the arena#21512
graphite-app[bot] merged 1 commit intomainfrom
perf/trivia-builder-arena-vec

Conversation

@Boshen
Copy link
Copy Markdown
Member

@Boshen Boshen commented Apr 16, 2026

Summary

Thread the arena lifetime into TriviaBuilder and store comments in an oxc_allocator::Vec instead of std::Vec. At the end of parse, comments are moved directly into program.comments, eliminating the finalize-time vec_from_iter copy.

Sys reallocs drop across the benchmark files:

File Sys reallocs (before → after)
checker.ts 21 → 10
cal.com.tsx 49 → 41
pdf.mjs 75 → 67
antd.js 235 → 221
binder.ts 7 → 0

Arena reallocs tick up slightly to absorb the comment pushes; arena bytes unchanged.

irregular_whitespaces stays as std::Vec — it's almost always empty in practice, and its Box<[Span]> escape would need a heap copy at finalize either way.

@github-actions github-actions Bot added A-parser Area - Parser C-performance Category - Solution not expected to change functional behavior, only performance labels Apr 16, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 16, 2026

Merging this PR will not alter performance

✅ 48 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing perf/trivia-builder-arena-vec (9d57fe4) with main (d2b9389)

Open in CodSpeed

Footnotes

  1. 3 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 force-pushed the perf/trivia-builder-arena-vec branch from 420e52a to 9d57fe4 Compare April 23, 2026 15:02
Copy link
Copy Markdown
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

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

Great!

I've rebased on latest main to fix merge conflicts.

Side note: The comment about irregular_whitespaces is not quite correct. Converting an ArenaVec<'a, T> to Box<'a, [T]> is free (no mem copy, no reallocation), as it doesn't need to free the spare capacity - unlike with heap-allocated Vec<T> -> Box<[T]> which does have to reallocate.

@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Apr 23, 2026
Copy link
Copy Markdown
Member

overlookmotel commented Apr 23, 2026

Merge activity

## Summary

Thread the arena lifetime into `TriviaBuilder` and store `comments` in an `oxc_allocator::Vec` instead of `std::Vec`. At the end of `parse`, comments are moved directly into `program.comments`, eliminating the finalize-time `vec_from_iter` copy.

Sys reallocs drop across the benchmark files:

| File | Sys reallocs (before → after) |
| --- | --- |
| checker.ts | 21 → 10 |
| cal.com.tsx | 49 → 41 |
| pdf.mjs | 75 → 67 |
| antd.js | 235 → 221 |
| binder.ts | 7 → 0 |

Arena reallocs tick up slightly to absorb the comment pushes; arena bytes unchanged.

`irregular_whitespaces` stays as `std::Vec` — it's almost always empty in practice, and its `Box<[Span]>` escape would need a heap copy at finalize either way.
@graphite-app graphite-app Bot force-pushed the perf/trivia-builder-arena-vec branch from 9d57fe4 to b179688 Compare April 23, 2026 15:22
@graphite-app graphite-app Bot merged commit b179688 into main Apr 23, 2026
28 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Apr 23, 2026
@graphite-app graphite-app Bot deleted the perf/trivia-builder-arena-vec branch April 23, 2026 15:30
camc314 pushed a commit that referenced this pull request Apr 27, 2026
### 💥 BREAKING CHANGES

- 502e804 ast: [**BREAKING**] Reduce size of `TSTypePredicateName`
(#21711) (overlookmotel)
- 5651539 ast: [**BREAKING**] Reduce size of `JSXExpression` (#21710)
(overlookmotel)
- c44e280 ast: [**BREAKING**] Reduce size of `ArrayExpressionElement`
(#21709) (overlookmotel)
- c5b3deb syntax: [**BREAKING**] Remove `CommentNodeId` (#21679)
(overlookmotel)

### 🚀 Features

- b738a39 allocator: Add `Allocator::cursor_ptr` method (#21773)
(overlookmotel)
- 678767e ast: Generate node_id accessors for AST enum wrappers (#21653)
(camc314)
- f091d77 minifier: Inline constant spread elements into arrays (#21095)
(Armano)

### 🐛 Bug Fixes

- 0d608c2 minifier: Preserve raw CR in template literals (#21645)
(Dunqing)
- a889ea9 minifier: Track pure functions in DCE mode (#21722) (Dunqing)
- 674dfac allocator: `Arena` retry allocation when chunk size approaches
maximum (#21777) (overlookmotel)
- f130cc0 allocator: Fix arithmetic overflow in
`Arena::new_chunk_memory_details` (#21745) (overlookmotel)
- b9bf239 allocator: Fix UB in `Arena::grow_zeroed` (#21739)
(overlookmotel)
- d2b9389 allocator: Clippy warning when building without `testing`
feature (#21681) (camc314)
- 503dc86 codegen: Map sourcemaps from visible output starts (#21662)
(Dunqing)
- c92bd3b transformer: Use SPAN for synthesized helper calls to prevent
comment misattribution (#21578) (Dunqing)
- 0d80441 codegen: Add mapping before printing `#` for private ident
(#21619) (camc314)

### ⚡ Performance

- 9fa362e napi/parser: Do not generate tokens except in tests (#21811)
(overlookmotel)
- 0044392 allocator: Reduce branches when allocating new chunk (#21776)
(overlookmotel)
- 7896bd0 allocator: `Allocator::used_bytes` do not use chunk iterator
(#21771) (overlookmotel)
- a5c562f allocator: Remove check in `Arena::new_chunk_memory_details`
(#21750) (overlookmotel)
- 35bbe1f allocator: `Arena` use unchecked size round up where
guaranteed no overflow (#21743) (overlookmotel)
- ffe229b allocator: Remove unnecessary check from
`Arena::try_alloc_layout_slow_impl` (#21732) (overlookmotel)
- 72fece5 allocator: Use `NonNull::offset_from_unsigned` in
`Arena::chunk_capacity` (#21731) (overlookmotel)
- cab32ae ast: Add `#[inline(always)]` to `node_id` methods on enums
with all variants unboxed (#21707) (overlookmotel)
- b179688 parser: Allocate `TriviaBuilder` comments in the arena
(#21512) (Boshen)
- 2290f31 lexer: Fix perf of `Token::set_*` methods on Rust 1.95.0
(#21659) (overlookmotel)
- 1b58029 allocator: Move code into cold path in `Arena::alloc_layout`
(#21622) (overlookmotel)
- 3cf7cef allocator: Reduce instructions on allocation hot path (#21510)
(overlookmotel)

### 📚 Documentation

- ce65070 data_structures: Document why `as_ref` and `as_mut` on
`NonNullConst` and `NonNullMut` take `self` (#21800) (overlookmotel)
- 93b7dbd allocator: Improve doc comments for `ChunkFooter` (#21733)
(overlookmotel)
- 295db8d transformer: Fix comment (#21717) (overlookmotel)
- 5c93af8 ast: Add comments explaining `#[inline(always)]` to `node_id`
methods on enums (#21706) (overlookmotel)
- e4cea25 transform: Use the `node:` namespace in the example (#19998)
(루밀LuMir)

### 🛡️ Security

- d8076c9 deps: Update rolldown (#21639) (renovate)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-parser Area - Parser 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