perf!: borrow strings in SourceMap, drop Arc<str>#329
Conversation
Merging this PR will improve performance by 5.9%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | add_sourcemap_loop |
363 µs | 339.8 µs | +6.83% |
| ⚡ | from_sourcemaps |
344.3 µs | 322 µs | +6.95% |
| ⚡ | lookup_table[real_medium] |
1.5 µs | 1.4 µs | +4.03% |
| ❌ | serialize[real_large] |
20.3 µs | 20.7 µs | -2.29% |
| ⚡ | lookup_table[real_small] |
1.4 µs | 1.3 µs | +2.18% |
| ❌ | serialize[real_medium] |
4.9 µs | 5.1 µs | -3.37% |
| ❌ | serialize[real_small] |
4.3 µs | 4.4 µs | -2.52% |
| ⚡ | parse[real_large] |
54.7 µs | 50.6 µs | +8.14% |
| ⚡ | from_json_string_inline |
15.8 µs | 14.3 µs | +10.6% |
| ⚡ | parse[real_medium] |
16.5 µs | 15 µs | +9.82% |
| ⚡ | parse[real_small] |
13.2 µs | 11.7 µs | +12.31% |
| ⚡ | parse[real_xlarge] |
1.7 ms | 1.4 ms | +20.59% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing perf/decode-borrowed-deserialization (e26b447) with main (9323530)
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. ↩
6ae304a to
1f1d209
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f1d209c34
ℹ️ 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".
BREAKING CHANGE: `SourceMap` is now `SourceMap<'a>`. All public string fields previously stored as `Arc<str>` are now `Cow<'a, str>` and all accessors that returned `&Arc<str>` now return `&str`. Motivation ========== The previous design held `Vec<Arc<str>>` for `names` / `sources` / `sources_content`. Even after the borrowed-deserialization wrapper in the in-flight version of this PR, every name and source still cost one `Arc::from(&str)` allocation (`ArcInner` header + data copy). And the public `Arc<str>` API forced concat to clone an `Arc` per token — fine for refcount, but still 8 bytes of atomic state per string. The "no Arc, borrow everything" answer is to make the strings actual borrows. With `Cow<'a, str>` and `#[serde(borrow)]`, the deserializer returns a slice into the input JSON buffer for every unescaped string — zero allocations, just a (ptr, len) pair. Escaped strings still allocate (a `String` in `Cow::Owned`), but those are the exception, not the rule. The concat builder benefits the most: it now re-borrows strings from its input `SourceMap`s instead of cloning `Arc`s, so concatenation does zero string allocations. Trade-offs ========== * `SourceMap` now carries a lifetime parameter. For data parsed from JSON, that lifetime ties the map to its input buffer; callers must keep the buffer alive. For maps built via `SourceMapBuilder` / `ConcatSourceMapBuilder`, the lifetime is implied by the inputs (or `'static` for the standalone builder). * `Cow<'_, str>` is 32 bytes per entry vs `Arc<str>`'s 16, so the in-memory `Vec`s are larger; this offsets some of the per-string allocation savings on the parse path. Numbers (vs main with the xlarge fixture) ========================================= | benchmark | before | after | Δ | |-----------------------|---------|---------|--------| | parse/real_large | 4.29 µs | 3.87 µs | -10% | | parse/real_medium | 666 ns | 569 ns | -15% | | parse/real_small | 445 ns | 375 ns | -16% | | concat/from_sourcemaps| ~19 µs | 10.2 µs | -46% | Public API changes ================== * `SourceMap` → `SourceMap<'a>` * `SourceMap::new(file: Option<Arc<str>>, names: Vec<Arc<str>>, ...)` → `(file: Option<Cow<'a, str>>, names: Vec<Cow<'a, str>>, ...)` * `get_file() -> Option<&Arc<str>>` → `Option<&str>` * `get_name(id) -> Option<&Arc<str>>` → `Option<&str>` * `get_source(id) -> Option<&Arc<str>>` → `Option<&str>` * `get_source_content(id) -> Option<&Arc<str>>` → `Option<&str>` * `get_names() -> impl Iterator<Item = &Arc<str>>` → `&str` * `get_sources() -> impl Iterator<Item = &Arc<str>>` → `&str` * `get_source_contents() -> impl Iterator<Item = Option<&Arc<str>>>` → `Option<&str>` * `SourceViewToken<'a>` → `SourceViewToken<'sm, 'data>` (two lifetimes) * `ConcatSourceMapBuilder` → `ConcatSourceMapBuilder<'a>`
1f1d209 to
bc3e0a2
Compare
Removed the `#[serde(default)]` attributes I'd added to fields that the sourcemap spec actually requires (`sources`, `mappings`, etc.), so malformed JSON missing those fields now errors out as it does on main. Only `names` keeps `#[serde(default)]`, matching the original `JSONSourceMap` shape. Restores the `sourcesMissing` tc39 spec test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adb4af904b
ℹ️ 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".
Allocate owned copies of every borrowed string in a `SourceMap` so that the result is `SourceMap<'static>`, detached from the original JSON input buffer (or from a concat builder's input lifetime). Discovered while linking the new lifetime-parameterized SourceMap into downstream consumers (oxc, rolldown). Several places need a concat or parse result to outlive its source data: the previous Arc-based API let them share cheaply, the new Cow-based API needs an explicit copy step. `into_owned` is that step.
…sforms Add `SourceMapParts<'a>` — a destructured form of `SourceMap<'a>` that exposes its owned `Cow<'a, str>` fields. Returned by `SourceMap::into_parts`, consumed by `SourceMap::from_parts` / `From<SourceMapParts>`. Use case: downstream code that wants to *transform* a sourcemap (rewrite tokens, swap in a different `file`, drop `token_chunks`) without going through string accessors. The previous API only exposed `&str` views, which forced a clone of every name / source / sourcesContent string when the caller wanted to take ownership and reassemble. With `into_parts`, that becomes a `Vec<Cow<'a, str>>` move — zero per-string allocations. Pairs naturally with `set_file` / `set_sources` etc. for in-place mutation when the caller only needs to change one field.
Implements the hybrid design (Option 3): - `SourceMap` (no lifetime, buf+offsets) — primary type, used wherever the map needs to outlive its input or be stored as a struct field. - `BorrowedSourceMap<'a>` (Cow<'a, str>) — zero-copy parse view, used when the caller wants to read tokens / strings without paying for an owned string buffer. `SourceMap::from_json_string` now goes through the borrowed parse path internally (skipping the serde_json String allocations for unescaped fields), then copies into the owned buffer. So parse-into-owned has zero per-string heap allocations. API surface: ```rust // Direct: parses with allocations (one buf for all strings) let owned = SourceMap::from_json_string(json)?; // Zero-copy: parses with Cow::Borrowed where possible let borrowed = BorrowedSourceMap::from_json_string(json)?; // Same accessors as SourceMap (return &str) let name = borrowed.get_name(0); // Promote when you need to detach from `json`: let owned: SourceMap = borrowed.into_owned(); ``` Trade-off vs. PR #329's single `SourceMap<'a>`: - API: no lifetime parameter on the primary type (resolves the "<'static> noise" in rolldown). - Allocations: ~5 per owned SourceMap (buf + offset arrays) instead of O(names + sources + sources_content). - Wall-clock parse: ~10-25% slower than PR #329 because every string gets memcpy'd into the buffer at construction time. - Concat: ~65% slower than PR #329 for the same reason. Users who want max parse throughput on read-only workloads use `BorrowedSourceMap`; users who need owned storage use `SourceMap`. Numbers (xlarge fixture): - parse/real_xlarge: 145 µs (vs 119 µs in #329, +22%) - parse_borrowed/real_xlarge: 135 µs (vs 119 µs in #329, +13%) - concat/from_sourcemaps: 17 µs (vs 10 µs in #329, +65%) - serialize/real_xlarge: 44.6 µs (matches #329)
|
Already merged via #334 (squash). Closing. |
Summary
Lifetime-parameterize
SourceMapasSourceMap<'a>and switch every ownedArc<str>toCow<'a, str>. Strings parsed from a JSON buffer are now zero-copy borrows into that buffer; concat builders re-borrow from their inputs; only fields explicitly built from owned data allocate.This is a breaking change — every consumer that names
SourceMapas a type now writesSourceMap<'a>(orSourceMap<'static>for owned). Warrants a 7.0 release.Why
The previous design held
Vec<Arc<str>>fornames/sources/sources_content. Every parse, even with a borrowed-deserialization wrapper, still allocated oneArc::from(&str)per string. The publicArc<str>API also added an atomic refcount per string that was rarely needed — most consumers (oxc, rolldown) put the wholeSourceMapinOption<SourceMap>and never shared individual strings across owners.With
Cow<'a, str>+#[serde(borrow)], the deserializer returns a slice into the input JSON buffer for every unescaped string — zero allocations, just(ptr, len). Escaped strings (typically only insidesourcesContent) still allocate aStringviaCow::Owned, but they're the exception.API surface (the breaking bits)
SourceMap→SourceMap<'a>SourceMap::new(file: Option<Arc<str>>, names: Vec<Arc<str>>, ...)→(file: Option<Cow<'a, str>>, names: Vec<Cow<'a, str>>, ...)&Arc<str>now return&str:get_file,get_name,get_source,get_source_content,get_source_and_contentget_names,get_sources,get_source_contents(iterator items)SourceViewToken<'a>→SourceViewToken<'sm, 'data>ConcatSourceMapBuilder→ConcatSourceMapBuilder<'a>SourceMapBuilder::into_sourcemapreturnsSourceMap<'static>New helpers for the owned case
SourceMap::into_owned() -> SourceMap<'static>— detach from the input buffer by upgrading everyCow::BorrowedtoCow::Owned.Cow::Ownedentries move for free.SourceMap::into_parts() -> SourceMapParts<'a>/from_parts(SourceMapParts<'a>) -> Self— destructure and reassemble without going through accessors. Lets downstream code rewrite tokens or swap one field while moving the rest of the strings (zero per-string allocations).Numbers (vs main, with the xlarge fixture from #328)
The concat win is the headline —
Arc::cloneper token (refcount bump) is replaced withCow::Borrowed(just copy&str), so concat does zero string allocations.Verified against downstream consumers
Patched
oxcandrolldownlocally to use this branch:oxc_codegen): 3 small lifetime annotations (-> SourceMap<'static>in the builder,Option<SourceMap<'static>>inCodegenReturn, one test fix).SourceMap→SourceMap<'static>on struct fields. The two interesting rewrites:rolldown_sourcemap::adjust_sourcemap_dst_linesnow moves all strings viainto_parts()instead of cloning — zero allocations beyond the new tokensVec.rolldown_binding'sBindingMagicString::source_map(opts)with anopts.fileoverride used to rebuild the whole map (cloning every name / source / content); now it mutates in place viaset_file— zero string allocations.Both build clean;
rolldown_sourcemaptests pass.Caveats
Cow<'_, str>is 32 bytes vsArc<str>'s 16, so the in-memoryVecs are 2× larger. The savings from skipping per-string allocations offset that on parse; concat sees only the win.Arc::clonefor shared ownership of individual strings need to wrap the wholeSourceMapinArc<SourceMap>instead, or callinto_owned()to detach a copy.