Merge #125

125: Speed up bitmap iteration r=Kerollmops a=saik0

A low hanging fruit I found:

Bitmap iteration currently tests every bit for each non-zero element

This PR skips runs of zeros by inspecting the least significant bit, which can be computed with one instruction on most architectures.

```
iter bitmap 1..10_000   time:   [48.602 us 48.787 us 48.978 us]                                   
                        change: [-44.893% -44.519% -44.164%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  3 (3.00%) high mild
  1 (1.00%) high severe

iter bitmap sparse      time:   [4.8238 us 4.8478 us 4.8737 us]                                
                        change: [-3.6741% -3.1564% -2.6669%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

iter bitmap dense       time:   [160.14 us 160.73 us 161.29 us]                              
                        change: [-27.912% -27.207% -26.571%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

iter bitmap minimal     time:   [18.447 us 18.512 us 18.579 us]                                 
                        change: [-2.8722% -2.1247% -1.3247%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) high mild
  5 (5.00%) high severe

iter bitmap full        time:   [312.97 us 314.10 us 315.35 us]                             
                        change: [-18.450% -17.937% -17.471%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe

Benchmarking iter parsed: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 7.8s, enable flat sampling, or reduce sample count to 50.
iter parsed             time:   [1.5311 ms 1.5371 ms 1.5436 ms]                         
                        change: [-22.496% -21.943% -21.450%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe
```

Co-authored-by: saik0 <github@saik0.net>

benchmarks/benches/lib.rs

@@ -245,17 +245,64 @@ fn insert_range_bitmap(c: &mut Criterion) {
 }
 
 fn iter(c: &mut Criterion) {
-    c.bench_function("iter", |b| {
+    c.bench_function("iter bitmap 1..10_000", |b| {
         let bitmap: RoaringBitmap = (1..10_000).collect();
+        b.iter(|| {
+            bitmap.iter().for_each(|i| {
+                black_box(i);
+            });
+        });
+    });
+
+    c.bench_function("iter bitmap sparse", |b| {
+        let bitmap: RoaringBitmap = (0..1 << 16).step_by(61).collect();
+        b.iter(|| {
+            bitmap.iter().for_each(|i| {
+                black_box(i);
+            });
+        });
+    });
 
+    c.bench_function("iter bitmap dense", |b| {
+        let bitmap: RoaringBitmap = (0..1 << 16).step_by(2).collect();
         b.iter(|| {
-            let mut sum: u32 = 0;
+            bitmap.iter().for_each(|i| {
+                black_box(i);
+            });
+        });
+    });
 
-            for (_, element) in bitmap.iter().enumerate() {
-                sum += element;
-            }
+    c.bench_function("iter bitmap minimal", |b| {
+        let bitmap: RoaringBitmap = (0..4096).collect();
+        b.iter(|| {
+            bitmap.iter().for_each(|i| {
+                black_box(i);
+            });
+        });
+    });
+
+    c.bench_function("iter bitmap full", |b| {
+        let bitmap: RoaringBitmap = (0..1 << 16).collect();
+        b.iter(|| {
+            bitmap.iter().for_each(|i| {
+                black_box(i);
+            });
+        });
+    });
+
+    c.bench_function("iter parsed", |b| {
+        let files = self::datasets_paths::WIKILEAKS_NOQUOTES_SRT;
+        let parsed_numbers = parse_dir_files(files).unwrap();
 
-            assert_eq!(sum, 49_995_000);
+        let bitmaps: Vec<_> = parsed_numbers
+            .into_iter()
+            .map(|(_, r)| r.map(|iter| RoaringBitmap::from_sorted_iter(iter).unwrap()).unwrap())
+            .collect();
+
+        b.iter(|| {
+            bitmaps.iter().flat_map(|bitmap| bitmap.iter()).for_each(|i| {
+                black_box(i);
+            });
         });
     });
 }

benchmarks/build.rs

@@ -49,6 +49,7 @@ fn main() -> anyhow::Result<()> {
         "// This file is generated by the build script.\n// Do not modify by hand, use the build.rs file.\n"
     )?;
 
+    #[allow(clippy::single_element_loop)]
     for dataset in &[DATASET_WIKILEAKS_NOQUOTES_SRT] {
         let out_path = out_dir.join(dataset);
         let url = format!("{}/{}.zip", BASE_URL, dataset);

src/bitmap/store.rs

@@ -24,7 +24,7 @@ pub enum Iter<'a> {
 
 pub struct BitmapIter<B: Borrow<[u64; BITMAP_LENGTH]>> {
     key: usize,
-    bit: usize,
+    value: u64,
     bits: B,
 }
 
@@ -699,15 +699,7 @@ impl Clone for Store {
 
 impl<B: Borrow<[u64; BITMAP_LENGTH]>> BitmapIter<B> {
     fn new(bits: B) -> BitmapIter<B> {
-        BitmapIter { key: 0, bit: 0, bits }
-    }
-
-    fn move_next(&mut self) {
-        self.bit += 1;
-        if self.bit == 64 {
-            self.bit = 0;
-            self.key += 1;
-        }
+        BitmapIter { key: 0, value: bits.borrow()[0], bits }
     }
 }
 
@@ -716,17 +708,17 @@ impl<B: Borrow<[u64; BITMAP_LENGTH]>> Iterator for BitmapIter<B> {
 
     fn next(&mut self) -> Option<u16> {
         loop {
-            if self.key == BITMAP_LENGTH {
-                return None;
-            } else if (unsafe { self.bits.borrow().get_unchecked(self.key) } & (1u64 << self.bit))
-                != 0
-            {
-                let result = Some((self.key * 64 + self.bit) as u16);
-                self.move_next();
-                return result;
-            } else {
-                self.move_next();
+            if self.value == 0 {
+                self.key += 1;
+                if self.key >= BITMAP_LENGTH {
+                    return None;
+                }
+                self.value = unsafe { *self.bits.borrow().get_unchecked(self.key) };
+                continue;
             }
+            let lsb = self.value.trailing_zeros() as usize;
+            self.value &= u64::MAX << 1 << lsb;
+            return Some((64 * self.key + lsb) as u16);
         }
     }
 

tests/iter.rs

@@ -1,8 +1,11 @@
 extern crate roaring;
-use roaring::RoaringBitmap;
 
 use std::iter::FromIterator;
 
+use quickcheck_macros::quickcheck;
+
+use roaring::RoaringBitmap;
+
 #[test]
 fn array() {
     let original = (0..2000).collect::<RoaringBitmap>();
@@ -48,3 +51,16 @@ fn bitmaps() {
     assert_eq!(clone, original);
     assert_eq!(clone2, original);
 }
+
+#[quickcheck]
+fn qc_iter(values: Vec<u32>) {
+    let expected = {
+        let mut vec = values.clone();
+        vec.sort();
+        vec.dedup();
+        vec
+    };
+
+    let b = RoaringBitmap::from_iter(values.into_iter());
+    assert_eq!(b.iter().collect::<Vec<u32>>(), expected);
+}