Avoid unnecessary allocations in float_lit and integer_lit.
#55384
Conversation
This commit avoids an allocation when parsing any float and integer literals that don't involved underscores. This reduces the number of allocations done for the `tuple-stress` benchmark by 10%, reducing its instruction count by just under 1%.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| // Strip underscores without allocating a new String unless necessary. | ||
| let s2; | ||
| let s = if s.chars().any(|c| c == '_') { | ||
| s2 = s.chars().filter(|&c| c != '_').collect::<String>(); |
There was a problem hiding this comment.
Hm, since underscores are in ASCII, couldn't we into_bytes, retain, and then String::from_utf8, maybe unchecked?
There was a problem hiding this comment.
That would work, but it's not clear to me that it would be any faster; it might even be slower because there are two allocations involved? (Because it converts to a vector, and then back to a new String?)
Besides, this is the cold path, so I'm satisfied with reusing the existing code, which until now had been considered good enough for the hot path :)
There was a problem hiding this comment.
Just FYI, String::from_utf8 consumes the vector given to it, so no additional allocation would happen.
|
|
||
| // Strip underscores without allocating a new String unless necessary. | ||
| let s2; | ||
| let s = if s.chars().any(|c| c == '_') { |
There was a problem hiding this comment.
It would be interesting to see if std::slice::memchr::memchr(b'_', s.as_bytes()).is_some() is faster than s.chars().any().
There was a problem hiding this comment.
I tried it. Cachegrind says it's marginally more instructions: 22,526,559,505 up from 22,516,683,388. So I think I'll stick with the original.
|
Although I haven't changed the code, IMO I've addressed the comments above, so this is ready for re-review. |
|
Thanks, @nnethercote! @bors r+ |
|
📌 Commit eb637d2 has been approved by |
bors
added
S-waiting-on-bors
…t_lit, r=michaelwoerister Avoid unnecessary allocations in `float_lit` and `integer_lit`. This commit avoids an allocation when parsing any float and integer literals that don't involved underscores. This reduces the number of allocations done for the `tuple-stress` benchmark by 10%, reducing its instruction count by just under 1%.
Rollup of 9 pull requests Successful merges: - #54965 (update tcp stream documentation) - #55269 (fix typos in various places) - #55384 (Avoid unnecessary allocations in `float_lit` and `integer_lit`.) - #55423 (back out bogus `Ok`-wrapping suggestion on `?` arm type mismatch) - #55426 (Make a bunch of trivial methods of NonNull be `#[inline]`) - #55438 (Avoid directly catching BaseException in bootstrap configure script) - #55439 (Remove unused sys import from generate-deriving-span-tests) - #55440 (Remove unreachable code in hasClass function in Rustdoc) - #55447 (Fix invalid path in generate-deriving-span-tests.py.) Failed merges: r? @ghost
This commit avoids an allocation when parsing any float and integer
literals that don't involved underscores.
This reduces the number of allocations done for the
tuple-stressbenchmark by 10%, reducing its instruction count by just under 1%.