Use unchecked_sub in str indexing#123561
Conversation
| let len = self.end - self.start; | ||
| ptr::slice_from_raw_parts(ptr, len) as *const str | ||
| unsafe { | ||
| let new_len = unchecked_sub(self.end, self.start); |
There was a problem hiding this comment.
Does this get different codegen if you just use the public stdlib unchecked_sub?
There was a problem hiding this comment.
It gets a boatload more MIR, or at least it will once #121571 lands.
There was a problem hiding this comment.
(We really need a "look, nobody cares about the parameter debug info from this method" for things like that and i32:PartialOrd and …)
There was a problem hiding this comment.
No, but it doubles up on the precondition check.
|
Ah, good catch! |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (4e431fa): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @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.899s -> 667.449s (-0.07%) |
@rustbot label: +perf-regression-triaged |
#108763 applied this logic to indexing for slices, but of course
strhas its own separate impl.Found this by skimming over the codegen for https://github.com/oxidecomputer/hubris/; their dist builds enable overflow checks so the lack of
unchecked_subwas producing an impossible-to-hit overflow check and also inhibiting some inlining.r? scottmcm