polish

Signed-off-by: Alexander Droste <[email protected]>

Author: 0ax1

vortex-buffer/src/bit/buf.rs

@@ -440,14 +440,13 @@ impl BitBuffer {
             for bit_idx in 0..bits_in_first_byte {
                 let is_set = (byte & (1 << (start_bit_in_byte + bit_idx))) != 0;
                 f(bit_pos, is_set);
-                bit_pos += 1;
             }
 
+            bit_pos += bits_in_first_byte;
             current_byte += 1;
         }
 
-        let bits_remaining = total_bits - bit_pos;
-        let complete_bytes = bits_remaining / 8;
+        let complete_bytes = (total_bits - bit_pos) / 8;
 
         // Process complete bytes.
         for byte_idx in 0..complete_bytes {
@@ -458,87 +457,17 @@ impl BitBuffer {
                 f(bit_pos + byte_idx * 8 + bit_idx, is_set);
             }
         }
+        bit_pos += complete_bytes * 8;
         current_byte += complete_bytes;
 
         // Handle remaining bits at the end.
-        let remaining_bits = bits_remaining % 8;
+        let remaining_bits = (total_bits - bit_pos) % 8;
         if remaining_bits > 0 {
-            let bit_idx_start = bit_pos + complete_bytes * 8;
             let byte = unsafe { *buffer_ptr.add(current_byte) };
 
             for bit_idx in 0..remaining_bits {
                 let is_set = (byte & (1 << bit_idx)) != 0;
-                f(bit_idx_start + bit_idx, is_set);
-            }
-        }
-    }
-
-    /// Iterate through bits in a buffer in reverse.
-    ///
-    /// # Arguments
-    ///
-    /// * `f` - Callback function taking (bit_index, is_set)
-    ///
-    /// # Panics
-    ///
-    /// Panics if the range is outside valid bounds of the buffer.
-    #[inline]
-    pub fn iter_bits_reverse<F>(&self, mut f: F)
-    where
-        F: FnMut(usize, bool),
-    {
-        let total_bits = self.len;
-        if total_bits == 0 {
-            return;
-        }
-
-        let buffer_ptr = self.buffer.as_ptr();
-        let offset = self.offset;
-        let start_byte = offset / 8;
-        let start_bit_in_byte = offset % 8;
-        let mut bit_pos = total_bits;
-
-        let bits_in_first_byte = if start_bit_in_byte > 0 {
-            (8 - start_bit_in_byte).min(total_bits)
-        } else {
-            0
-        };
-
-        let bits_after_first = total_bits - bits_in_first_byte;
-        let first_byte_offset = if start_bit_in_byte > 0 { 1 } else { 0 };
-        let complete_bytes = (bits_after_first - bits_after_first % 8) / 8;
-        let trailing_bits = bits_after_first % 8;
-
-        // Handle remaining bit at the end.
-        if trailing_bits > 0 {
-            let byte = unsafe { *buffer_ptr.add(start_byte + first_byte_offset + complete_bytes) };
-
-            for bit_idx in (0..trailing_bits).rev() {
-                bit_pos -= 1;
-                let is_set = (byte & (1 << bit_idx)) != 0;
-                f(bit_pos, is_set);
-            }
-        }
-
-        // Process complete bytes.
-        for byte_idx in (0..complete_bytes).rev() {
-            let byte = unsafe { *buffer_ptr.add(start_byte + first_byte_offset + byte_idx) };
-
-            for bit_idx in (0..8).rev() {
-                bit_pos -= 1;
-                let is_set = (byte & (1 << bit_idx)) != 0;
-                f(bit_pos, is_set);
-            }
-        }
-
-        // Handle incomplete first byte.
-        if bits_in_first_byte > 0 {
-            let byte = unsafe { *buffer_ptr.add(start_byte) };
-
-            for bit_idx in (0..bits_in_first_byte).rev() {
-                bit_pos -= 1;
-                let is_set = (byte & (1 << (start_bit_in_byte + bit_idx))) != 0;
-                f(bit_pos, is_set);
+                f(bit_pos + bit_idx, is_set);
             }
         }
     }
@@ -657,31 +586,6 @@ mod tests {
         }
     }
 
-    #[rstest]
-    #[case(5)]
-    #[case(8)]
-    #[case(10)]
-    #[case(13)]
-    #[case(16)]
-    #[case(23)]
-    #[case(100)]
-    fn test_iter_bits_reverse(#[case] len: usize) {
-        let buf = BitBuffer::collect_bool(len, |i| i % 2 == 0);
-
-        let mut collected = Vec::new();
-        buf.iter_bits_reverse(|idx, is_set| {
-            collected.push((idx, is_set));
-        });
-
-        assert_eq!(collected.len(), len);
-
-        for (i, (idx, is_set)) in collected.iter().enumerate() {
-            let expected_idx = len - 1 - i;
-            assert_eq!(*idx, expected_idx);
-            assert_eq!(*is_set, expected_idx % 2 == 0);
-        }
-    }
-
     #[rstest]
     #[case(3, 5)]
     #[case(3, 8)]
@@ -704,29 +608,4 @@ mod tests {
             assert_eq!(is_set, (offset + idx) % 2 == 0);
         }
     }
-
-    #[rstest]
-    #[case(3, 5)]
-    #[case(3, 8)]
-    #[case(5, 10)]
-    #[case(2, 16)]
-    fn test_iter_bits_reverse_with_offset(#[case] offset: usize, #[case] len: usize) {
-        let total_bits = offset + len;
-        let buf = BitBuffer::collect_bool(total_bits, |i| i % 2 == 0);
-        let buf_with_offset = BitBuffer::new_with_offset(buf.inner().clone(), len, offset);
-
-        let mut collected = Vec::new();
-        buf_with_offset.iter_bits_reverse(|idx, is_set| {
-            collected.push((idx, is_set));
-        });
-
-        assert_eq!(collected.len(), len);
-
-        for (i, (idx, is_set)) in collected.iter().enumerate() {
-            let expected_idx = len - 1 - i;
-            assert_eq!(*idx, expected_idx);
-            // The bits should match the original buffer at positions offset + expected_idx
-            assert_eq!(*is_set, (offset + expected_idx) % 2 == 0);
-        }
-    }
 }

vortex-compute/benches/expand_buffer.rs

@@ -3,21 +3,30 @@
 
 //! Expand benchmarks for `Buffer`.
 
-use divan::Bencher;
-use vortex_buffer::{Buffer, BufferMut};
+use vortex_buffer::Buffer;
 use vortex_compute::expand::Expand;
 use vortex_mask::Mask;
 
+// buffer size, selectivity
+const PARAMETERS: &[(usize, f64)] = &[
+    (256, 0.1),
+    (256, 0.5),
+    (256, 0.9),
+    (1024, 0.1),
+    (1024, 0.5),
+    (1024, 0.9),
+    (4096, 0.1),
+    (4096, 0.5),
+    (4096, 0.9),
+    (16384, 0.1),
+    (16384, 0.5),
+    (16384, 0.9),
+];
+
 fn main() {
     divan::main();
 }
 
-const BUFFER_SIZE: usize = 1024;
-
-const SELECTIVITIES: &[f64] = &[
-    0.01, 0.10, 0.20, 0.30, 0.40, 0.50, 0.60, 0.70, 0.80, 0.90, 0.99,
-];
-
 fn create_test_buffer<T>(size: usize) -> Buffer<T>
 where
     T: Copy + Default + From<u8> + Send + 'static,
@@ -30,18 +39,6 @@ where
     Buffer::from(data)
 }
 
-fn create_test_buffer_mut<T>(size: usize) -> BufferMut<T>
-where
-    T: Copy + Default + From<u8> + Send + 'static,
-{
-    let mut data = Vec::with_capacity(size);
-    for i in 0..size {
-        #[expect(clippy::cast_possible_truncation)]
-        data.push(T::from((i % 256) as u8));
-    }
-    Buffer::from(data).into_mut()
-}
-
 fn generate_mask(len: usize, selectivity: f64) -> Mask {
     let mut selection = vec![false; len];
     let mut indices: Vec<usize> = (0..len).collect();
@@ -61,35 +58,20 @@ fn generate_mask(len: usize, selectivity: f64) -> Mask {
     Mask::from_iter(selection)
 }
 
-#[divan::bench(types = [u8, u32, u64], args = SELECTIVITIES, sample_count = 1000)]
-fn expand_copy<T: Copy + Default + From<u8> + Send + 'static>(bencher: Bencher, selectivity: f64) {
+#[divan::bench(types = [u8, u32, u64], args = PARAMETERS, sample_count = 1000)]
+fn expand_buffer<T: Copy + Default + From<u8> + Send + 'static>(
+    bencher: divan::Bencher,
+    (buffer_size, selectivity): (usize, f64),
+) {
     bencher
         .with_inputs(|| {
-            let mask = generate_mask(BUFFER_SIZE, selectivity);
+            let mask = generate_mask(buffer_size, selectivity);
             let true_count = mask.true_count();
             let buffer = create_test_buffer::<T>(true_count);
             (buffer, mask)
         })
-        .bench_values(|(buffer, mask)| {
-            let result = buffer.expand(&mask);
+        .bench_refs(|(buffer, mask)| {
+            let result = buffer.expand(mask);
             divan::black_box(result);
         });
 }
-
-#[divan::bench(types = [u8, u32, u64], args = SELECTIVITIES, sample_count = 1000)]
-fn expand_inplace<T: Copy + Default + From<u8> + Send + 'static>(
-    bencher: Bencher,
-    selectivity: f64,
-) {
-    bencher
-        .with_inputs(|| {
-            let mask = generate_mask(BUFFER_SIZE, selectivity);
-            let true_count = mask.true_count();
-            let buffer = create_test_buffer_mut::<T>(true_count);
-            (buffer, mask)
-        })
-        .bench_values(|(mut buffer, mask)| {
-            (&mut buffer).expand(&mask);
-            divan::black_box(buffer);
-        });
-}