Skip to content

perf(semantic): V8-style walk-up reference resolution#20292

Merged
graphite-app[bot] merged 1 commit intomainfrom
refactor/walk-up-reference-resolution
Mar 13, 2026
Merged

perf(semantic): V8-style walk-up reference resolution#20292
graphite-app[bot] merged 1 commit intomainfrom
refactor/walk-up-reference-resolution

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Mar 12, 2026

Summary

  • Replace bubble-up reference resolution with V8-style walk-up algorithm
  • References are collected in a flat Vec<(Ident, ReferenceId)> during traversal instead of per-scope hashmaps
  • After full AST visit, all references are resolved in a single pass by walking up the scope chain from each reference's scope
  • leave_scope() no longer does any reference resolution work — only direct_eval flag propagation remains
  • Early resolution via checkpoint mechanism preserved for function parameters and catch parameters (prevents binding to body variables that share the same scope)
  • Deduplicated walk-up logic into a single #[inline(always)] helper shared by both normal and early resolution paths
  • Uses to_vec+truncate instead of split_off to preserve Vec capacity and avoid reallocation
  • Removes smallvec and oxc_data_structures dependencies from oxc_semantic

Why this is faster

Operation Bubble-up (old) Walk-up (new)
Per reference creation Hashmap insert Vec push (O(1) amortized)
Per scope exit Drain + merge hashmap Nothing
Per reference resolution Implicit via merging Walk scope chain, O(depth) lookups
Total hashmap mutations O(refs × depth) inserts+drains 0 during traversal

Hashmap lookups (read-only) are cheaper than hashmap drain+insert (mutating, allocating). The flat Vec also eliminates creating/recycling per-depth hashmaps entirely.

Snapshot changes

3 jest linter rule snapshots have diagnostic reordering (same diagnostics, different emission order) due to references being processed in insertion order rather than scope-by-scope.

🤖 Generated with Claude Code

@github-actions github-actions bot added A-linter Area - Linter A-semantic Area - Semantic C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Mar 12, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 12, 2026

Merging this PR will improve performance by 6.01%

⚡ 4 improved benchmarks
✅ 49 untouched benchmarks
⏩ 3 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation semantic[binder.ts] 3 ms 2.9 ms +5.64%
Simulation semantic[react.development.js] 1.2 ms 1.1 ms +6.01%
Simulation semantic[RadixUIAdoptionSection.jsx] 62.1 µs 59.3 µs +4.73%
Simulation pipeline[binder.ts] 15 ms 14.4 ms +3.82%

Comparing refactor/walk-up-reference-resolution (ddfa853) with main (5c97b14)2

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.

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

@Boshen Boshen changed the title refactor(semantic): V8-style walk-up reference resolution refactor(semantic): V8-style walk-up reference resolution (POC) Mar 12, 2026
@Boshen Boshen changed the title refactor(semantic): V8-style walk-up reference resolution (POC) perf(semantic): V8-style walk-up reference resolution (POC) Mar 12, 2026
@github-actions github-actions bot added the C-performance Category - Solution not expected to change functional behavior, only performance label Mar 12, 2026
@Boshen Boshen requested a review from overlookmotel March 12, 2026 15:21
@Boshen
Copy link
Member Author

Boshen commented Mar 12, 2026

POC - please redo this PR if necessary.

@Boshen Boshen force-pushed the refactor/walk-up-reference-resolution branch from 637fd88 to e022db3 Compare March 12, 2026 15:22
@Boshen Boshen marked this pull request as ready for review March 12, 2026 15:24
@Boshen Boshen requested review from Dunqing and camc314 as code owners March 12, 2026 15:24
Copilot AI review requested due to automatic review settings March 12, 2026 15:24
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 refactors oxc_semantic reference resolution from a per-scope “bubble-up” hashmap merge strategy to a V8-style “walk-up” strategy, aiming to reduce allocation and mutation overhead during traversal while preserving early-resolution correctness for parameters/catch bindings.

Changes:

  • Collect unresolved references into a flat Vec<(Ident, ReferenceId)> during AST traversal and resolve them in a single post-traversal pass by walking up the scope chain.
  • Preserve early resolution via a checkpoint mechanism for function parameters and catch parameters.
  • Remove smallvec and oxc_data_structures dependencies from oxc_semantic and update associated snapshots (coverage, memory tracking, linter).

Reviewed changes

Copilot reviewed 9 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tasks/track_memory_allocations/allocs_semantic.snap Updates allocation tracking snapshot after semantic perf changes.
tasks/track_memory_allocations/allocs_minifier.snap Updates allocation tracking snapshot after semantic perf changes.
tasks/coverage/snapshots/semantic_typescript.snap Updates TS semantic coverage snapshot reflecting new reference processing order.
tasks/coverage/snapshots/semantic_misc.snap Updates misc semantic coverage snapshot reflecting new reference processing order.
crates/oxc_semantic/src/unresolved_stack.rs Replaces per-scope unresolved stack with a flat unresolved reference list + checkpoints.
crates/oxc_semantic/src/scoping.rs Adjusts root unresolved reference recording API to match new resolution flow.
crates/oxc_semantic/src/builder.rs Implements walk-up batch resolution and checkpoint-based early resolution.
crates/oxc_semantic/Cargo.toml Drops smallvec and oxc_data_structures deps from oxc_semantic.
crates/oxc_linter/src/snapshots/jest_valid_describe_callback.snap Snapshot reorder due to changed diagnostic emission order.
crates/oxc_linter/src/snapshots/jest_prefer_lowercase_title@jest.snap Snapshot reorder due to changed diagnostic emission order.
crates/oxc_linter/src/snapshots/jest_no_confusing_set_timeout.snap Snapshot reorder due to changed diagnostic emission order.
Cargo.lock Lockfile updates reflecting removed dependencies.

You can also share your feedback on Copilot code review. Take the survey.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e022db3f66

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions github-actions bot added the A-transformer Area - Transformer / Transpiler label Mar 13, 2026
@Dunqing Dunqing force-pushed the refactor/walk-up-reference-resolution branch from db34e70 to 9ce9870 Compare March 13, 2026 03:29
@Dunqing
Copy link
Member

Dunqing commented Mar 13, 2026

5604bcb change improved 3% performance in each benchmark 😅

@Dunqing Dunqing force-pushed the refactor/walk-up-reference-resolution branch 2 times, most recently from 205eb30 to 5604bcb Compare March 13, 2026 05:56
@Dunqing Dunqing changed the title perf(semantic): V8-style walk-up reference resolution (POC) perf(semantic): V8-style walk-up reference resolution Mar 13, 2026
Copy link
Member

@Dunqing Dunqing left a comment

Choose a reason for hiding this comment

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

Pretty cool! Less dependency, more performant, and much cleaner code now!

@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Mar 13, 2026
Copy link
Member

Dunqing commented Mar 13, 2026

Merge activity

## Summary

- Replace bubble-up reference resolution with V8-style walk-up algorithm
- References are collected in a flat `Vec<(Ident, ReferenceId)>` during traversal instead of per-scope hashmaps
- After full AST visit, all references are resolved in a single pass by walking up the scope chain from each reference's scope
- `leave_scope()` no longer does any reference resolution work — only `direct_eval` flag propagation remains
- Early resolution via checkpoint mechanism preserved for function parameters and catch parameters (prevents binding to body variables that share the same scope)
- Deduplicated walk-up logic into a single `#[inline(always)]` helper shared by both normal and early resolution paths
- Uses `to_vec+truncate` instead of `split_off` to preserve `Vec` capacity and avoid reallocation
- Removes `smallvec` and `oxc_data_structures` dependencies from `oxc_semantic`

### Why this is faster

| Operation | Bubble-up (old) | Walk-up (new) |
|-----------|-----------------|---------------|
| Per reference creation | Hashmap insert | Vec push (O(1) amortized) |
| Per scope exit | Drain + merge hashmap | Nothing |
| Per reference resolution | Implicit via merging | Walk scope chain, O(depth) lookups |
| Total hashmap mutations | O(refs × depth) inserts+drains | 0 during traversal |

Hashmap lookups (read-only) are cheaper than hashmap drain+insert (mutating, allocating). The flat Vec also eliminates creating/recycling per-depth hashmaps entirely.

### Snapshot changes

3 jest linter rule snapshots have diagnostic reordering (same diagnostics, different emission order) due to references being processed in insertion order rather than scope-by-scope.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app bot force-pushed the refactor/walk-up-reference-resolution branch from ddfa853 to 5474d0a Compare March 13, 2026 06:20
@graphite-app graphite-app bot merged commit 5474d0a into main Mar 13, 2026
22 checks passed
@graphite-app graphite-app bot deleted the refactor/walk-up-reference-resolution branch March 13, 2026 06:31
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 13, 2026
Boshen added a commit that referenced this pull request Mar 14, 2026
### 🚀 Features

- e7163b6 ecmascript: Add known-globals to side-effect-free property
reads (#20212) (Dunqing)
- 139ab68 ecmascript: Add `property_write_side_effects` to
`MayHaveSideEffectsContext` (#20217) (Dunqing)

### 🐛 Bug Fixes

- 78c264a parser: Fix conditional expressions with arrow-function
alternates in TS (#20356) (camc314)
- 5c97b14 minifier: Recognize object spread of object literals as
side-effect-free (#20299) (Boshen)
- 1ff5c1d transformer/typescript: Rewrite extensions in dynamic
`import()` expressions (#20121) (Sverre Johansen)
- 1c07b3b diagnostics: Handle `WouldBlock` in stdout writes to prevent
panic (#20295) (Boshen)
- ade14d4 ecmascript: Enhance side-effect detection for classes,
TypedArrays, computed members, and spread (#20213) (Dunqing)

### ⚡ Performance

- 5474d0a semantic: V8-style walk-up reference resolution (#20292)
(Boshen)

### 📚 Documentation

- e4aa5b5 parser/napi, linter/plugins: Add JSDoc comments to raw
transfer constants (#20286) (overlookmotel)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
camc314 pushed a commit that referenced this pull request Mar 16, 2026
# Oxlint
### 🚀 Features

- c95951f linter/plugins: Implement `sourceCode.markVariableAsUsed` (#20357) (overlookmotel)
- 7a2a7d0 linter: Implement `n/handle-callback-err` rule (#19616) (Mikhail Baev)

### 🐛 Bug Fixes

- f8fbd6e linter/plugins: Remove `hashbang` property from AST (#20365) (overlookmotel)
- 6eb5b01 linter/prefer-await-to-then: Ignore Promise static methods (#20347) (camc314)
- a4b61f7 linter: Remove `defineConfig` check (#20308) (camc314)
- 3ad7f53 linter/explicit-module-boundary-types: False positive with satisfies expr (#20309) (camc314)
- f547401 linter/no-unused-private-class-members: Treat switch discriminants as read (#20307) (camc314)
- 1c07b3b diagnostics: Handle `WouldBlock` in stdout writes to prevent panic (#20295) (Boshen)

### ⚡ Performance

- e4f7248 linter: Remove unnecessary clone of owned String in drain loop (#20388) (Boshen)
- 4a67f1d linter: Eliminate Vec allocation in disable directive matching (#20387) (Boshen)
- 618a598 linter/plugins: Add fast path for files with no comments (#20366) (overlookmotel)
- b0125c5 linter/plugins: Deserialize comments without AST (#20364) (overlookmotel)
- 9cd612f linter/plugins: Recycle comment objects (#20362) (overlookmotel)
- bf442f8 linter/plugins: Cheaper `Token` creation (#20360) (overlookmotel)
- 5474d0a semantic: V8-style walk-up reference resolution (#20292) (Boshen)
- 7946eba linter/plugins: Avoid arguments spread and temp array when merging (#20318) (overlookmotel)
- fc7cf8a linter/plugins: Pre-define less CFG merger functions (#20317) (overlookmotel)
- 3b9eb28 linter/plugins: Streamline getting/creating visit fn mergers (#20319) (overlookmotel)
- f04e850 linter/plugins: Inline binary search functions into call sites (#20312) (overlookmotel)
- fe24afe linter/plugins: Apply replace globals TSDown plugin to JS files (#20305) (overlookmotel)
- 77cdacc linter/plugins: Use array buffer views for tokens (#20301) (overlookmotel)
- 910c941 linter/plugins: Reorder branches in `getTokenByRangeStart` (#20296) (overlookmotel)
- af7674c linter/tokens: Avoid extra token value allocation (#20013) (camc314)

### 📚 Documentation

- 24490b5 linter: Improve formatting for 80ish rules' docs. (#20411) (connorshea)
- 3383523 linter: Improve `--tsconfig` flag docs (#20342) (camc314)
# Oxfmt
### 🚀 Features

- d22c443 oxfmt: Export `OxfmtConfig` type (#20275) (leaysgur)
- a11ecff oxfmt/lsp: Respect `angular` language id as `.component.html` file (#20242) (Sysix)

### 🐛 Bug Fixes

- ce65099 formatter: Preserve parentheses around as expression before private field access (#20419) (bab)
- f908742 oxfmt: Revert #20326 partially (#20413) (leaysgur)
- 4ef93ea formatter: Honor trailing ignore comments after list separators (#19925) (Andreas Lubbe)
- 68fb0d0 oxfmt: Skip vite.config.ts which fails to import (#20326) (leaysgur)
- 88ee826 oxfmt: Handle literalline for script-in-vue (#20130) (leaysgur)
- 1c07b3b diagnostics: Handle `WouldBlock` in stdout writes to prevent panic (#20295) (Boshen)

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-linter Area - Linter A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior 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