Add inline tags to UninitSlice methods #443
Conversation
This appears to be the primary cause of significant performance regressions in the `prost` test suite in the 0.5 to 0.6 transition. See danburkert/prost#381.
Co-authored-by: Alice Ryhl <alice@ryhl.io>
src/buf/uninit_slice.rs
Outdated
| pub fn write_byte(&mut self, index: usize, byte: u8) { | ||
| assert!(index < self.len()); | ||
|
|
||
| unsafe { self[index..].as_mut_ptr().write(byte) } | ||
| unsafe { self.as_mut_ptr().add(index).write(byte) } |
There was a problem hiding this comment.
I'd expect the bounds check to have been elided, since it's asserted just before... It's not?
There was a problem hiding this comment.
I didn't check the assembly, but empirically I found in #442 that a similar change had a measurable impact on microbenchmarks in the prost repo. It's very possible that the issue there was that the UninitSlice index call was not inlined, and so the similar double check wasn't eligible to be optimized away. Realistically I'm not going to have time to read the ASM here, but I'm happy to back out this part of the change if you'd prefer that.
There was a problem hiding this comment.
Using s[i] vs s.as_ptr().add(i) gets me the same thing with the assert in the function:
example::index:
push rax
cmp rsi, rdx
jbe .LBB7_1
mov al, byte ptr [rdi + rdx]
pop rcx
ret
.LBB7_1:
lea rdi, [rip + .L__unnamed_4]
call std::panicking::begin_panic
ud2
example::ptr:
push rax
cmp rsi, rdx
jbe .LBB8_2
mov al, byte ptr [rdi + rdx]
pop rcx
ret
.LBB8_2:
lea rdi, [rip + .L__unnamed_5]
call std::panicking::begin_panic
ud2There was a problem hiding this comment.
OK, would you like me to switch it back?
There was a problem hiding this comment.
It seems reasonable to me to keep to safe things if they make no performance difference.
There was a problem hiding this comment.
sounds good, updated.
|
Thanks! |
This appears to be the primary cause of significant performance regressions in the `prost` test suite in the 0.5 to 0.6 transition. See danburkert/prost#381.
This appears to be the primary cause of significant performance
regressions in the
prosttest suite in the 0.5 to 0.6 transition. Seedanburkert/prost#381.