Skip to content

perf(parser): defer diagnostic creation until parse exit#24663

Merged
graphite-app[bot] merged 1 commit into
mainfrom
feat/defer-parser-diagnostics
Jul 19, 2026
Merged

perf(parser): defer diagnostic creation until parse exit#24663
graphite-app[bot] merged 1 commit into
mainfrom
feat/defer-parser-diagnostics

Conversation

@Boshen

@Boshen Boshen commented Jul 19, 2026

Copy link
Copy Markdown
Member

Why

This eliminates the eager boxing of diagnostics that are created and then thrown away during speculative parsing (arrow-function / type-vs-expression backtracking). On real-world files — all valid, so nearly every one of these allocations was a speculation-discarded diagnostic

@github-actions github-actions Bot added the A-parser Area - Parser label Jul 19, 2026
@Boshen
Boshen force-pushed the feat/defer-parser-diagnostics branch from 4583845 to 442fb5e Compare July 19, 2026 10:14
@codspeed-hq

codspeed-hq Bot commented Jul 19, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 4.14%

⚡ 4 improved benchmarks
✅ 63 untouched benchmarks
⏩ 9 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation parser[kitchen-sink.tsx] 21.9 ms 20.9 ms +4.67%
Simulation parser[App.tsx] 7.9 ms 7.5 ms +4.24%
Simulation parser[binder.ts] 3.1 ms 3 ms +3.97%
Simulation parser[react.development.js] 1.2 ms 1.2 ms +3.69%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing feat/defer-parser-diagnostics (c2bcf2d) with main (3f03a5b)

Open in CodSpeed

Footnotes

  1. 9 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.

@Boshen
Boshen force-pushed the feat/defer-parser-diagnostics branch from 442fb5e to c2bcf2d Compare July 19, 2026 10:39
@Boshen
Boshen marked this pull request as ready for review July 19, 2026 11:04
@Boshen
Boshen requested a review from overlookmotel as a code owner July 19, 2026 11:04
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jul 19, 2026

Boshen commented Jul 19, 2026

Copy link
Copy Markdown
Member Author

Merge activity

> [!NOTE]
> Draft. Builds on the merged diagnostics cleanup (#24661), and has been through two `/simplify` review passes.
>
> One design point a perf reviewer may ask about: `ParserCheckpoint` (copied on every `checkpoint`/`rewind`/`lookahead`) embeds `FatalError` **inline**, not boxed. Boxing it would shrink the checkpoint from 3 → 2 cache lines, but only by adding a heap allocation — counter to this PR's goal. The deferral already shrank the checkpoint substantially for free: `FatalError` previously held an inline `OxcDiagnostic` (~150 B), now a 72 B POD `ParserDiagnostic`.

Parser diagnostics are now stored in a deferred (unmaterialized) form during parsing and materialized into `OxcDiagnostic` only once, at parse exit.

## Design

A `parser_diagnostics!` macro in `diagnostics.rs` generates, from one entry per diagnostic:
- an enum variant `ParserDiagnostic::Foo { .. }` holding only the POD arguments (`Span`, `&'a str`, `Copy` types);
- a `#[cold] into_diagnostic()` match arm holding the eager construction (box + label strings + `format!`) — all the allocation/formatting work concentrated in one cold function;
- an `#[inline]` constructor with the **same name and signature as before**, so the ~330 `diagnostics::foo(...)` call sites are unchanged.

The parser/lexer error vecs and `FatalError` now carry `ParserDiagnostic<'a>`, so pushing an error, snapshotting the error set on `checkpoint()`, and discarding it on `rewind()` never allocate. Materialization happens at the two `parse()` / `parse_expression()` exit points. Foreign already-materialized diagnostics (from `oxc_regular_expression`) ride an `Eager(Box<OxcDiagnostic>)` escape hatch; the two module-record diagnostics that are built once at exit stay eager.

## Why

This eliminates the eager boxing of diagnostics that are created and then thrown away during speculative parsing (arrow-function / type-vs-expression backtracking). On real-world files — all valid, so nearly every one of these allocations was a speculation-discarded diagnostic:

| file | sys allocs before → after |
|---|---|
| checker.ts | 1818 → 23 |
| antd.js | 3661 → 190 |
| kitchen-sink.tsx | 2051 → 190 |
| App.tsx | 360 → 23 |
| binder.ts | 85 → 1 |

Diagnostic output is byte-identical — parser conformance (Test262, Babel, TypeScript) snapshots are unchanged.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app
graphite-app Bot force-pushed the feat/defer-parser-diagnostics branch from c2bcf2d to bcc9de0 Compare July 19, 2026 11:06
@graphite-app
graphite-app Bot merged commit bcc9de0 into main Jul 19, 2026
32 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jul 19, 2026
@graphite-app
graphite-app Bot deleted the feat/defer-parser-diagnostics branch July 19, 2026 11:10
graphite-app Bot pushed a commit that referenced this pull request Jul 19, 2026
`OxcDiagnostic` is an unboxed ~300-byte struct constructed at thousands of callsites. Boxing its inner data behind an 8-byte handle means every callsite, every `Result<T, OxcDiagnostic>`, and every move copies 8 bytes instead of ~300, so the compiler emits far less inlined construction/copy code.

## Binary size (stripped, real release builds, aarch64-apple-darwin)

| Binary | Before | After | Delta |
|---|---:|---:|---:|
| `oxlint` | 14,177,616 B | 13,665,680 B | **−499.9 KiB (−3.61%)** |
| napi transform (`.node`) | 3,482,624 B | 3,433,040 B | **−48.4 KiB (−1.42%)** |

Most of the win lands in `oxc_linter`, whose ~1,500 diagnostic-construction sites inline into rule bodies; `oxc_react_compiler` also contributes.

The `error`/`warn` constructors delegate to an outlined `#[inline(never)] fn new`, so the `Box` allocation + field initialization exist once in the binary rather than inlined at every construction site.

This reverses #22406, which unboxed the inner to save a heap allocation. That is now safe: the parser no longer constructs diagnostics on its hot path (#24663).

Supersedes the draft #24656, which additionally carried a now-redundant copy of the parser deferral that has since landed via #24663.
camc314 added a commit that referenced this pull request Jul 21, 2026
### 💥 BREAKING CHANGES

- 54cc121 ast: [**BREAKING**] Split `MetaProperty` into `ImportMeta` and
`NewTarget` (#24557) (camc314)

### 🚀 Features

- 4c71560 parser: More friendly error for spread element in dynamic
imports (#24705) (sapphi-red)
- 7b045cd minfier: Drop last break from last switch case (#24673)
(Armano)
- 7d3c178 minifier: Remove unreachable recursive functions (#24125)
(Dunqing)
- 94f99b3 ast: Allow `NONE` to be passed to AST builder methods where
`Option<ArenaVec>` is expected (#24629) (overlookmotel)
- 77230c5 ast: Accept arrays for `ArenaVec` params of AST builder
methods (#24621) (overlookmotel)
- f08b152 allocator: Implement `FromIn` for array to `Vec` conversion
(#24620) (overlookmotel)
- 2338c13 track-memory-allocations: Track heap deallocs, alloc bytes,
and peak growth (#24619) (Boshen)
- 7aa4739 syntax,transformer: Move JSX entity decoder to `oxc_syntax`
(#24617) (camc314)
- 2b097c4 str: Export `Str` as `ArenaStr` (#24604) (overlookmotel)
- 3acf4c1 minifier: Expand switch optimiation to remove empty cases
(#24520) (Armano)
- 129b759 parser: Improve diagnostics for unparenthesized LHS on
exponential expr (#24569) (camc314)
- 4d0c601 minifier: Fold arithmetic over undefined and null operands
(#24485) (Dunqing)
- 91541dd minifier: Drop empty switch statements  (#24527) (Armano)
- d05224d ast_visit: Generate VisitJs visitor that skips TypeScript
type-space (#24499) (Boshen)
- 3d22307 parser: Add `ParseOptions::enable_ident_hashes` (#24491)
(Boshen)

### 🐛 Bug Fixes

- 64c2241 minifier: Align class heritage removal with assumptions
(#24533) (Dunqing)
- 48b59f4 parser: Span ambient generator diagnostics (#24711) (camc314)
- e750a82 ecmascript: Fix false negative for may_have_side_effects on
dynamic property access (#24709) (sapphi-red)
- f145d73 minifier: Guard reordered identifier reads (#24698) (Dunqing)
- a2ef382 isolated-declarations: Reject `window.Symbol` as global symbol
reference (#24689) (camc314)
- b1bcf72 minifier: Invalidate facts for redeclared bindings (#24658)
(Dunqing)
- 921b834 minifier: Don't treat a conditionally-assigned var as
write-once (#24650) (Dunqing)
- 061af1f minifier: Avoid stale pure function summaries (#24636)
(Dunqing)
- 40c2f43 allocator: `Vec::from_array_in` do not allocate zero-length
array (#24628) (overlookmotel)
- 70994ae codegen: Preserve comments before expression operands (#24510)
(Dunqing)
- 7b4baff parser: Reject new import member access (#23459) (camc314)
- 128b385 minifier: Clippy warning with no-debug-assertions (#24547)
(camc314)
- 8421feb parser: Use first `as` span for imported name (#24537)
(leaysgur)
- c517aa0 parser: Reject invalid accessor assertions (#24504) (camc314)

### ⚡ Performance

- 884d9eb parser: Pre-size cover-grammar assignment target buffers
(#24683) (Boshen)
- d3f07a0 diagnostics: Box OxcDiagnosticInner to reduce binary size
(#24665) (Boshen)
- bcc9de0 parser: Defer diagnostic creation until parse exit (#24663)
(Boshen)
- c35d8ab allocator: Mark `ReplaceWith` panic path cold (#24515)
(camc314)
- ba65790 semantic, allocator: Branchless `clone_in` for semantic IDs
(#24564) (overlookmotel)
- 747feec parser: Build AST nodes with the AST builder instead of
cloning (#24540) (Boshen)
- ba35a0d react_compiler: Use IndexVec for dense id-keyed maps (#24549)
(Boshen)
- b685062 react_compiler: Keep small hot-path collections inline
(#24514) (Marius Schulz)
- a149e95 transformer: Outline rare expression exits (#24512) (camc314)
- 7808a6e react_compiler: Make aliasing effects cheap to intern and
clone (#24506) (Marius Schulz)
- 3a36f2a react_compiler: Store AbstractValue reasons as a u16 bitmask
(#24480) (Boshen)
- 1c96753 react_compiler: Use FxHashMap for the lookup-only aliasing
node map (#24490) (Boshen)

Co-authored-by: Cameron <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-parser Area - Parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant