[codex] Optimize source map streaming hot paths#235
Conversation
Merging this PR will improve performance by 50%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | complex_replace_source_map |
9.4 ms | 6.2 ms | +50% |
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/callgrind-performance-30 (fcaa1d1) with main (98138ce)
Footnotes
-
7 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. ↩
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
OriginalSource+ReplaceSourcesource-map path for column-aware maps.ReplaceSource::source()fast path, ASCII UTF-16 accounting, lazyCachedSourceChunks, andConcatSource::add()shortcut.Why
CodSpeed CI did not report an improvement from the previous
complex_replace_source_sourcework because that benchmark is dominated by the unavoidable memory writes for the generated owned string. Local CodSpeed simulation showed Ir improving there, but DL write misses stayed effectively flat.This update targets
complex_replace_source_mapinstead. The CodSpeed-generated temporary Valgrind profile showed the remaining cost inOriginalSource/ReplaceSourcesource-map streaming and mappings encoding. For the hotOriginalSourcecase, we can preserve the same mapping semantics while bypassing the genericChunkscallback path and encoding common no-name mappings directly.This PR continues to ignore the
ReplaceSource::sizebenchmark as requested.Local Validation
cargo fmt --checkcargo testgit diff --checkcargo checkcargo clippy -- -D warningsRUSTDOCFLAGS='-D warnings' cargo doccargo codspeed build --features codspeed --bench benchcodspeed run --mode simulation --skip-upload --repository rstackjs/rspack-sources --profile-folder /tmp/codspeed-current-original-noname -- cargo codspeed run --bench benchObserved from CodSpeed-generated temporary Valgrind files, using clean baseline
98138ceversus this branch:complex_replace_source_mapIr:21,239,638->14,279,866(-32.8%)complex_replace_source_mapDL write misses:6,240->6,191complex_replace_source_map_cached_source_stream_chunksIr:11,011,162->11,010,860(flat after lazy inner chunks)complex_replace_source_sourceIr:1,229,608->789,068(-35.8%), but this is not the primary CI target because memory writes dominate the modeled time