mbutrovich commented on code in PR #10136:
URL: https://github.com/apache/arrow-rs/pull/10136#discussion_r4094204293
##########
arrow-buffer/src/util/bit_chunk_iterator.rs:
##########
@@ -324,6 +324,20 @@ impl<'a> BitChunks<'a> {
ceil(self.chunk_len * 64 + self.remainder_len, 8)
}
+ /// Returns the `index`th complete chunk of 64 bits, the value
+ /// [`Self::iter`] yields at that position
+ ///
+ /// # Panics
+ ///
+ /// Panics if `index >= self.chunk_len()`
+ #[inline]
+ pub fn chunk(&self, index: usize) -> u64 {
+ assert!(index < self.chunk_len, "chunk index out of bounds");
+ // Safety: `index < chunk_len`, and the constructor checked the buffer
+ // covers every complete chunk plus the remainder byte
+ unsafe { read_chunk(self.buffer, self.bit_offset, index) }
Review Comment:
I checked this on an Apple M5 Max (portable fallback). In the
`filter_bits_compress` disassembly the check is a single compare and branch per
nonzero mask word, just before the value word is loaded:
```asm
sub x6, x0, #1
cmp x6, x1
b.hs LBB1036_61 ; "chunk index out of bounds" panic
add x6, x15, x6, lsl #3
ldr x7, [x6] ; value word
```
The loop can't vectorize because the per-word `compress` loop depends on the
data. With the `assert!` turned into a `debug_assert!`, the `filter_bits` bench
moved between -7% and +10% across densities with no consistent direction, so I
don't think `chunk_unchecked` is worth the extra unsafe API here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
