Skip to content

perf(react_compiler): borrow binding names in prefilter instead of allocating#23471

Merged
Boshen merged 1 commit into
oxc-project:mainfrom
hyf0:perf/react-compiler-prefilter-no-string-alloc
Jun 16, 2026
Merged

perf(react_compiler): borrow binding names in prefilter instead of allocating#23471
Boshen merged 1 commit into
oxc-project:mainfrom
hyf0:perf/react-compiler-prefilter-no-string-alloc

Conversation

@hyf0

@hyf0 hyf0 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

What & why

The React-compiler prefilter (has_react_like_functions) runs on every file in transform()
before deciding whether to compile it, and walks the whole program (for non-React files it never
short-circuits). For every variable declarator and every assignment-target identifier it did:

current_name: Option<String>,
// ...
Some(ident.name.to_string())   // heap String, used only to call is_react_like_name(&str)

ident.name is already an arena Ident<'a> (as_str() returns &'a str), and current_name is
only ever read by is_react_like_name(name: &str). So the owned String — a heap alloc + memcpy of
the name bytes, per binding, thrown away immediately — is pure waste. This borrows it instead:

struct ReactLikeVisitor<'a> { found: bool, current_name: Option<&'a str> }
// ...
Some(ident.name.as_str())      // zero-cost arena borrow

The positive signal: one heap String allocation (malloc/free + name-byte memcpy) is removed per
declarator and per assignment-target identifier, across the whole program, on a path that runs for
every file. On non-React files (e.g. a large binder.ts) the prefilter never short-circuits, so this
is thousands of throwaway allocations eliminated. &str/Ident is Copy and the struct shrinks
(Option<&str> 16B vs Option<String> 24B), so it is strictly less work — never more.

oxc_react_compiler is not covered by allocs_*.snap, so there's no local snapshot to quantify
this; the react_compiler CodSpeed benchmark is the arbiter (removing system malloc/free is real
retired instructions, so it should register).

Behavior-preserving

The borrowed &'a str holds the identical bytes the String did; is_react_like_name is a pure
read. Prefilter include/exclude decisions are byte-for-byte identical. The lifetime is sound:
Ident::as_str() -> &'a str points into the arena (the program's 'a), which outlives the visitor.
cargo test -p oxc_react_compiler (17 tests) passes; no snapshot drift.

Note: prefilter.rs is vendored from the upstream React Compiler (react_compiler_oxc), but is
already substantially oxc-diverged (it adds ResourceManagementVisitor, memo/forwardRef
handling, etc.). This is an intentional oxc-side idiom tightening — flagging it for anyone re-syncing.


Prepared with AI assistance.

@codspeed-hq

codspeed-hq Bot commented Jun 16, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 6.43%

⚡ 1 improved benchmark
✅ 9 untouched benchmarks
⏩ 61 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation react_compiler[binder.ts] 10.8 µs 10.1 µs +6.43%

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 hyf0:perf/react-compiler-prefilter-no-string-alloc (0ee6ede) with main (12e4451)

Open in CodSpeed

Footnotes

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

@hyf0

hyf0 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

I wonder if I should touch things related react compiler? Are maintainability and alignment more important than performance. @Boshen cc

@hyf0 hyf0 marked this pull request as ready for review June 16, 2026 05:23
@Boshen Boshen self-assigned this Jun 16, 2026
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jun 16, 2026

Boshen commented Jun 16, 2026

Copy link
Copy Markdown
Member

Merge activity

  • Jun 16, 6:26 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 16, 6:26 AM UTC: Boshen added this pull request to the Graphite merge queue.

@Boshen Boshen merged commit 488b382 into oxc-project:main Jun 16, 2026
33 checks passed
Boshen added a commit that referenced this pull request Jun 18, 2026
### 💥 BREAKING CHANGES

- 7a76cd3 estree: [**BREAKING**] Make whether to include TS fields a
runtime option (#23574) (overlookmotel)
- e7b6b68 estree: [**BREAKING**] `ESTree` config use methods not consts
(#23573) (overlookmotel)

### 🚀 Features

- 556cc6d data_structures: Add `CodeBuffer::as_str` method (#23571)
(overlookmotel)
- 38c4b06 parser: Add friendly error for adjacent JSX elements (#23378)
(sapphi-red)
- 53509a8 minifier: Treeshake pure typed arrays and Set/Map array
literals (#23469) (Dunqing)
- 09762d9 minifier: Inline const value for read-only vars (#22593)
(Dunqing)

### 🐛 Bug Fixes

- 20375f9 react_compiler: Keep imports referenced only by a computed key
(#23586) (Boshen)
- 31bfd9b minifier: Keep Object introspection calls on a possible Proxy
(#23483) (Dunqing)
- 837a395 parser: Treat a line comment after ':' as leading, not
trailing (#23515) (Dunqing)
- e409fe0 minifier: Keep `new Map`/`WeakSet`/`WeakMap` with a string
argument (#23470) (Dunqing)
- ae02b4e ci/parser: Use `minimal` for vitest reporter (#23457)
(camc314)

### ⚡ Performance

- cf24329 mangler: Compile slot sort once instead of per CAPACITY
(#23577) (Boshen)
- 4058a6a parser: Reduce code bloat from verify_modifiers
monomorphization (#23576) (Boshen)
- 053b0c1 estree: Remove pointless `mem::take` (#23572) (overlookmotel)
- dfb52b6 transformer: Pre-size statement vecs in TS enum & namespace
lowering (#23516) (Yunfei He)
- 970e09a minifier: Compute template-literal inline checks in a single
pass (#23467) (Yunfei He)
- 3170c0e semantic,mangler,minifier: Fix `Semantic::stats` node count
and reuse stats in mangler builds (#23352) (Boshen)
- d1fa6e0 minifier: Evaluate ternary branches once in
minimize_conditional_expression (#23479) (Yunfei He)
- 3fa8051 transformer: Pre-size JSX props vec to attribute count
(#23466) (Yunfei He)
- 488b382 react_compiler: Borrow binding names in prefilter instead of
allocating (#23471) (Yunfei He)
- bcb3894 minifier: Incremental scoping refresh, delete
LiveUsageCollector (#23197) (Dunqing)

### 📚 Documentation

- f68641e data_structures: Improve docs on safety contract (#23575)
(overlookmotel)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
camc314 pushed a commit that referenced this pull request Jul 3, 2026
…locating (#23471)

## What & why

The React-compiler prefilter (`has_react_like_functions`) runs on
**every file** in `transform()`
before deciding whether to compile it, and walks the whole program (for
non-React files it never
short-circuits). For **every variable declarator and every
assignment-target identifier** it did:

```rust
current_name: Option<String>,
// ...
Some(ident.name.to_string())   // heap String, used only to call is_react_like_name(&str)
```

`ident.name` is already an arena `Ident<'a>` (`as_str()` returns `&'a
str`), and `current_name` is
only ever read by `is_react_like_name(name: &str)`. So the owned
`String` — a heap alloc + memcpy of
the name bytes, per binding, thrown away immediately — is pure waste.
This borrows it instead:

```rust
struct ReactLikeVisitor<'a> { found: bool, current_name: Option<&'a str> }
// ...
Some(ident.name.as_str())      // zero-cost arena borrow
```

**The positive signal:** one heap `String` allocation (malloc/free +
name-byte memcpy) is removed per
declarator and per assignment-target identifier, across the whole
program, on a path that runs for
every file. On non-React files (e.g. a large `binder.ts`) the prefilter
never short-circuits, so this
is thousands of throwaway allocations eliminated. `&str`/`Ident` is
`Copy` and the struct shrinks
(`Option<&str>` 16B vs `Option<String>` 24B), so it is strictly less
work — never more.

`oxc_react_compiler` is **not** covered by `allocs_*.snap`, so there's
no local snapshot to quantify
this; the `react_compiler` CodSpeed benchmark is the arbiter (removing
system `malloc`/`free` is real
retired instructions, so it should register).

## Behavior-preserving

The borrowed `&'a str` holds the identical bytes the `String` did;
`is_react_like_name` is a pure
read. Prefilter include/exclude decisions are byte-for-byte identical.
The lifetime is sound:
`Ident::as_str() -> &'a str` points into the arena (the program's `'a`),
which outlives the visitor.
`cargo test -p oxc_react_compiler` (17 tests) passes; no snapshot drift.

> Note: `prefilter.rs` is vendored from the upstream React Compiler
(`react_compiler_oxc`), but is
> already substantially oxc-diverged (it adds
`ResourceManagementVisitor`, `memo`/`forwardRef`
> handling, etc.). This is an intentional oxc-side idiom tightening —
flagging it for anyone re-syncing.

---

Prepared with AI assistance.
camc314 pushed a commit that referenced this pull request Jul 3, 2026
### 💥 BREAKING CHANGES

- 7a76cd3 estree: [**BREAKING**] Make whether to include TS fields a
runtime option (#23574) (overlookmotel)
- e7b6b68 estree: [**BREAKING**] `ESTree` config use methods not consts
(#23573) (overlookmotel)

### 🚀 Features

- 556cc6d data_structures: Add `CodeBuffer::as_str` method (#23571)
(overlookmotel)
- 38c4b06 parser: Add friendly error for adjacent JSX elements (#23378)
(sapphi-red)
- 53509a8 minifier: Treeshake pure typed arrays and Set/Map array
literals (#23469) (Dunqing)
- 09762d9 minifier: Inline const value for read-only vars (#22593)
(Dunqing)

### 🐛 Bug Fixes

- 20375f9 react_compiler: Keep imports referenced only by a computed key
(#23586) (Boshen)
- 31bfd9b minifier: Keep Object introspection calls on a possible Proxy
(#23483) (Dunqing)
- 837a395 parser: Treat a line comment after ':' as leading, not
trailing (#23515) (Dunqing)
- e409fe0 minifier: Keep `new Map`/`WeakSet`/`WeakMap` with a string
argument (#23470) (Dunqing)
- ae02b4e ci/parser: Use `minimal` for vitest reporter (#23457)
(camc314)

### ⚡ Performance

- cf24329 mangler: Compile slot sort once instead of per CAPACITY
(#23577) (Boshen)
- 4058a6a parser: Reduce code bloat from verify_modifiers
monomorphization (#23576) (Boshen)
- 053b0c1 estree: Remove pointless `mem::take` (#23572) (overlookmotel)
- dfb52b6 transformer: Pre-size statement vecs in TS enum & namespace
lowering (#23516) (Yunfei He)
- 970e09a minifier: Compute template-literal inline checks in a single
pass (#23467) (Yunfei He)
- 3170c0e semantic,mangler,minifier: Fix `Semantic::stats` node count
and reuse stats in mangler builds (#23352) (Boshen)
- d1fa6e0 minifier: Evaluate ternary branches once in
minimize_conditional_expression (#23479) (Yunfei He)
- 3fa8051 transformer: Pre-size JSX props vec to attribute count
(#23466) (Yunfei He)
- 488b382 react_compiler: Borrow binding names in prefilter instead of
allocating (#23471) (Yunfei He)
- bcb3894 minifier: Incremental scoping refresh, delete
LiveUsageCollector (#23197) (Dunqing)

### 📚 Documentation

- f68641e data_structures: Improve docs on safety contract (#23575)
(overlookmotel)

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

0-merge Merge with Graphite Merge Queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants