Add fast path to escape_string_symbol - #160453
Conversation
Co-Authored-By: Matthieu M <matthieum.147192@gmail.com>
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Add fast path to `escape_string_symbol`
|
Do you want it merged now and the std improvement later or do you want to wait on my PR? |
|
I don't really care either way, though it would be nice to see the perf. changes separately, so I wouldn't merge both in one PR. So maybe this one first, and then your PR + switch the function back to |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (67829a9): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -0.8%, secondary -2.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -0.3%, secondary -2.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 491.018s -> 490.024s (-0.20%) |
|
@bors r=the8472 |
|
Marking as rollup=iffy, because this only affects one benchmark, and we have a lot of rollup=never PRs in the queue. @bors rollup=iffy |
…uwer Rollup of 10 perf-sensitive pull requests Successful merges: - #157281 (perf: skip irrelevant foreign impls when building the specialization graph) - #159403 (Next steps for FnDef binder changes (instantiate most FnDef binders)) - #159763 (Optimize crate resolution for large workspace) - #160033 (Speed up `EverInitializedPlaces`) - #160268 (perf: store the fulfillment engine inline in ObligationCtxt) - #160317 (perf: Cache already-checked types in the privacy visitor) - #160399 (interpret: skip deref-projection validity checks when they are not needed) - #160451 (Deduplicate target and host filesearch) - #160453 (Add fast path to `escape_string_symbol`) - #160454 (Add offload guard flags to typeck to prevent perf regressions)
Rollup merge of #160453 - Kobzol:include-blob-opt, r=the8472 Add fast path to `escape_string_symbol` Discussed in #159916. So far used the manual escaping variant. CC @matthieu-m r? the8472
| } | ||
| } | ||
|
|
||
| // Fast-path: if we don't need escaping, just return the original symbol |
There was a problem hiding this comment.
I swear I did this exact optimization somewhere else a long time ago.
|
Verifying that actual perf results after merge match expected results |
This comment has been minimized.
This comment has been minimized.
…uwer Rollup of 10 perf-sensitive pull requests Successful merges: - rust-lang/rust#157281 (perf: skip irrelevant foreign impls when building the specialization graph) - rust-lang/rust#159403 (Next steps for FnDef binder changes (instantiate most FnDef binders)) - rust-lang/rust#159763 (Optimize crate resolution for large workspace) - rust-lang/rust#160033 (Speed up `EverInitializedPlaces`) - rust-lang/rust#160268 (perf: store the fulfillment engine inline in ObligationCtxt) - rust-lang/rust#160317 (perf: Cache already-checked types in the privacy visitor) - rust-lang/rust#160399 (interpret: skip deref-projection validity checks when they are not needed) - rust-lang/rust#160451 (Deduplicate target and host filesearch) - rust-lang/rust#160453 (Add fast path to `escape_string_symbol`) - rust-lang/rust#160454 (Add offload guard flags to typeck to prevent perf regressions)
|
Finished benchmarking commit (5f46ebd): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.4%, secondary 0.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 1.1%, secondary -4.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 489.577s -> 488.242s (-0.27%) |
Discussed in #159916. So far used the manual escaping variant.
CC @matthieu-m
r? the8472