Optimize core::char::CaseMappingIter#122616
Conversation
Godbolt says this saves a few instructions…
|
r? @Nilstrieb rustbot has assigned @Nilstrieb. Use r? to explicitly pick a reviewer |
There was a problem hiding this comment.
The changes seem fine, but this CaseMappingIter seems pretty complex for doing a simple operation. What's the performance of just using an array::IntoIter and stopping when that one sees a \0?
I did a bit of digging on the origin of this type and it seems to have been around forever (addaa5b).
Makes the iterator 2*usize larger, but I doubt that matters much. In exchange, we save a lot on instruction count. In the absence of delegation syntax, we must forward all the specialized impls manually…
core::char::CaseMappingIter layoutcore::char::CaseMappingIter
|
@Nilstrieb That is a much better approach, adopted. @rustbot -S-waiting-on-author S-waiting-on-review |
|
Thanks! |
| One(char), | ||
| Zero, | ||
| } | ||
| struct CaseMappingIter(core::array::IntoIter<char, 3>); |
There was a problem hiding this comment.
Unsure: are these something that people would commonly keep in a struct? Because while I do like the array::IntoIter phrasing of this, it means that iter will be 2×usize bigger than it used to be.
Feels like probably not, more likely something they'd just use directly and toss away, rather than use partially and keep around, but figured I'd mention it just in case.
(I wish we had an ArraySize<N> type that could be smaller for small N.)
…ut, r=Nilstrieb Optimize `core::char::CaseMappingIter` Godbolt says this saves a few instructions… `@rustbot` label T-libs A-layout C-optimization
|
💔 Test failed - checks-actions |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
@bors retry #122671 (comment) |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (1c19595): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 667.865s -> 668.406s (0.08%) |
Godbolt says this saves a few instructions…
@rustbot label T-libs A-layout C-optimization