Add an ASCII (single-byte) fast path in String::push
#20079
Merged
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
`String::push(&mut self, ch: char)` currently has a single code path that calls `Char::encode_utf8`. Perhaps it could be faster for ASCII `char`s, which are represented as a single byte in UTF-8. This commit leaves the method unchanged, adds a copy of it with the fast path, and adds benchmarks to compare them. Results show that the fast path very significantly improves the performance of repeatedly pushing an ASCII `char`, but does not significantly affect the performance for a non-ASCII `char` (where the fast path is not taken). Output of `make check-stage1-collections NO_REBUILD=1 PLEASE_BENCH=1 TESTNAME=string::tests::bench_push` ``` test string::tests::bench_push_char_one_byte ... bench: 59552 ns/iter (+/- 2132) = 167 MB/s test string::tests::bench_push_char_one_byte_with_fast_path ... bench: 6563 ns/iter (+/- 658) = 1523 MB/s test string::tests::bench_push_char_two_bytes ... bench: 71520 ns/iter (+/- 3541) = 279 MB/s test string::tests::bench_push_char_two_bytes_with_slow_path ... bench: 71452 ns/iter (+/- 4202) = 279 MB/s test string::tests::bench_push_str ... bench: 24 ns/iter (+/- 2) test string::tests::bench_push_str_one_byte ... bench: 38910 ns/iter (+/- 2477) = 257 MB/s ``` A benchmark of pushing a one-byte-long `&str` is added for comparison, but its performance [has varied a lot lately]( rust-lang#19640 (comment)). (When the input is fixed, `s.push_str("x")` could be used instead of `s.push('x')`.)
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see CONTRIBUTING.md for more information. |
|
Hmm...Why is |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Dec 21, 2014
`String::push(&mut self, ch: char)` currently has a single code path that calls `Char::encode_utf8`. This adds a fast path for ASCII `char`s, which are represented as a single byte in UTF-8. Benchmarks of stage1 libcollections at the intermediate commit show that the fast path very significantly improves the performance of repeatedly pushing an ASCII `char`, but does not significantly affect the performance for a non-ASCII `char` (where the fast path is not taken). ``` bench_push_char_one_byte 59552 ns/iter (+/- 2132) = 167 MB/s bench_push_char_one_byte_with_fast_path 6563 ns/iter (+/- 658) = 1523 MB/s bench_push_char_two_bytes 71520 ns/iter (+/- 3541) = 279 MB/s bench_push_char_two_bytes_with_slow_path 71452 ns/iter (+/- 4202) = 279 MB/s bench_push_str_one_byte 38910 ns/iter (+/- 2477) = 257 MB/s ``` A benchmark of pushing a one-byte-long `&str` is added for comparison, but its performance [has varied a lot lately](rust-lang#19640 (comment)). (When the input is fixed, `s.push_str("x")` could be used just as well as `s.push('x')`.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
String::push(&mut self, ch: char)currently has a single code path that callsChar::encode_utf8. This adds a fast path for ASCIIchars, which are represented as a single byte in UTF-8.Benchmarks of stage1 libcollections at the intermediate commit show that the fast path very significantly improves the performance of repeatedly pushing an ASCII
char, but does not significantly affect the performance for a non-ASCIIchar(where the fast path is not taken).A benchmark of pushing a one-byte-long
&stris added for comparison, but its performance has varied a lot lately. (When the input is fixed,s.push_str("x")could be used just as well ass.push('x').)