perf(concat): add owned-merge to avoid re-allocating moved strings#365
perf(concat): add owned-merge to avoid re-allocating moved strings#365hyfdev wants to merge 1 commit into
Conversation
Merging this PR will degrade performance by 3.37%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | from_sourcemaps |
304.9 µs | 323.5 µs | -5.72% |
| ❌ | add_sourcemap_loop |
322.7 µs | 341.1 µs | -5.39% |
| ❌ | serialize[real_medium] |
5 µs | 5.2 µs | -4.46% |
| ❌ | serialize[real_small] |
4.3 µs | 4.5 µs | -2.62% |
| ⚡ | build_single |
5.8 µs | 5.7 µs | +1.53% |
| 🆕 | add_sourcemap_owned |
N/A | 892.1 µs | N/A |
| 🆕 | into_owned |
N/A | 600.2 µs | N/A |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing hyf0:rdsm/concat-owned-merge (fe5d31b) with main (1bb86f5)
Footnotes
-
5 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. ↩
fe5d31b to
0dbe5c4
Compare
`ConcatSourceMapBuilder::add_sourcemap` borrows the input map's strings, so a concatenated result borrows from its inputs and a downstream `into_owned` deep-copies every name/source/sourcesContent string to detach it. Add `add_sourcemap_owned`, which takes a `SourceMap<'static>` by value and moves its strings in via `SourceMap::into_parts`, so the result is already `'static` and the caller can skip `into_owned` — an allocation-per-string becomes a `Vec` move. `add_sourcemap` is unchanged; a test asserts the owned path produces byte-identical output to the borrowed path. A `concat/into_owned` bench tracks the cost the owned path removes.
0dbe5c4 to
d1f76ea
Compare
|
Note on the CodSpeed report above — it's comparing I've since reverted The Benchmark workflow hasn't re-run on the fixed commits ( |
|
Heads-up: this branch now conflicts with #366, which reworked It's a design collision, not just a textual one. This PR's The motivating case is unchanged: when a consumer already owns the input maps (e.g. a bundler joining per-module maps into one chunk map), Since #366 is @Boshen your direction for this file, I'll defer to you on how to handle it — add a "move-in"/owned path on top of the deferred-ownership model, keep the owned merge out of this builder, or close this in favor of a consumer-side approach. Happy to adapt to whatever you prefer. |
2 participants
What
ConcatSourceMapBuilder::add_sourcemapstoresCow::Borrowedviews into each input map, so a concatenated result borrows from its inputs. A consumer that needs an owned ('static) result — e.g. a bundler joining per-module maps into one chunk map — must then callSourceMap::into_owned, which deep-copies every name / source /sourcesContentstring to detach it: one heap allocation per string, on a hot per-chunk path.This adds
ConcatSourceMapBuilder::add_sourcemap_owned, which takes aSourceMap<'static>by value and moves itsnames/sources/source_contentsstorage straight in via the already-publicSourceMap::into_parts. The merged map is then already'static, so the consumer can dropinto_ownedentirely — turning an allocation-per-string into aVecmove.TokenChunkbookkeeping is refactored into a shared privateappend_tokenshelper, so the borrowed and owned paths can't drift.concat/into_owned,concat/add_sourcemap_owned) make the path visible (the existingconcatbenches stop atinto_sourcemap(), so neither criterion nor the allocation tracker could see this cost before).Measured impact
Primary win is allocation count. Validated downstream in rolldown (the main consumer) with a local throwaway
SourceJoiner::join(self)prototype + a Cargo[patch]pointing at this branch, on a realistic 2000-module chunk:cargo allocs·join_with_sourcemap(heap allocations)joinwall-clock (release, mimalloc)Crate-side allocation pre-filter (System allocator, 50× the
real_largeperf fixture): 1355 → 32 allocations.Honest scope: the headline is the allocation reduction (fewer allocations ⇒ less allocator pressure on parallel builds), plus a clean ~25 % / ~100 µs-per-chunk speedup on
join.joinruns once per output chunk and is a small fraction of total bundle time, so the end-to-end build-time delta is small — this is an allocation-efficiency + per-op win, not a large end-to-end number.Two-part note
This method is the enabling half and is safe to land alone (additive, byte-identical output). The rolldown saving is realized only once rolldown bumps the
oxc_sourcemapdependency and adopts the owned path (SourceJoiner::join(self)→add_sourcemap_owned, droppinginto_owned). The companion rolldown adoption is a separate draft PR (rolldown/rolldown#9881); the numbers above were measured with that prototype wiring.Behavior
Full CI-parity checks green: tc39 conformance,
cargo test, clippy-D warnings,cargo doc -D warnings,cargo fmt --check, typos. The owned path is byte-identical to the borrowed path (parallel test).