Optimize CharIndices::advance_by by thaliaarchi · Pull Request #137761 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Optimize CharIndices::advance_by by delegating to the optimized Chars::advance_by. Since this can also be used to count chars, add a benchmark for it.
(Interator::nth uses Iterator::advance_by.)
cc @orlp
r? @scottmcm
rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
labels
Benchmarking shows that .chars().advance_by(usize::MAX) compares favorably against .chars().count() for counting chars.
Also, whether the advance_by argument is a constant usize::MAX or black_boxed makes no difference in performance.
x bench library/coretests --stage 1 --test-args str::char_count
str::char_count::emoji_huge::case00_chars_count 26895.51ns/iter +/- 69143.99
str::char_count::emoji_huge::case01_chars_advance_by 39908.10ns/iter +/- 4735.18
str::char_count::emoji_huge::case02_filter_count_cont_bytes 143432.06ns/iter +/- 6447.94
str::char_count::emoji_huge::case03_iter_chars_increment 294685.87ns/iter +/- 19699.59
str::char_count::emoji_huge::case04_manual_char_len 294636.37ns/iter +/- 20001.07
str::char_count::emoji_large::case00_chars_count 374.25ns/iter +/- 66.28
str::char_count::emoji_large::case01_chars_advance_by 586.73ns/iter +/- 135.90
str::char_count::emoji_large::case02_filter_count_cont_bytes 2226.21ns/iter +/- 104.03
str::char_count::emoji_large::case03_iter_chars_increment 4584.05ns/iter +/- 270.57
str::char_count::emoji_large::case04_manual_char_len 4585.11ns/iter +/- 258.49
str::char_count::emoji_medium::case00_chars_count 60.87ns/iter +/- 26.87
str::char_count::emoji_medium::case01_chars_advance_by 78.37ns/iter +/- 6.77
str::char_count::emoji_medium::case02_filter_count_cont_bytes 288.41ns/iter +/- 25.85
str::char_count::emoji_medium::case03_iter_chars_increment 583.22ns/iter +/- 21.35
str::char_count::emoji_medium::case04_manual_char_len 589.36ns/iter +/- 33.22
str::char_count::emoji_small::case00_chars_count 21.08ns/iter +/- 1.09
str::char_count::emoji_small::case01_chars_advance_by 12.49ns/iter +/- 0.75
str::char_count::emoji_small::case02_filter_count_cont_bytes 29.74ns/iter +/- 2.08
str::char_count::emoji_small::case03_iter_chars_increment 36.68ns/iter +/- 2.66
str::char_count::emoji_small::case04_manual_char_len 37.47ns/iter +/- 1.30
str::char_count::emoji_tiny::case00_chars_count 7.77ns/iter +/- 1.00
str::char_count::emoji_tiny::case01_chars_advance_by 5.81ns/iter +/- 0.68
str::char_count::emoji_tiny::case02_filter_count_cont_bytes 6.24ns/iter +/- 0.71
str::char_count::emoji_tiny::case03_iter_chars_increment 3.44ns/iter +/- 0.16
str::char_count::emoji_tiny::case04_manual_char_len 3.87ns/iter +/- 0.80
str::char_count::en_huge::case00_chars_count 25270.06ns/iter +/- 3517.67
str::char_count::en_huge::case01_chars_advance_by 36994.25ns/iter +/- 8664.94
str::char_count::en_huge::case02_filter_count_cont_bytes 136098.38ns/iter +/- 3237.48
str::char_count::en_huge::case03_iter_chars_increment 190540.45ns/iter +/- 375994.04
str::char_count::en_huge::case04_manual_char_len 286710.43ns/iter +/- 33275.76
str::char_count::en_large::case00_chars_count 355.26ns/iter +/- 20.44
str::char_count::en_large::case01_chars_advance_by 571.21ns/iter +/- 17.40
str::char_count::en_large::case02_filter_count_cont_bytes 2123.16ns/iter +/- 503.87
str::char_count::en_large::case03_iter_chars_increment 2921.19ns/iter +/- 356.64
str::char_count::en_large::case04_manual_char_len 4431.56ns/iter +/- 536.13
str::char_count::en_medium::case00_chars_count 55.92ns/iter +/- 8.05
str::char_count::en_medium::case01_chars_advance_by 73.93ns/iter +/- 4.66
str::char_count::en_medium::case02_filter_count_cont_bytes 282.48ns/iter +/- 139.73
str::char_count::en_medium::case03_iter_chars_increment 377.55ns/iter +/- 72.04
str::char_count::en_medium::case04_manual_char_len 556.65ns/iter +/- 66.79
str::char_count::en_small::case00_chars_count 18.46ns/iter +/- 0.93
str::char_count::en_small::case01_chars_advance_by 12.49ns/iter +/- 6.43
str::char_count::en_small::case02_filter_count_cont_bytes 18.91ns/iter +/- 0.52
str::char_count::en_small::case03_iter_chars_increment 19.19ns/iter +/- 1.00
str::char_count::en_small::case04_manual_char_len 41.81ns/iter +/- 20.10
str::char_count::en_tiny::case00_chars_count 7.78ns/iter +/- 3.34
str::char_count::en_tiny::case01_chars_advance_by 13.17ns/iter +/- 0.98
str::char_count::en_tiny::case02_filter_count_cont_bytes 6.23ns/iter +/- 0.24
str::char_count::en_tiny::case03_iter_chars_increment 5.34ns/iter +/- 0.79
str::char_count::en_tiny::case04_manual_char_len 8.90ns/iter +/- 0.50
str::char_count::ru_huge::case00_chars_count 23521.49ns/iter +/- 4525.93
str::char_count::ru_huge::case01_chars_advance_by 34957.96ns/iter +/- 16442.47
str::char_count::ru_huge::case02_filter_count_cont_bytes 128222.60ns/iter +/- 9503.80
str::char_count::ru_huge::case03_iter_chars_increment 176097.38ns/iter +/- 26276.12
str::char_count::ru_huge::case04_manual_char_len 231660.15ns/iter +/- 28604.21
str::char_count::ru_large::case00_chars_count 337.69ns/iter +/- 32.97
str::char_count::ru_large::case01_chars_advance_by 565.93ns/iter +/- 45.96
str::char_count::ru_large::case02_filter_count_cont_bytes 1989.00ns/iter +/- 53.45
str::char_count::ru_large::case03_iter_chars_increment 2709.66ns/iter +/- 241.61
str::char_count::ru_large::case04_manual_char_len 3510.50ns/iter +/- 617.17
str::char_count::ru_medium::case00_chars_count 56.29ns/iter +/- 3.25
str::char_count::ru_medium::case01_chars_advance_by 117.97ns/iter +/- 3.20
str::char_count::ru_medium::case02_filter_count_cont_bytes 262.44ns/iter +/- 69.23
str::char_count::ru_medium::case03_iter_chars_increment 341.12ns/iter +/- 157.75
str::char_count::ru_medium::case04_manual_char_len 395.95ns/iter +/- 730.95
str::char_count::ru_small::case00_chars_count 16.23ns/iter +/- 0.95
str::char_count::ru_small::case01_chars_advance_by 7.02ns/iter +/- 0.23
str::char_count::ru_small::case02_filter_count_cont_bytes 16.08ns/iter +/- 1.56
str::char_count::ru_small::case03_iter_chars_increment 17.23ns/iter +/- 1.29
str::char_count::ru_small::case04_manual_char_len 17.67ns/iter +/- 1.08
str::char_count::ru_tiny::case00_chars_count 8.52ns/iter +/- 0.40
str::char_count::ru_tiny::case01_chars_advance_by 8.51ns/iter +/- 1.06
str::char_count::ru_tiny::case02_filter_count_cont_bytes 7.39ns/iter +/- 0.40
str::char_count::ru_tiny::case03_iter_chars_increment 6.33ns/iter +/- 0.82
str::char_count::ru_tiny::case04_manual_char_len 6.79ns/iter +/- 0.59
str::char_count::zh_huge::case00_chars_count 21915.29ns/iter +/- 1553.56
str::char_count::zh_huge::case01_chars_advance_by 32533.42ns/iter +/- 2794.91
str::char_count::zh_huge::case02_filter_count_cont_bytes 119467.06ns/iter +/- 2970.87
str::char_count::zh_huge::case03_iter_chars_increment 319110.20ns/iter +/- 15128.61
str::char_count::zh_huge::case04_manual_char_len 316619.37ns/iter +/- 14316.98
str::char_count::zh_large::case00_chars_count 316.41ns/iter +/- 45.45
str::char_count::zh_large::case01_chars_advance_by 492.66ns/iter +/- 20.85
str::char_count::zh_large::case02_filter_count_cont_bytes 1850.94ns/iter +/- 70.57
str::char_count::zh_large::case03_iter_chars_increment 4927.22ns/iter +/- 145.58
str::char_count::zh_large::case04_manual_char_len 4944.76ns/iter +/- 480.41
str::char_count::zh_medium::case00_chars_count 55.57ns/iter +/- 5.57
str::char_count::zh_medium::case01_chars_advance_by 67.27ns/iter +/- 1.85
str::char_count::zh_medium::case02_filter_count_cont_bytes 242.38ns/iter +/- 20.20
str::char_count::zh_medium::case03_iter_chars_increment 599.00ns/iter +/- 57.02
str::char_count::zh_medium::case04_manual_char_len 593.71ns/iter +/- 19.71
str::char_count::zh_small::case00_chars_count 19.19ns/iter +/- 0.66
str::char_count::zh_small::case01_chars_advance_by 9.03ns/iter +/- 3.14
str::char_count::zh_small::case02_filter_count_cont_bytes 17.37ns/iter +/- 1.16
str::char_count::zh_small::case03_iter_chars_increment 23.59ns/iter +/- 0.93
str::char_count::zh_small::case04_manual_char_len 24.84ns/iter +/- 1.11
str::char_count::zh_tiny::case00_chars_count 8.12ns/iter +/- 0.32
str::char_count::zh_tiny::case01_chars_advance_by 7.63ns/iter +/- 0.48
str::char_count::zh_tiny::case02_filter_count_cont_bytes 6.65ns/iter +/- 0.44
str::char_count::zh_tiny::case03_iter_chars_increment 5.00ns/iter +/- 1.01
str::char_count::zh_tiny::case04_manual_char_len 5.26ns/iter +/- 0.96
Benchmarking shows that
.chars().advance_by(usize::MAX)compares favorably against.chars().count()for counting chars.
Not this PR, but if that's better than the super::count::count_chars(self.as_str()) that Chars::count is using, might as well update whatever count_chars is doing...
It seems Chars::count is still a little faster than Chars::advance_by in most cases.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good to me, but just to be sure can you add tests either for advance_by or nth in library/alloc/tests/str.rs?
I didn't spot any, and since it doesn't seem to be exercised by a doc-comment either, it'd be good to have a couple of basic things.
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Chars::advance_by is already optimized. Delegate to it.
I've added a test for using Chars::advance_by for counting chars. I have yet to write tests for it in a general sense (mostly because it's code I didn't write and I haven't yet taken the time to understand it completely to write good tests which would exercise it fully).
Status: Awaiting review from the assignee but also interested parties.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
Labels
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.