refactor: simplify SourceMapBuilder string storage#361
Conversation
Merging this PR will improve performance by 4.52%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | build_single |
6.2 µs | 5.8 µs | +6.75% |
| ⚡ | serialize[real_medium] |
5.1 µs | 5 µs | +2.33% |
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 codex/simplify-sourcemap-builder-borrows (7805d07) with main (26834e5)
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. ↩
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7805d07c0a
ℹ️ 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".
| /// `source_content` is borrowed for `'a` — the content is never copied. | ||
| pub fn set_source_and_content(&mut self, source: Cow<'a, str>, source_content: &'a str) -> u32 { | ||
| /// The source name and source content are borrowed for `'a` — neither is copied. | ||
| pub fn set_source_and_content(&mut self, source: &'a str, source_content: &'a str) -> u32 { |
There was a problem hiding this comment.
When callers build source names from Path::to_string_lossy() or a formatted String inside a loop and then call into_owned_sourcemap, the previous Cow<'a, str> parameter let the builder take ownership of those transient source names. Requiring &'a str now forces downstream code to keep every generated String alive until the builder is consumed, and existing Cow call sites no longer compile, so this is a public API and usability regression for generated/non-UTF8 paths while building owned sourcemaps.
Useful? React with 👍 / 👎.
Mirror the `SourceMapBuilder` change in #361: store `&'a str` internally instead of `Cow<'a, str>` and defer the borrow-vs-own decision to the terminal methods. Adds `into_owned_sourcemap` alongside `into_sourcemap` to match the `SourceMapBuilder` surface. The public API is backward compatible (fields are `pub(crate)`, existing method signatures are unchanged).
1 participant
Refactor
SourceMapBuilderto store borrowed strings directly and allocate owned strings only ininto_owned_sourcemap.Checks:
cargo testcargo clippy --all-targets --all-featurestypos