Skip to content

perf: cut two heap allocations from wrapped ESM init finalize#9901

Merged
graphite-app[bot] merged 1 commit into
mainfrom
perf/wrapped-esm-init-allocs
Jun 20, 2026
Merged

perf: cut two heap allocations from wrapped ESM init finalize#9901
graphite-app[bot] merged 1 commit into
mainfrom
perf/wrapped-esm-init-allocs

Conversation

@Boshen

@Boshen Boshen commented Jun 20, 2026

Copy link
Copy Markdown
Member

What

wrapped_esm_init_stmt_for_import_record (scope-hoisting finalize path) performed two heap allocations per wrapped-ESM import record. This removes both, with no behavior change.

1. Per-record FxIndexSetVec

collect_wrapped_esm_init_modules_for_import_record returned an owned FxIndexSet<ModuleIdx> only to dedup owners within a single record. Its sole caller already filters every owner through the persistent generated_init_esm_importee_ids (FxHashSet) before building the init call, so the local set's dedup is fully subsumed by the global one — a duplicate is dropped at the global check and never re-emits. Switching to Vec<ModuleIdx> replaces the IndexMap insert/rehash with a plain push and drops one hashmap allocation per record.

2. Drop the .collect::<Vec<_>>() of init expressions

The expressions were collected into a std::Vec only to end the &mut self iterator borrow before reaching for self.ast_factory. AstFactory is Copy, so copying it out lets the iterator stay borrowed while nodes are still built through the copy. Driving the iterator by hand means the common 0- and 1-statement cases allocate nothing, and only the rare (a(), b()) sequence case allocates — straight into the arena. Every branch consumes the iterator to exhaustion, preserving the global-dedup side effects exactly.

Evidence

cargo asm -p rolldown --lib wrapped_esm_init_stmt (aarch64, release lto = "fat"):

before after
instructions 808 731
calls (bl) 40 27

Removed calls include indexmap::IndexMap::insert_full, drop_in_place<IndexSet<ModuleIdx>>, and the std Vec's __rust_alloc / RawVecInner::reserve / __rust_dealloc. The remaining hashbrown calls are the global generated_init_esm_importee_ids dedup, which is intentionally kept.

Verification

cargo test -p rolldown — 1756 passed, 0 failed; all snapshots unchanged (byte-identical output).

@netlify

netlify Bot commented Jun 20, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

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

@Boshen Boshen marked this pull request as ready for review June 20, 2026 07:04
@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/wrapped-esm-init-allocs (9352a19) 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.

@hyf0 hyf0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Been thinking a to have infra to track every stage's mem/cpu opts

Boshen commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • Jun 20, 7:18 AM 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 20, 7:18 AM UTC: Boshen added this pull request to the Graphite merge queue.
  • Jun 20, 7:24 AM UTC: Merged by the Graphite merge queue.

## What

`wrapped_esm_init_stmt_for_import_record` (scope-hoisting finalize path) performed two heap allocations per wrapped-ESM import record. This removes both, with no behavior change.

### 1. Per-record `FxIndexSet` → `Vec`

`collect_wrapped_esm_init_modules_for_import_record` returned an owned `FxIndexSet<ModuleIdx>` only to dedup owners within a single record. Its sole caller already filters every owner through the persistent `generated_init_esm_importee_ids` (`FxHashSet`) **before** building the init call, so the local set's dedup is fully subsumed by the global one — a duplicate is dropped at the global check and never re-emits. Switching to `Vec<ModuleIdx>` replaces the IndexMap insert/rehash with a plain push and drops one hashmap allocation per record.

### 2. Drop the `.collect::<Vec<_>>()` of init expressions

The expressions were collected into a `std::Vec` only to end the `&mut self` iterator borrow before reaching for `self.ast_factory`. `AstFactory` is `Copy`, so copying it out lets the iterator stay borrowed while nodes are still built through the copy. Driving the iterator by hand means the common 0- and 1-statement cases allocate nothing, and only the rare `(a(), b())` sequence case allocates — straight into the arena. Every branch consumes the iterator to exhaustion, preserving the global-dedup side effects exactly.

## Evidence

`cargo asm -p rolldown --lib wrapped_esm_init_stmt` (aarch64, release `lto = "fat"`):

| | before | after |
|---|---|---|
| instructions | 808 | **731** |
| calls (`bl`) | 40 | **27** |

Removed calls include `indexmap::IndexMap::insert_full`, `drop_in_place<IndexSet<ModuleIdx>>`, and the std `Vec`'s `__rust_alloc` / `RawVecInner::reserve` / `__rust_dealloc`. The remaining `hashbrown` calls are the global `generated_init_esm_importee_ids` dedup, which is intentionally kept.

## Verification

`cargo test -p rolldown` — 1756 passed, 0 failed; all snapshots unchanged (byte-identical output).
@graphite-app graphite-app Bot force-pushed the perf/wrapped-esm-init-allocs branch from 9352a19 to 4b9c35e Compare June 20, 2026 07:19
@graphite-app graphite-app Bot merged commit 4b9c35e into main Jun 20, 2026
34 checks passed
@graphite-app graphite-app Bot deleted the perf/wrapped-esm-init-allocs branch June 20, 2026 07:24
@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.

3 participants