-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle array manually in str case conversion methods #52116
Conversation
r? @aidanhs (rust_highfive has picked a reviewer for you, use r? to override) |
Ping from triage, @aidanhs. This PR requires your review. |
Ping from triage, @aidanhs / @rust-lang/libs: This PR requires your review. |
Looks good, thanks! @bors r+ |
📌 Commit ad7621d has been approved by |
⌛ Testing commit ad7621d with merge 8b23f7531ba2a8ef729fe6f63eff5a00aea3519b... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Seems to be #50887. |
Handle array manually in str case conversion methods Avoiding the overhead incurred from `String.extend(char.to_lowercase())` showed a notable performance improvement when I benchmarked it. I tested on these strings: ```rust ALL_LOWER: "loremipsumdolorsitametduosensibusmnesarchumabcdefgh" ALL_UPPER: "LOREMIPSUMDOLORSITAMETDUOSENSIBUSMNESARCHUMABCDEFGH" REALISTIC_UPPER: "LOREM IPSUM DOLOR SIT AMET, DUO SENSIBUS MNESARCHUM" SIGMAS: "ΣΣΣΣΣ ΣΣΣΣΣ ΣΣΣΣΣ ΣΣΣ ΣΣΣΣ, ΣΣΣ ΣΣΣΣΣΣΣΣ ΣΣΣΣΣΣΣΣΣΣ" WORD_UPPER: "Lorem Ipsum Dolor Sit Amet, Duo Sensibus Mnesarchum" ``` the performance improvements of `to_lowercase()` were ``` running 10 tests test tests::all_lower ... bench: 1,752 ns/iter (+/- 49) test tests::all_lower_new ... bench: 1,266 ns/iter (+/- 15) -28% test tests::all_upper ... bench: 1,832 ns/iter (+/- 39) test tests::all_upper_new ... bench: 1,337 ns/iter (+/- 18) -27% test tests::realistic_upper ... bench: 1,993 ns/iter (+/- 14) test tests::realistic_upper_new ... bench: 1,445 ns/iter (+/- 22) -27% test tests::sigmas ... bench: 1,342 ns/iter (+/- 39) test tests::sigmas_new ... bench: 1,226 ns/iter (+/- 16) -9% test tests::word_upper ... bench: 1,899 ns/iter (+/- 12) test tests::word_upper_new ... bench: 1,381 ns/iter (+/- 26) -27% ``` and of `to_uppercase()` ``` running 10 tests test tests::all_lower ... bench: 1,813 ns/iter (+/- 20) test tests::all_lower_new ... bench: 1,321 ns/iter (+/- 16) -27% test tests::all_upper ... bench: 1,629 ns/iter (+/- 22) test tests::all_upper_new ... bench: 1,241 ns/iter (+/- 9) -24% test tests::realistic_upper ... bench: 1,670 ns/iter (+/- 24) test tests::realistic_upper_new ... bench: 1,241 ns/iter (+/- 17) -26% test tests::sigmas ... bench: 2,053 ns/iter (+/- 20) test tests::sigmas_new ... bench: 1,753 ns/iter (+/- 23) -15% test tests::word_upper ... bench: 1,873 ns/iter (+/- 30) test tests::word_upper_new ... bench: 1,412 ns/iter (+/- 25) -25% ``` I gave up on the more advanced method from rust-lang#52061 as it wasn't always a clear improvement and would help in even less cases if this PR was merged.
Rollup of 13 pull requests Successful merges: - #51628 (use checked write in `LineWriter` example) - #52116 (Handle array manually in str case conversion methods) - #52218 (Amend option.take examples) - #52418 (Do not use desugared ident when suggesting adding a type) - #52439 (Revert some changes from #51917 to fix custom libdir) - #52455 (Fix doc comment: use `?` instead of `.unwrap()`) - #52458 (rustc: Fix a suggestion for the `proc_macro` feature) - #52464 (Allow clippy to be installed with make install) - #52472 (rustc: Enable `use_extern_macros` in 2018 edition) - #52477 (Clarify short-circuiting behvaior of Iterator::zip.) - #52480 (Cleanup #24958) - #52487 (Don't build twice the sanitizers on Linux) - #52510 (rustdoc: remove FIXME about macro redirects) Failed merges: r? @ghost
Avoiding the overhead incurred from
String.extend(char.to_lowercase())
showed a notable performance improvement when I benchmarked it.I tested on these strings:
the performance improvements of
to_lowercase()
wereand of
to_uppercase()
I gave up on the more advanced method from #52061 as it wasn't always a clear improvement and would help in even less cases if this PR was merged.