Skip to content

Add fast path to escape_string_symbol - #160453

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
Kobzol:include-blob-opt
Aug 4, 2026
Merged

Add fast path to escape_string_symbol#160453
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
Kobzol:include-blob-opt

Conversation

@Kobzol

Copy link
Copy Markdown
Member

Discussed in #159916. So far used the manual escaping variant.

CC @matthieu-m

r? the8472

Co-Authored-By: Matthieu M <matthieum.147192@gmail.com>
@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 3, 2026
@Kobzol

Copy link
Copy Markdown
MemberAuthor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 3, 2026
@rust-bors

This comment has been minimized.

rust-borsBot pushed a commit that referenced this pull request Aug 3, 2026
Add fast path to `escape_string_symbol`
@the8472

Copy link
Copy Markdown
Member

Do you want it merged now and the std improvement later or do you want to wait on my PR?

@Kobzol

Copy link
Copy Markdown
MemberAuthor

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 escape_default, and ideally perf. should stay neutral.

@the8472the8472 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, LGTM.

r=me once perf results are back and as expected

View changes since this review

@rust-bors

rust-borsBot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 67829a9 (67829a9875f34c2eb6ef188686f3d7b8876dd3db)
Base parent: 7c329d6 (7c329d6c76e11ca40c5673818ab0439c1be8962c)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (67829a9): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking 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
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

meanrangecount
Regressions ❌
(primary)
--0
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
-19.2%[-34.8%, -4.4%]14
All ❌✅ (primary)--0

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.

meanrangecount
Regressions ❌
(primary)
0.8%[0.6%, 1.0%]2
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
-2.5%[-3.8%, -1.1%]2
Improvements ✅
(secondary)
-2.0%[-5.5%, -0.4%]4
All ❌✅ (primary)-0.8%[-3.8%, 1.0%]4

Cycles

Results (primary -0.3%, secondary -2.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
1.0%[0.5%, 1.8%]3
Regressions ❌
(secondary)
0.7%[0.4%, 1.5%]5
Improvements ✅
(primary)
-1.0%[-2.3%, -0.4%]6
Improvements ✅
(secondary)
-4.4%[-15.4%, -0.7%]7
All ❌✅ (primary)-0.3%[-2.3%, 1.8%]9

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 491.018s -> 490.024s (-0.20%)
Artifact size: 390.29 MiB -> 391.07 MiB (0.20%)

@rustbotrustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 3, 2026
@Kobzol

Copy link
Copy Markdown
MemberAuthor

@bors r=the8472

@rust-bors

rust-borsBot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 9605762 has been approved by the8472

It is now in the queue for this repository.

@rust-borsrust-borsBot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 4, 2026
@Kobzol

Copy link
Copy Markdown
MemberAuthor

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

rust-borsBot pushed a commit that referenced this pull request Aug 4, 2026
…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)
@rust-bors
rust-borsBot merged commit 7636221 into rust-lang:mainAug 4, 2026
14 checks passed
@rustbotrustbot added this to the 1.99.0 milestone Aug 4, 2026
rust-timer added a commit that referenced this pull request Aug 4, 2026
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
@Kobzol
Kobzol deleted the include-blob-opt branch August 4, 2026 16:39
}
}

// Fast-path: if we don't need escaping, just return the original symbol

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I swear I did this exact optimization somewhere else a long time ago.

@JonathanBrouwer

Copy link
Copy Markdown
Member

Verifying that actual perf results after merge match expected results
@rust-timer build 5f46ebd

@rust-timer

This comment has been minimized.

WhySoBad pushed a commit to WhySoBad/miri that referenced this pull request Aug 5, 2026
…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)
@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (5f46ebd): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking 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
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

meanrangecount
Regressions ❌
(primary)
--0
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
-19.2%[-34.8%, -4.4%]14
All ❌✅ (primary)--0

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.

meanrangecount
Regressions ❌
(primary)
0.8%[0.6%, 1.1%]6
Regressions ❌
(secondary)
0.6%[0.4%, 1.1%]16
Improvements ✅
(primary)
-0.7%[-1.0%, -0.5%]2
Improvements ✅
(secondary)
-0.4%[-0.4%, -0.4%]1
All ❌✅ (primary)0.4%[-1.0%, 1.1%]8

Cycles

Results (primary 1.1%, secondary -4.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
1.4%[0.6%, 2.2%]5
Regressions ❌
(secondary)
1.2%[0.4%, 2.8%]6
Improvements ✅
(primary)
-0.5%[-0.5%, -0.5%]1
Improvements ✅
(secondary)
-8.3%[-20.8%, -0.4%]10
All ❌✅ (primary)1.1%[-0.5%, 2.2%]6

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 489.577s -> 488.242s (-0.27%)
Artifact size: 390.24 MiB -> 390.27 MiB (0.01%)

flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Aug 17, 2026
…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)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@Kobzol@rust-timer@the8472@JonathanBrouwer@nnethercote@rustbot