Skip to content

perf(rolldown): fuse the canonical-name dedup and insert in the renamer#9900

Merged
graphite-app[bot] merged 1 commit into
mainfrom
perf/renamer-entry-api
Jun 21, 2026
Merged

perf(rolldown): fuse the canonical-name dedup and insert in the renamer#9900
graphite-app[bot] merged 1 commit into
mainfrom
perf/renamer-entry-api

Conversation

@Boshen

@Boshen Boshen commented Jun 20, 2026

Copy link
Copy Markdown
Member

What

Renamer::add_symbol_in_root_scope runs once per top-level symbol. On the deconflict path it probed canonical_names twice for the same canonical_ref:

  • a contains_key dedup guard up front (to skip building/allocating the owned name when the ref was already assigned), and
  • an insert at the end.

This fuses them into a single entry() probe — an Occupied slot is the no-op dedup (still skipping the allocation), the Vacant slot is written after resolve. Behaviour is identical.

While here, the JSX-aware "original name" derivation (upper-case the first char when module.preserve_jsx is set) was a free function in renamer.rs that reached across the crate boundary to recombine three rolldown_common primitives for its single caller. It moves to SymbolRefDb::original_name, beside its sibling canonical_name_for_or_original and the MustStartWithCapitalLetterForJSX flag it depends on — which also lets the renamer drop a now-dead canonical_name local and the SymbolRefFlags import.

Evidence (cargo-asm, release / fat-LTO)

The two single-probe changes compound on the inlined add_symbol_in_root_scope (measured cumulatively):

asm lines contains_key RawTable::find insert
original 927 1 1 4
+ #9899 783 0 3
+ this PR 647 0 0 2

This PR removes the second canonical_names probe (the RawTable::find dedup walk) and one more insert.

Verification

  • ConflictResolver unit tests pass
  • topics/deconflict integration snapshots pass byte-identically
  • clippy clean (rolldown + rolldown_common)

Companion of #9899; the two are independent (disjoint files) and can land in either order.

@netlify

netlify Bot commented Jun 20, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

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

@Boshen Boshen marked this pull request as ready for review June 20, 2026 05:17
@codspeed-hq

codspeed-hq Bot commented Jun 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing perf/renamer-entry-api (735856d) with main (752820d)2

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.

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

Boshen commented Jun 21, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • Jun 21, 1:37 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 21, 1:37 PM UTC: Boshen added this pull request to the Graphite merge queue.
  • Jun 21, 1:49 PM UTC: Merged by the Graphite merge queue.

graphite-app Bot pushed a commit that referenced this pull request Jun 21, 2026
…9899)

## What

`ConflictResolver::resolve` is the per-symbol deconfliction primitive (shared by `Renamer` and the cross-chunk export pass) — called once for every top-level symbol in every chunk.

Its fast path (the common case where the bare name is free) hashed and walked the `used` map **twice** for the same key:

```rust
if !self.used.contains_key(&base) && accept(&base, true) {
  self.used.insert(base.clone(), 0);   // second hash + table walk of `base`
  return base;
}
```

This replaces the `contains_key` + `insert` pair with a single `entry()` probe. Semantics are identical:

- `Vacant` + accepted → the old fast path (write the slot we already located)
- `Vacant` + vetoed → leave `base` unrecorded, start the suffix search at 1
- `Occupied` → jump past it using its stored counter

The clone count is unchanged (the map must own the key either way); the win is **one fewer hash + table walk per symbol** on the hot path.

## Evidence (cargo-asm, release / fat-LTO)

Measured on the inlined caller `Renamer::add_symbol_in_root_scope`:

| | asm lines | `contains_key` | `insert` |
|---|---|---|---|
| before | 927 | 1 | 4 |
| after | 783 | 0 | 3 |

## Verification

- `ConflictResolver` unit tests (incl. the counter-jump / veto edge cases) pass
- `topics/deconflict` integration snapshots pass **byte-identically**
- clippy clean

Companion of #9900, which applies the same single-probe pattern to the `canonical_names` dedup in `add_symbol_in_root_scope`. The two are independent (disjoint files) and can land in either order.
…er (#9900)

## What

`Renamer::add_symbol_in_root_scope` runs once per top-level symbol. On the deconflict path it probed `canonical_names` **twice** for the same `canonical_ref`:

- a `contains_key` dedup guard up front (to skip building/allocating the owned name when the ref was already assigned), and
- an `insert` at the end.

This fuses them into a single `entry()` probe — an `Occupied` slot is the no-op dedup (still skipping the allocation), the `Vacant` slot is written after `resolve`. Behaviour is identical.

While here, the JSX-aware "original name" derivation (upper-case the first char when `module.preserve_jsx` is set) was a free function in `renamer.rs` that reached across the crate boundary to recombine three `rolldown_common` primitives for its single caller. It moves to `SymbolRefDb::original_name`, beside its sibling `canonical_name_for_or_original` and the `MustStartWithCapitalLetterForJSX` flag it depends on — which also lets the renamer drop a now-dead `canonical_name` local and the `SymbolRefFlags` import.

## Evidence (cargo-asm, release / fat-LTO)

The two single-probe changes compound on the inlined `add_symbol_in_root_scope` (measured cumulatively):

| | asm lines | `contains_key` | `RawTable::find` | `insert` |
|---|---|---|---|---|
| original | 927 | 1 | 1 | 4 |
| + #9899 | 783 | 0 | — | 3 |
| + this PR | 647 | 0 | 0 | 2 |

This PR removes the second `canonical_names` probe (the `RawTable::find` dedup walk) and one more `insert`.

## Verification

- `ConflictResolver` unit tests pass
- `topics/deconflict` integration snapshots pass **byte-identically**
- clippy clean (`rolldown` + `rolldown_common`)

Companion of #9899; the two are independent (disjoint files) and can land in either order.
@graphite-app graphite-app Bot force-pushed the perf/renamer-entry-api branch from 735856d to 2c249f4 Compare June 21, 2026 13:42
@graphite-app graphite-app Bot merged commit 2c249f4 into main Jun 21, 2026
34 checks passed
@graphite-app graphite-app Bot deleted the perf/renamer-entry-api branch June 21, 2026 13:49
@rolldown-guard rolldown-guard Bot mentioned this pull request Jun 24, 2026
shulaoda added a commit that referenced this pull request Jun 24, 2026
## [1.1.3] - 2026-06-24

### 🐛 Bug Fixes

- `defer_drop` crashes the browser main thread (#9942) by @shulaoda
- camel-case: correct camel case for nested values (#9933) by @kb019
- cli: display --help options in camelCase (#9941) by @IWANABETHATGUY
- preserve used re-exports under preserveModules (#9122) (#9934) by @IWANABETHATGUY
- watch: make close reentrant in event callbacks (#9904) by @hyf0
- git for windows treats symlink files as regular files (#9915) by @AliceLanniste
- dev: cancel pending full reload on build error (#9903) by @h-a-n-a
- chunking: pass plugin meta to codeSplitting groups name function (#9267) by @Kyujenius
- dev: serve assets emitted during HMR/lazy compile (vite#22596) (#9815) by @h-a-n-a
- release: dry-run step no longer publishes binding packages (#9866) by @Boshen

### 🚜 Refactor

- rolldown_common: model ModuleId as a classified Path/Virtual/Bare enum (#9927) by @Boshen
- remove unused LegacyModuleIdx (#9872) by @shulaoda
- remove unused StmtInfos::get_namespace_stmt_info (#9870) by @shulaoda
- remove unused Module::as_external_mut (#9871) by @shulaoda
- remove unused EcmaAst::is_body_empty (#9869) by @shulaoda
- drop dead is_css_module handling in resolve_dependencies (#9867) by @shulaoda
- drop redundant with_commonjs on cjs source type (#9868) by @shulaoda

### 📚 Documentation

- clarify on drafting PRs (#9952) by @h-a-n-a
- update contribution guidelines (#9944) by @fubhy
- note Rust crates don't follow semver in AGENTS.md (#9905) by @IWANABETHATGUY
- add feedback form (#9159) by @TheAlexLichter

### ⚡ Performance

- utils: avoid allocation in default_sanitize_file_name for clean names (#9928) by @Boshen
- binding: box once-per-build futures before spawn_future (#9864) by @Boshen
- utils: avoid wasted allocation in legitimize_identifier_name (#9926) by @Boshen
- rolldown: fuse the canonical-name dedup and insert in the renamer (#9900) by @Boshen
- rolldown: probe the name map once in ConflictResolver::resolve (#9899) by @Boshen
- cut two heap allocations from wrapped ESM init finalize (#9901) by @Boshen
- rolldown_plugin_vite_reporter: hoist invariant out_dir prefix out of reporter loop (#9873) by @shulaoda
- drop throwaway Vec in wrapped esm init stmt (#9878) by @shulaoda
- borrow owner_filename in build-import-analysis AddDeps (#9874) by @shulaoda

### 🧪 Testing

- cover preserveModules named export via namespace re-export (#6010) (#9937) by @IWANABETHATGUY

### ⚙️ Miscellaneous Tasks

- deps: update napi to v3.9.4 (#9954) by @shulaoda
- reduce noise from CODEOWNERS for trival changes (#9953) by @h-a-n-a
- deps: update mimalloc-safe to 0.1.64 (#9950) by @shulaoda
- deps: update rollup submodule for tests to v4.62.2 (#9931) by @rolldown-guard[bot]
- deps: test mimalloc-safe upstream-mimalloc switch in CI (#9930) by @shulaoda
- rolldown_plugin_vite_build_import_analysis: remove unused v2 code path (#9917) by @shulaoda
- rolldown_plugin_vite_manifest: remove unused is_enable_v2 code path (#9916) by @shulaoda
- rolldown_plugin_vite_asset_import_meta_url: remove unexposed native vite plugin (#9896) by @shulaoda
- rolldown_plugin_vite_asset: remove unexposed native vite plugin (#9895) by @shulaoda
- rolldown_plugin_vite_css_post: remove unexposed native vite plugin (#9894) by @shulaoda
- rolldown_plugin_vite_css: remove unexposed native vite plugin (#9893) by @shulaoda
- rolldown_plugin_vite_html_inline_proxy: remove unexposed native vite plugin (#9892) by @shulaoda
- rolldown_plugin_vite_html: remove unexposed native vite plugin (#9891) by @shulaoda
- deps: update github actions (#9909) by @renovate[bot]
- deps: update rust crate oxc_sourcemap to v8.0.2 (#9910) by @renovate[bot]
- deps: update npm packages (#9912) by @renovate[bot]
- deps: update github actions to v7 (#9913) by @renovate[bot]
- deps: update rolldown-plugin-dts to ^0.26.0 (#9897) by @renovate[bot]
- remove rolldown_filter_analyzer crate (#9865) by @Boshen

### ❤️ New Contributors

* @fubhy made their first contribution in [#9944](#9944)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants