Skip to content

Conversation

@Dunqing
Copy link
Member

@Dunqing Dunqing commented Nov 7, 2025

This PR aims to replace the Formatter's standard Vec and Box usages with ArenaVec and ArenaBox to gain performance, which could reduce the drop overhead.

There are three key changes:

  1. Change the Document's element from Vec<FormatElement<'a>>
pub struct Document<'a> {
-    elements: Vec<FormatElement<'a>>,
+    elements: &'a [FormatElement<'a>],
}
  1. Change the FormatElement::interned's argument from Rc<[FormatElement<'a>]> to &'a [FormatElement<'a>]
- pub struct Interned<'a>(Rc<[FormatElement<'a>]>);
+ pub struct Interned<'a>(&'a [FormatElement<'a>]);
  1. Change the FormatElement::variants's variants from Box<[Box<[FormatElement<'a>]>]> to &'a [&'a [FormatElement<'a>]]
pub struct BestFittingElement<'a> {
-    variants: Box<[Box<[FormatElement<'a>]>]>,
+    variants: &'a [&'a [FormatElement<'a>]],
}

All other changes stem from those mentioned above.

Replacing the Rc with &a reference approach was suggested and guided by @overlookmotel. Thank you!

@github-actions github-actions bot added the A-formatter Area - Formatter label Nov 7, 2025
Copy link
Member Author

Dunqing commented Nov 7, 2025


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.

@github-actions github-actions bot added the C-performance Category - Solution not expected to change functional behavior, only performance label Nov 7, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 7, 2025

CodSpeed Performance Report

Merging #15420 will improve performances by 6.61%

Comparing 11-07-perf_formatter_use_arenavec_and_arenabox (28f5485) with main (e615162)1

Summary

⚡ 8 improvements
✅ 30 untouched
⏩ 7 skipped2

Benchmarks breakdown

Mode Benchmark BASE HEAD Change
Simulation formatter[App.tsx] 56.9 ms 53.4 ms +6.46%
Simulation formatter[RadixUIAdoptionSection.jsx] 520.6 µs 499.2 µs +4.27%
Simulation formatter[Search.tsx] 2.1 ms 2 ms +4.87%
Simulation formatter[core.js] 2.5 ms 2.4 ms +4.86%
Simulation formatter[errors.ts] 862.6 µs 826.5 µs +4.38%
Simulation formatter[handle-comments.js] 3.9 ms 3.8 ms +4.81%
Simulation formatter[index.tsx] 5.2 ms 4.9 ms +6.61%
Simulation formatter[next.ts] 3.2 ms 3 ms +5.97%

Footnotes

  1. No successful run was found on main (d98f3e5) during the generation of this report, so e615162 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 7 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.

@Dunqing Dunqing force-pushed the 11-07-perf_formatter_reuse_previous_indent_stack_in_fitsmeasurer branch from 6084899 to 86f714d Compare November 7, 2025 10:14
@Dunqing Dunqing force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from 6c2d823 to 22e9152 Compare November 7, 2025 10:14
@graphite-app graphite-app bot changed the base branch from 11-07-perf_formatter_reuse_previous_indent_stack_in_fitsmeasurer to graphite-base/15420 November 7, 2025 10:21
@graphite-app graphite-app bot force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from 22e9152 to c67a54a Compare November 7, 2025 10:27
@graphite-app graphite-app bot force-pushed the graphite-base/15420 branch from 86f714d to 29f35b2 Compare November 7, 2025 10:27
@graphite-app graphite-app bot changed the base branch from graphite-base/15420 to main November 7, 2025 10:28
@graphite-app graphite-app bot force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from c67a54a to 9f99f78 Compare November 7, 2025 10:28
@Dunqing Dunqing force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch 4 times, most recently from a0fe000 to 03bf15b Compare November 10, 2025 09:11
@Dunqing Dunqing force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch 2 times, most recently from d6ebbbd to 5463ad4 Compare November 17, 2025 07:42
@Dunqing Dunqing changed the base branch from main to graphite-base/15420 November 17, 2025 07:44
@Dunqing Dunqing changed the base branch from graphite-base/15420 to main November 17, 2025 07:44
@Dunqing Dunqing force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from 5463ad4 to a183ddb Compare November 17, 2025 08:16
@Dunqing Dunqing changed the base branch from main to graphite-base/15420 November 17, 2025 08:29
@Dunqing Dunqing force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from a183ddb to 1424803 Compare November 17, 2025 08:29
@Dunqing Dunqing changed the base branch from graphite-base/15420 to 11-17-feat_allocator_vec_add_vec_into_bump_slice_method November 17, 2025 08:30
@Dunqing Dunqing force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from 1424803 to dfdcbb5 Compare November 17, 2025 08:50
@Dunqing Dunqing marked this pull request as ready for review November 17, 2025 13:02
@Dunqing Dunqing requested a review from leaysgur November 17, 2025 13:02
@graphite-app graphite-app bot changed the base branch from 11-17-feat_allocator_vec_add_vec_into_bump_slice_method to graphite-base/15420 November 17, 2025 13:21
@graphite-app graphite-app bot force-pushed the graphite-base/15420 branch from be713e8 to 102365d Compare November 17, 2025 13:27
@graphite-app graphite-app bot force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from dfdcbb5 to b38764e Compare November 17, 2025 13:27
@graphite-app graphite-app bot changed the base branch from graphite-base/15420 to main November 17, 2025 13:28
@graphite-app graphite-app bot force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from b38764e to a7c9340 Compare November 17, 2025 13:28
Copilot AI review requested due to automatic review settings November 18, 2025 02:31
@Dunqing Dunqing force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from a7c9340 to f0e48a1 Compare November 18, 2025 02:31
Copilot finished reviewing on behalf of Dunqing November 18, 2025 02:35
@Dunqing Dunqing force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from f0e48a1 to 28f5485 Compare November 18, 2025 02:39
@leaysgur leaysgur added the 0-merge Merge with Graphite Merge Queue label Nov 18, 2025
Copy link
Member

leaysgur commented Nov 18, 2025

Merge activity

Copy link
Contributor

Copilot AI left a comment

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 optimizes the formatter by replacing standard Vec and Box with arena-allocated ArenaVec and ArenaBox to reduce drop overhead and improve performance. The key changes include converting Document elements from Vec<FormatElement<'a>> to &'a [FormatElement<'a>], changing Interned from Rc<[FormatElement<'a>]> to &'a [FormatElement<'a>], and updating BestFittingElement variants to use arena-allocated slices.

  • Replace reference-counted and heap-allocated collections with arena-allocated equivalents throughout the formatter
  • Update hash and equality implementations to work with raw pointers instead of Rc
  • Thread allocator references through method signatures where needed for arena allocation

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/oxc_formatter/src/write/jsx/child_list.rs Converts JSX child list builders to use ArenaVec and adds allocator parameters
crates/oxc_formatter/src/write/call_arguments.rs Replaces into_boxed_slice() with into_bump_slice() and uses ArenaVec::from_array_in()
crates/oxc_formatter/src/lib.rs Adds allocator parameter to transform method signature
crates/oxc_formatter/src/ir_transform/sort_imports/source_line.rs Updates write method signature to accept ArenaVec
crates/oxc_formatter/src/ir_transform/sort_imports/mod.rs Threads allocator parameter through transform and uses ArenaVec for element storage
crates/oxc_formatter/src/formatter/formatter.rs Updates allocator() return type and intern_vec to work with ArenaVec
crates/oxc_formatter/src/formatter/format_element/mod.rs Converts Interned from Rc to reference and updates BestFittingElement to use arena slices
crates/oxc_formatter/src/formatter/format_element/document.rs Changes Document to store &'a [FormatElement<'a>] instead of Vec
crates/oxc_formatter/src/formatter/builders.rs Updates BestFitting implementation to create arena-allocated variants
crates/oxc_formatter/src/formatter/buffer.rs Converts VecBuffer to use ArenaVec and updates buffer operations with take_in()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

This PR aims to replace the Formatter's standard `Vec` and `Box` usages with `ArenaVec` and `ArenaBox` to gain performance, which could reduce the `drop` overhead.

There are three key changes:

1. Change the Document's element from `Vec<FormatElement<'a>>`
```diff
pub struct Document<'a> {
-    elements: Vec<FormatElement<'a>>,
+    elements: &'a [FormatElement<'a>],
}
```

2. Change the `FormatElement::interned`'s argument from `Rc<[FormatElement<'a>]>` to `&'a [FormatElement<'a>]`
```diff
- pub struct Interned<'a>(Rc<[FormatElement<'a>]>);
+ pub struct Interned<'a>(&'a [FormatElement<'a>]);
```

3. Change the `FormatElement::variants`'s variants from `Box<[Box<[FormatElement<'a>]>]>` to `&'a [&'a [FormatElement<'a>]]`
```diff
pub struct BestFittingElement<'a> {
-    variants: Box<[Box<[FormatElement<'a>]>]>,
+    variants: &'a [&'a [FormatElement<'a>]],
}
```

All other changes stem from those mentioned above.

Replacing the `Rc` with `&a` reference approach was suggested and guided by @overlookmotel. Thank you!
@graphite-app graphite-app bot force-pushed the 11-07-perf_formatter_use_arenavec_and_arenabox branch from 3b9c55d to 4fe3aac Compare November 18, 2025 03:44
@graphite-app graphite-app bot merged commit 4fe3aac into main Nov 18, 2025
20 checks passed
@graphite-app graphite-app bot deleted the 11-07-perf_formatter_use_arenavec_and_arenabox branch November 18, 2025 03:50
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Nov 18, 2025
overlookmotel added a commit that referenced this pull request Nov 24, 2025
# Oxlint
### 💥 BREAKING CHANGES

- cbb27fd ast: [**BREAKING**] Add `TSGlobalDeclaration` type (#15712)
(overlookmotel)

### 🚀 Features

- 72660f7 linter: Support auto generate config document for tuple lint
option (#15904) (Duc Nghiem Xuan)
- 0c1f82b linter/plugins: Add `tokens` property to `Program` (#16020)
(overlookmotel)
- 9e61beb linter/plugins: Implement `SourceCode#getFirstToken()`
(#16002) (Arsh)
- 9a548dd linter/plugins: Implement `SourceCode#getLastTokens()`
(#16000) (Arsh)
- 0b6cb11 linter/plugins: Implement `SourceCode#getFirstTokens()`
(#15976) (Arsh)
- 166781e linter/plugins: Implement `SourceCode#getTokenAfter()`
(#15973) (Arsh)
- 6ae232b linter: Expose type errors via tsgolint (#15917) (camc314)
- 2bfdd26 linter/plugins: Implement `SourceCode#getTokensAfter()`
(#15971) (Arsh)
- 45fffc1 linter/plugins: Implement `SourceCode#getTokenBefore()`
(#15956) (Arsh)
- 776e473 linter/plugins: Implement `SourceCode#getTokensBefore()`
(#15955) (Arsh)
- 595867a oxlint: Generate markdownDescription fields for oxlint JSON
schema. (#15959) (connorshea)
- 5569317 vscode: Add quick actions to status bar tooltip (#15962)
(Sysix)
- 986cac1 linter/plugins: Token-related `SourceCode` APIs (TS ESLint
implementation) (#15861) (Arsh)
- a21f9e4 linter: Implement unicorn/prefer-bigint-literals rule (#15923)
(Michiel Vrins)
- 4b9d8d2 linter/type-aware: Include range with tsconfig diagnostics
(#15916) (camc314)
- 220d01e editor: Improve the status bar item for the VS Code extension
by adding a tooltip. (#15819) (connorshea)

### 🐛 Bug Fixes

- 2bd3cb6 apps, editors, napi: Fix `oxlint-disable` comments (#16014)
(overlookmotel)
- 81f5360 linter/prefer-number-properties: Get fixer to replace entire
call expr (#15979) (camc314)
- e4ba07f language_server: Always write to memory file system (#15975)
(Sysix)
- a8a2032 linter: Support missing `range` for internal diagnostics
(#15964) (camc314)
- 619a226 oxlint/lsp: Don't register `textDocument/formatting`
capability (#15882) (Sysix)
- 6ab1a20 linter: Fix no_useless_spread producing invalid syntax when
removing empty object spreads (#15905) (camc314)
- be4b6df linter: Unicorn/prefer-string-replace-all incorrectly escapes
backslashes (#15901) (camc314)
- 9fa9ef2 linter: Gracefully fail when using import plugin, large file
counf and JS plugins (#15864) (camc314)
- c027398 linter/plugins: Correct bindings package names glob in TSDown
config (#15871) (overlookmotel)
- b622ef8 linter: Fix `oxc/bad_array_method_on_arguments` rule behavior.
(#15854) (connorshea)
- aa06c3f linter: Recognize NewExpression as value context in
no-unused-private-class-members (#15843) (camc314)
- e89c5ba typescript/prefer-namespace-keyword: Skip nested
`TSModuleDeclaration`s (#15806) (overlookmotel)
- 646d020 linter/exhaustive-dependencies: Prevent is_callee_of_call_expr
flag from leaking into nested expressions (#15832) (camc314)
- 46bd6bd linter/plugins: Pin `@typescript-eslint/scope-manager`
dependency (#15807) (overlookmotel)
- fba31fa linter: Patch `@typescript-eslint/scope manager` (#15214)
(Arsh)
- 50307c1 linter/jest: Ignore `expect` identifier in argument position
(#15785) (camc314)

### ⚡ Performance

- 024b48a linter/plugins: Lazy-load tokens parsing code (#16011)
(overlookmotel)
- 15365c9 linter/plugins: Reduce var assignments (#15953)
(overlookmotel)
- 84d1f4f linter/plugins: Downgrade some checks to debug-only (#15922)
(overlookmotel)
- a49f704 linter/typescript: Avoid searching source text unless required
(#15805) (overlookmotel)

### 📚 Documentation

- ceffa5a linter: Add config option docs for various rules. (#16024)
(connorshea)
- 9a0ed13 linter: Fix config option docs for eslint/operator-assignment
rule. (#16030) (connorshea)
- 0b18005 linter: Add config docs generation for rules with Regex
arguments (#15978) (connorshea)
- 48d18e0 linter: Improve diagnostic message for promise/catch-or-return
rule (#15980) (connorshea)
- 6c72e84 linter: Use backticks for code elements across more rule
diagnostics (#15958) (connorshea)
- a63dad7 linter/plugins: Add comment (#15952) (overlookmotel)
- 81ea642 vscode: Use markdownDescription for better formatting in
VSCode Settings (#15889) (connorshea)
- db6a110 linter/plugins: Fix JSDoc comment (#15884) (overlookmotel)
- 1487271 linter: Add config option docs for `jsdoc/require-param` and
`jsdoc/require-returns` rules (#15857) (connorshea)
- fbf0fd4 linter/plugins: Add JSDoc comments to `Plugin` and `Rule`
types (#15815) (overlookmotel)
- ac5e4b5 linter/plugins: Add JSDoc comments and improve comments
(#15814) (overlookmotel)
- 9b7b083 linter: Fix error in `curly` `"all"` example (#15801)
(camc314)
- 65a3520 linter: Improve diagnostic for consistent-type-definitions
rule. (#15752) (connorshea)

### 🛡️ Security

- f9b9276 deps: Update dependency rolldown to v1.0.0-beta.51 (#15856)
(renovate[bot])
# Oxfmt
### 💥 BREAKING CHANGES

- a937890 formatter: [**BREAKING**] Default to `lineWidth: 100` (#15933)
(leaysgur)
- 03d5f5a formatter/sort-imports: [**BREAKING**] Change default order to
`natural` with `natord` crate (#15828) (leaysgur)
- cbb27fd ast: [**BREAKING**] Add `TSGlobalDeclaration` type (#15712)
(overlookmotel)

### 🚀 Features

- 7818e22 formatter/sort-imports: Support `options.groups` (#15831)
(leaysgur)
- f9a502c oxfmt: `oxfmt --lsp` support (#15765) (leaysgur)

### 🐛 Bug Fixes

- 4817486 formatter: Revert `FormatElement::BestFitting` printing logic
(#16028) (Dunqing)
- 5562dd6 formatter: Incorrect formatting method chain with trailing
comments (#16027) (Dunqing)
- 6d14c8b formatter: Comments in export class decorators are printing
incorrectly (#15897) (Dunqing)
- 683c764 formatter: Correct a few minor mismatched typescript tests
(#15894) (Dunqing)
- c11cc07 formatter: Improve formatting for default type on type
parameters (#15893) (Dunqing)
- 0bff596 formatter: Handle JSX expresssion dangling comment (#15890)
(leaysgur)
- 16a9dc8 formatter: Inconsistent printing of class extends and
interface extends (#15892) (Dunqing)
- 300b496 formatter: Inconsistent CallExpression and NewExpression
around member chain and logical expression (#15858) (Dunqing)

### ⚡ Performance

- 65174cc formatter: Reduce the size of `TextWidth` to 4 byte (#15827)
(Dunqing)
- 4fe3aac formatter: Use `ArenaVec` and `ArenaBox` (#15420) (Dunqing)

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

Labels

A-formatter Area - Formatter 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.

3 participants