Skip to content

perf(sourcemap): owned merge in SourceJoiner::join (4005->5 allocs/chunk)#10250

Merged
graphite-app[bot] merged 1 commit into
mainfrom
perf/sourcemap-owned-join
Jul 13, 2026
Merged

perf(sourcemap): owned merge in SourceJoiner::join (4005->5 allocs/chunk)#10250
graphite-app[bot] merged 1 commit into
mainfrom
perf/sourcemap-owned-join

Conversation

@Boshen

@Boshen Boshen commented Jul 13, 2026

Copy link
Copy Markdown
Member

SourceJoiner::join borrowed every input map's strings into a ConcatSourceMapBuilder, then called into_owned on the merged result to detach it — deep-copying every name/source/sourcesContent string of every module, one heap allocation each, on the per-chunk codegen path.

This moves each source's owned SourceMap into the builder via ConcatSourceMapBuilder::add_sourcemap_owned; the merged map is already 'static, so the trailing into_owned is gone. Sources that can only lend a borrowed map (e.g. one shared through an Arc) fall back to add_sourcemap + copy-on-detach, so output is unchanged.

Measured via tasks/track_memory_allocations (2000-module chunk):

sourcemap/join_with_sourcemap    4005 -> 5 allocs

A second commit refines the join: it takes &mut self — the merge only needs iter_mut() + take_sourcemap(), so it never needed to consume self — and the benches move to iter_batched_ref so the micro-bench stops timing the per-iteration joiner teardown (that teardown, not the concat, was the spurious join_no_sourcemap CodSpeed delta). It also skips the newline scan of the final joined source, whose line count only advances the offset for a following source.

This is the focused, measured core extracted from #9881 (original work by @hyf0). It touches only rolldown_sourcemap plus the benches/allocation snapshot; the ecma_generator/format pipeline is left untouched, so real-build sourcemap output is identical to main. Wiring the chunk renderer to hand join its owned per-module maps — instead of the Arc-shared borrow it copies today — is a follow-up.

@netlify

netlify Bot commented Jul 13, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

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

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

Copy link
Copy Markdown

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: 8feb727553

ℹ️ 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".

Comment thread crates/rolldown_sourcemap/src/source_joiner.rs Outdated
Comment thread tasks/track_memory_allocations/allocs.snap
@codspeed-hq

codspeed-hq Bot commented Jul 13, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 14.45%

⚡ 1 improved benchmark
✅ 6 untouched benchmarks
⏩ 10 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
join_with_sourcemap 10.6 ms 9.3 ms +14.45%

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 perf/sourcemap-owned-join (1bf63cf) with main (705c701)

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.

@hyfdev

hyfdev commented Jul 13, 2026

Copy link
Copy Markdown
Member

No issues from side, I think it's good to merge, but there're some nit comments from others.

@Boshen
Boshen force-pushed the perf/sourcemap-owned-join branch 2 times, most recently from 7b6b3f8 to 7745bfb Compare July 13, 2026 05:47

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

Copy link
Copy Markdown

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: 7745bfbf94

ℹ️ 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".

Comment thread crates/bench/benches/bench.rs
@Boshen
Boshen force-pushed the perf/sourcemap-owned-join branch from 7745bfb to 1bf63cf Compare July 13, 2026 07:12

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

Copy link
Copy Markdown

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: 1bf63cf69e

ℹ️ 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".

Comment thread crates/rolldown_sourcemap/src/source_joiner.rs

Boshen commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • Jul 13, 7:27 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.
  • Jul 13, 7:27 AM UTC: Boshen added this pull request to the Graphite merge queue.
  • Jul 13, 7:33 AM UTC: Merged by the Graphite merge queue.

…unk) (#10250)

`SourceJoiner::join` borrowed every input map's strings into a `ConcatSourceMapBuilder`, then called `into_owned` on the merged result to detach it — deep-copying every name/source/sourcesContent string of every module, one heap allocation each, on the per-chunk codegen path.

This moves each source's owned `SourceMap` into the builder via `ConcatSourceMapBuilder::add_sourcemap_owned`; the merged map is already `'static`, so the trailing `into_owned` is gone. Sources that can only lend a borrowed map (e.g. one shared through an `Arc`) fall back to `add_sourcemap` + copy-on-detach, so output is unchanged.

Measured via `tasks/track_memory_allocations` (2000-module chunk):

```
sourcemap/join_with_sourcemap    4005 -> 5 allocs
```

A second commit refines the join: it takes `&mut self` — the merge only needs `iter_mut()` + `take_sourcemap()`, so it never needed to consume `self` — and the benches move to `iter_batched_ref` so the micro-bench stops timing the per-iteration joiner teardown (that teardown, not the concat, was the spurious `join_no_sourcemap` CodSpeed delta). It also skips the newline scan of the *final* joined source, whose line count only advances the offset for a following source.

This is the focused, measured core extracted from #9881 (original work by @hyf0). It touches only `rolldown_sourcemap` plus the benches/allocation snapshot; the `ecma_generator`/format pipeline is left untouched, so real-build sourcemap output is identical to `main`. Wiring the chunk renderer to hand `join` its owned per-module maps — instead of the `Arc`-shared borrow it copies today — is a follow-up.
@graphite-app
graphite-app Bot force-pushed the perf/sourcemap-owned-join branch from 1bf63cf to 1c36c89 Compare July 13, 2026 07:28
@graphite-app
graphite-app Bot merged commit 1c36c89 into main Jul 13, 2026
33 of 34 checks passed
@graphite-app
graphite-app Bot deleted the perf/sourcemap-owned-join branch July 13, 2026 07:33
@rolldown-guard rolldown-guard Bot mentioned this pull request Jul 15, 2026
shulaoda added a commit that referenced this pull request Jul 15, 2026
## [1.2.0] - 2026-07-15

### 🚀 Features

- dev: skip shipping factories for newly imported top-level modules (#10223) by @h-a-n-a
- dev: per-client ship map for HMR patch sizing (#10208) by @h-a-n-a
- dev: client-side HMR (#10164) by @h-a-n-a
- dev: send a full-reload update to clients when a tsconfig changes (#10262) by @shulaoda
- treat `import.meta['url']` and `import.meta['ROLLUP_FILE_URL_*']` as side-effect free (#10267) by @sapphi-red
- rewrite `import.meta['url']` (#10251) by @sapphi-red
- add `FILE_NOT_FOUND` error (#10220) by @sapphi-red
- treat `import.meta.ROLLUP_FILE_URL_*` as side-effect free (#10217) by @sapphi-red

### 🐛 Bug Fixes

- sourcemap: preserve unmapped boundaries during composition (#10254) by @hyfdev
- `[format]` in `*FileNames` option for ESM format should be `es` instead of `esm` (#10214) by @sapphi-red
- sourcemap: preserve coarse mappings during composition (#10249) by @hyfdev
- rolldown_plugin_vite_import_glob: support tsconfig paths with `import.meta.glob` (#10167) by @sapphi-red
- dev: clear tsconfig caches for bare full builds (#10276) by @shulaoda
- dev: force a full rebuild when a tsconfig changes (#10261) by @shulaoda
- treat rooted drive-less module ids as absolute in preserveModules naming (#10235) by @IWANABETHATGUY
- watch: rebuild when tsconfig files change (#10258) by @shulaoda
- watch: drop tsconfig-merged transform options on each rebuild (#10257) by @shulaoda
- incorrect `EMPTY_IMPORT_META` warning for `import.meta.ROLLUP_FILE_URL_*` for CJS output (#10221) by @sapphi-red
- deconflict: rename CJS locals shadowing wrapped-ESM namespace objects (#9970) by @IWANABETHATGUY
- rolldown: drop the unused runtime module after entry-level external flattening (#10237) by @IWANABETHATGUY
- rolldown: re-propagate has_dynamic_exports to transitive star importers (#10239) by @IWANABETHATGUY
- tree-shaking: tree-shake destructured dynamic import namespace bindings (#10213) by @logaretm
- s390x: use json-escape-simd 3.1.1 for big-endian JSON escaping fix (#10211) by @satyamg1620

### 🚜 Refactor

- dev: move full-reload to client side (#10207) by @h-a-n-a
- readability follow-ups to the ReplaceWith migration (#10286) by @IWANABETHATGUY
- replace take_in-then-write-back with ReplaceWith and by-value moves (#10285) by @Boshen
- share the main resolver's cache with the transformer's tsconfig lookups (#10205) by @shulaoda
- rolldown: extract the ns star-external __reExport emission rule into LinkingMetadata (#10238) by @IWANABETHATGUY
- rolldown: unify link/generate diagnostics into a Diagnostics accumulator (#10234) by @IWANABETHATGUY
- sourcemap_filenames: drop dead sourcemap-filename plumbing (#10189) by @IWANABETHATGUY
- extract external import symbol merging into a method (#10224) by @IWANABETHATGUY
- rolldown: skip CJS namespace merging under strict execution order (#10203) by @hyfdev
- resolve the manual tsconfig per file instead of once at startup (#10200) by @shulaoda
- rolldown: route interop ESM init emission through a shared init-target view (#10202) by @hyfdev
- rolldown: collapse vestigial wrap-kind state and share chunk sort helper (#10201) by @hyfdev

### 📚 Documentation

- show plugin kinds in JSDoc and each hook's description (#10218) by @sapphi-red
- add an explanation about removing imports from external modules without any messages (#10215) by @sapphi-red

### ⚡ Performance

- sourcemap: owned merge in SourceJoiner::join (4005->5 allocs/chunk) (#10250) by @Boshen
- avoid redundant sourcemap string copies in collapse and minify paths (#10093) by @Boshen

### 🧪 Testing

- code-splitting: establish strict-order review baselines (#10287) by @hyfdev
- dev: add hot API test cases (#10181) by @h-a-n-a
- code-splitting: normalize strict execution order variants (#10277) by @hyfdev
- code-splitting: harden strict execution order coverage (#10252) by @hyfdev
- code-splitting: add strict execution order regressions (#10253) by @hyfdev

### ⚙️ Miscellaneous Tasks

- deps: update github actions (#10241) by @renovate[bot]
- deps: update oxc to 0.140.0 (#10274) by @shulaoda
- update Yunfei's GitHub username (#10275) by @hyfdev
- deps: update napi (#10260) by @renovate[bot]
- deps: update test262 submodule for tests (#10266) by @rolldown-guard[bot]
- deps: update dependency vite-plus to v0.2.4 (#10256) by @renovate[bot]
- deps: update napi (#10240) by @renovate[bot]
- deps: update oxc resolver to v11.24.2 (#10245) by @renovate[bot]
- deps: update rust crates (#10244) by @renovate[bot]
- disable Renovate updates for idna_adapter (#10248) by @shulaoda
- deps: update oxc resolver to v11.24.1 (#10232) by @renovate[bot]
- deps: update rust crate oxc_sourcemap to v8.1.1 (#10233) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to ^0.27.0 (#10206) by @renovate[bot]
- deps: upgrade sugar_path to v3 (#10230) by @hyfdev
- add `dist-*` to `.gitignore` in sourcemap-filenames/hash-final-content fixture (#10216) by @sapphi-red
- deps: update dependency rust to v1.97.0 (#10209) by @renovate[bot]

### ❤️ New Contributors

* @satyamg1620 made their first contribution in [#10211](#10211)

Co-authored-by: shulaoda <[email protected]>
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