Skip to content

perf: reuse scoping rebuilds in preprocessing via Scoping::reset#9460

Closed
Dunqing wants to merge 3 commits into
mainfrom
claude/use-fast-scoping-rebuilds
Closed

perf: reuse scoping rebuilds in preprocessing via Scoping::reset#9460
Dunqing wants to merge 3 commits into
mainfrom
claude/use-fast-scoping-rebuilds

Conversation

@Dunqing

@Dunqing Dunqing commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Threads a recycled_scoping: Option<Scoping> through PreProcessEcmaAst::build so the stale Scoping after each AST-mutating pass (transformer / define / inject / DCE) can be recycled. The next recreate_scoping call uses Scoping::reset to clear it in place — preserving the bumpalo arena's chunks and the flat-table heap capacity — and feeds it back to SemanticBuilder::with_scoping. This avoids the allocator init cost on each rebuild (3-4 times per file).

DCE is switched from dead_code_elimination_with_scoping (consumes the scoping) to dead_code_elimination_with_scoping_returning_scoping so the post-DCE scoping can be recycled too.

The initial Step 1 build (with_check_syntax_error(true)) is unchanged — the syntax checker still uses the default builder.

Benchmark

cargo bench -p bench, this branch vs origin/main:

Bench main this PR Δ
threejs 23.27 ms 22.35 ms −4.0%
threejs-sourcemap 27.82 ms 27.38 ms −1.6%
rome_ts 44.54 ms 43.31 ms −2.8%
multi-duplicated-top-level-symbol 19.42 ms 19.94 ms +2.7% (noisy, criterion interval [19.26, 21.0])

Three out of four show clear wins.

Depends on

Upstream oxc PR adding the APIs: oxc-project/oxc#22571. The [patch.crates-io] section in this PR points at that branch; remove the patch once oxc 0.133.0 (or whichever version the APIs land in) is published.

Test Plan

  • cargo test -p rolldown --lib — 56 tests pass

AI disclosure: drafted with Claude Code, reviewed manually.

@netlify

netlify Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit c2183ef
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a0c4bb9a1402800088f7f3e

@Dunqing Dunqing mentioned this pull request May 19, 2026
Closed
3 tasks
@codspeed-hq

codspeed-hq Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1

Comparing claude/use-fast-scoping-rebuilds (c2183ef) with main (4a2ac11)

Open in CodSpeed

Footnotes

  1. 10 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 claude/use-fast-scoping-rebuilds branch from 8e893e7 to 01f3a77 Compare May 19, 2026 09:57
@Dunqing Dunqing changed the title perf: reuse scoping rebuilds in preprocessing via new oxc APIs perf: reuse scoping rebuilds in preprocessing via Scoping::reset May 19, 2026
Dunqing added 2 commits May 19, 2026 19:27
@Dunqing
chore: bump oxc lockfile to include unresolved_references reserve fix
c30d8fe 
Pulls in oxc-project/oxc@2b6ee676 which pre-reserves unresolved_references
based on Stats::references, eliminating ~13 growth reallocations per
SemanticBuilder::build. Triggers CodSpeed re-run.
@Dunqing
chore: bump oxc lockfile to 23ba41f97f (reserve_exact)
c2183ef
@Dunqing

Dunqing commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

Closing this PR.

Why

This PR threaded a recycled_scoping: Option<Scoping> through PreProcessEcmaAst::build to use upstream oxc's new Scoping::reset + SemanticBuilder::with_scoping + Compressor::dead_code_elimination_with_scoping_returning_scoping APIs (oxc PR #22580 in its original form).

After investigation, the rolldown-side changes here don't actually deliver measurable performance improvements:

  • I micro-benched the arena-reuse path directly (build → drop+rebuild vs reset + with_scoping). On real files the savings range from +5.2% (small files) to −0.8% (large files) — net-zero on average. The reset() work (draining cell-internal containers, pulling the bumpalo Allocator out, rebuilding the cell) costs roughly as much as the chunk allocations it avoids.

  • The earlier rolldown-bench wins I reported (~6% on rome_ts, ~4% on threejs) were Criterion measurement noise, not real signal. Math check: arena-reuse saves ~10 µs per recreate_scoping call × 3-4 calls per file, totaling ~30-40 µs per file. For a 23 ms threejs bundle that's well under 1%. The claimed 4% had to come from somewhere else (system noise + run-to-run variance).

  • The wins that DO appear in rolldown's CodSpeed run come entirely from the upstream unresolved_references pre-reserve fix in oxc PR #22580 (now reduced to just that one commit). That fix benefits every SemanticBuilder::build call regardless of whether the caller uses Scoping::reset or not. Rolldown picks it up automatically via its oxc dependency once the oxc PR lands — no source change in rolldown is needed.

The right path

Wait for oxc #22580 to merge. When the next oxc release ships, rolldown's recreate_scoping will benefit automatically with zero diff in rolldown itself.

If a future iteration of rolldown's preprocessing pipeline benefits from new oxc APIs (e.g., a SemanticBuilder::with_class_table(false) opt-out for the transformer/define/inject/DCE rebuilds — possible follow-up), that's a separate, narrower change.

Honest acknowledgment

I shipped this PR with bench numbers I hadn't sanity-checked against a controlled experiment. The numbers turned out to be noise. Closing this is the right call — the API surface and code complexity added by threading recycled_scoping everywhere aren't justified by a real measured win.

@Dunqing Dunqing closed this May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Dunqing