perf(buffer): read byte-aligned bit operands directly as words#8436
Conversation
the direct word path only fired for 64-bit-aligned operands. it now reads the bytes as `u64` via `as_chunks::<8>` for any byte-aligned offset, leaving `iter_padded` for sub-byte. Signed-off-by: Han Damin <miniex@daminstudio.net>
Merging this PR will improve performance by 24.7%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | chunked_bool_canonical_into[(1000, 10)] |
20.4 µs | 35.6 µs | -42.6% |
| ❌ | Simulation | chunked_varbinview_canonical_into[(1000, 10)] |
161.6 µs | 198 µs | -18.38% |
| ❌ | Simulation | chunked_varbinview_into_canonical[(1000, 10)] |
176.6 µs | 213 µs | -17.1% |
| ⚡ | Simulation | bitwise_and_vortex_buffer[65536] |
24.3 µs | 15.1 µs | +60.89% |
| ⚡ | Simulation | bitwise_or_vortex_buffer[65536] |
24.3 µs | 15.2 µs | +60.08% |
| ⚡ | Simulation | bitwise_and_vortex_buffer[2048] |
4.4 µs | 2.9 µs | +50.81% |
| ⚡ | Simulation | bitwise_and_vortex_buffer[1024] |
4.1 µs | 2.8 µs | +48.06% |
| ⚡ | Simulation | bitwise_and_vortex_buffer[16384] |
9.9 µs | 6.7 µs | +47.62% |
| ⚡ | Simulation | bitwise_or_vortex_buffer[16384] |
9.9 µs | 6.8 µs | +46.14% |
| ⚡ | Simulation | bitwise_or_vortex_buffer[1024] |
4 µs | 2.8 µs | +45.94% |
| ⚡ | Simulation | bitwise_or_vortex_buffer[2048] |
4.3 µs | 3 µs | +45.88% |
| ⚡ | Simulation | bitwise_and_vortex_buffer[128] |
4.7 µs | 3.4 µs | +38.44% |
| ⚡ | Simulation | bitwise_or_vortex_buffer[128] |
4.7 µs | 3.4 µs | +36.41% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[128] |
273.6 ns | 215.3 ns | +27.1% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[1024] |
333.9 ns | 275.6 ns | +21.17% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[2048] |
456.9 ns | 398.6 ns | +14.63% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing miniex:perf/bit-collect (d1786df) with develop (e935986)
Footnotes
-
10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
| // bytes straight as words instead of shifting through `iter_padded`. | ||
| if left.offset().is_multiple_of(8) && right.offset().is_multiple_of(8) { |
There was a problem hiding this comment.
| // bytes straight as words instead of shifting through `iter_padded`. | |
| if left.offset().is_multiple_of(8) && right.offset().is_multiple_of(8) { | |
| // bytes straight as words instead of shifting through `iter_padded`. | |
| // TODO(joe): make this into an iterator. | |
| if left.offset().is_multiple_of(8) && right.offset().is_multiple_of(8) { |
Can you add a todo to make this into an iterator
| let lhs = &left.inner().as_slice()[l_start..l_start + n_bytes]; | ||
| let rhs = &right.inner().as_slice()[r_start..r_start + n_bytes]; | ||
|
|
||
| let (lhs_words, lhs_tail) = lhs.as_chunks::<8>(); | ||
| let (rhs_words, rhs_tail) = rhs.as_chunks::<8>(); | ||
|
|
||
| let words = lhs_words | ||
| .iter() | ||
| .zip(rhs_words) | ||
| .map(|(l, r)| (u64::from_le_bytes(*l), u64::from_le_bytes(*r))) | ||
| .chain((!lhs_tail.is_empty()).then(|| (read_u64_le(lhs_tail), read_u64_le(rhs_tail)))); | ||
| return combine_words(words, len, op); |
There was a problem hiding this comment.
I think this will make LLVM generate better code
|
I just merged in a fix to another crate from develop |
the byte-aligned path used a `with_capacity` + push loop. build it from a `TrustedLen` iterator instead so it pre-sizes and lets LLVM generate better code. Signed-off-by: Han Damin <miniex@daminstudio.net>
|
done, made it a TrustedLen iterator |
|
I was talking about the input too, You really have found the difference between an aligned and unaligned byte iterator. Lets land this and maybe we can have a follow up |
joseph-isaacs
left a comment
There was a problem hiding this comment.
Thanks for this. I wonder if we can work this into an iterator left and right without a perf loss?
thanks for merging! lets dig into that in a follow up. also #8425 is the collect+count half stacked on this. i'll rebase it onto develop now that this landed so it's just the fuse, then it's ready for review. |
Summary
Split out of #8425 so each perf change can be looked at on its own, as suggested by @joseph-isaacs. This is the "bit collect" half, no count fusion.
bitwise_binary_oponly took the direct word path when both operands were 64-bit aligned, otherwise falling back toiter_padded. It now reads the backing bytes asu64viaas_chunks::<8>for any byte-aligned offset, leavingiter_paddedfor sub-byte offsets only. This also drops thefrom_trusted_len_iterunsafe for a safeBufferMutpush.Testing
cargo test -p vortex-bufferpasses, plus a per-bit reference test (binary_op_matches_naive) across operand offsets and lengths. It also passes undermiri(strict provenance).fmt+clippy --all-targets --all-featuresclean.I'm Korean, so sorry if any wording reads a little awkward.