From 4e344a8f144331ef885810b88d016d6f4f21787c Mon Sep 17 00:00:00 2001 From: GZTime Date: Tue, 1 Oct 2024 14:21:15 +0800 Subject: [PATCH 1/6] fix(typos): fix some typos with `typos` --- roaring/src/bitmap/store/array_store/visitor.rs | 2 +- roaring/src/treemap/multiops.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/roaring/src/bitmap/store/array_store/visitor.rs b/roaring/src/bitmap/store/array_store/visitor.rs index 51dda9b9..d7293d7b 100644 --- a/roaring/src/bitmap/store/array_store/visitor.rs +++ b/roaring/src/bitmap/store/array_store/visitor.rs @@ -32,7 +32,7 @@ impl VecWriter { pub fn into_inner(self) -> Vec { // Consider shrinking the vec here. - // Exacty len could be too aggressive. Len rounded up to next power of 2? + // Exactly len could be too aggressive. Len rounded up to next power of 2? // Related, but not exact issue: https://github.com/RoaringBitmap/roaring-rs/issues/136 self.vec } diff --git a/roaring/src/treemap/multiops.rs b/roaring/src/treemap/multiops.rs index 39c6e15f..3a0adba6 100644 --- a/roaring/src/treemap/multiops.rs +++ b/roaring/src/treemap/multiops.rs @@ -91,8 +91,8 @@ where (key, bitmap) } None => { - let poped = PeekMut::pop(peek); - (poped.key, poped.bitmap) + let popped = PeekMut::pop(peek); + (popped.key, popped.bitmap) } }; @@ -209,8 +209,8 @@ where (key, bitmap) } None => { - let poped = PeekMut::pop(peek); - (poped.key, poped.bitmap) + let popped = PeekMut::pop(peek); + (popped.key, popped.bitmap) } }; From 841004b68cf7013a678fff8784a6fd3dd12f096a Mon Sep 17 00:00:00 2001 From: Zachary Dremann Date: Wed, 9 Oct 2024 21:04:22 -0400 Subject: [PATCH 2/6] Fix warnings when testing with nightly Keep testing arbitrary implementations as pub(crate). This avoids warnings about undocumented public APIs --- roaring/src/bitmap/arbitrary.rs | 2 +- roaring/src/treemap/arbitrary.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/roaring/src/bitmap/arbitrary.rs b/roaring/src/bitmap/arbitrary.rs index a8ca25be..6bb8b356 100644 --- a/roaring/src/bitmap/arbitrary.rs +++ b/roaring/src/bitmap/arbitrary.rs @@ -189,7 +189,7 @@ mod test { impl RoaringBitmap { prop_compose! { - pub fn arbitrary()(bitmap in (0usize..=16).prop_flat_map(containers)) -> RoaringBitmap { + pub(crate) fn arbitrary()(bitmap in (0usize..=16).prop_flat_map(containers)) -> RoaringBitmap { bitmap } } diff --git a/roaring/src/treemap/arbitrary.rs b/roaring/src/treemap/arbitrary.rs index c7adb7d5..6c8e096a 100644 --- a/roaring/src/treemap/arbitrary.rs +++ b/roaring/src/treemap/arbitrary.rs @@ -6,7 +6,7 @@ mod test { impl RoaringTreemap { prop_compose! { - pub fn arbitrary()(map in btree_map(0u32..=16, RoaringBitmap::arbitrary(), 0usize..=16)) -> RoaringTreemap { + pub(crate) fn arbitrary()(map in btree_map(0u32..=16, RoaringBitmap::arbitrary(), 0usize..=16)) -> RoaringTreemap { // we’re NEVER supposed to start with a treemap containing empty bitmaps // Since we can’t configure this in arbitrary we’re simply going to ignore the generated empty bitmaps let map = map.into_iter().filter(|(_, v)| !v.is_empty()).collect(); From 55a0b18c71333e162654eeb2a80d24ee863d2c5a Mon Sep 17 00:00:00 2001 From: Zachary Dremann Date: Thu, 10 Oct 2024 20:45:13 -0400 Subject: [PATCH 3/6] Also fix clippy warnings on nightly --- roaring/src/bitmap/container.rs | 2 +- roaring/src/bitmap/store/mod.rs | 2 +- roaring/src/treemap/iter.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/roaring/src/bitmap/container.rs b/roaring/src/bitmap/container.rs index 3ab1e5ad..09860b88 100644 --- a/roaring/src/bitmap/container.rs +++ b/roaring/src/bitmap/container.rs @@ -295,7 +295,7 @@ impl IntoIterator for Container { } } -impl<'a> Iterator for Iter<'a> { +impl Iterator for Iter<'_> { type Item = u32; fn next(&mut self) -> Option { self.inner.next().map(|i| util::join(self.key, i)) diff --git a/roaring/src/bitmap/store/mod.rs b/roaring/src/bitmap/store/mod.rs index bb0d5822..d0661639 100644 --- a/roaring/src/bitmap/store/mod.rs +++ b/roaring/src/bitmap/store/mod.rs @@ -497,7 +497,7 @@ impl PartialEq for Store { } } -impl<'a> Iterator for Iter<'a> { +impl Iterator for Iter<'_> { type Item = u16; fn next(&mut self) -> Option { diff --git a/roaring/src/treemap/iter.rs b/roaring/src/treemap/iter.rs index 96ccb00d..f8094a37 100644 --- a/roaring/src/treemap/iter.rs +++ b/roaring/src/treemap/iter.rs @@ -11,7 +11,7 @@ struct To64Iter<'a> { inner: Iter32<'a>, } -impl<'a> Iterator for To64Iter<'a> { +impl Iterator for To64Iter<'_> { type Item = u64; fn next(&mut self) -> Option { self.inner.next().map(|n| util::join(self.hi, n)) @@ -109,7 +109,7 @@ pub struct IntoIter { size_hint: u64, } -impl<'a> Iter<'a> { +impl Iter<'_> { fn new(map: &BTreeMap) -> Iter { let size_hint: u64 = map.iter().map(|(_, r)| r.len()).sum(); let i = map.iter().flat_map(to64iter as _); @@ -125,7 +125,7 @@ impl IntoIter { } } -impl<'a> Iterator for Iter<'a> { +impl Iterator for Iter<'_> { type Item = u64; fn next(&mut self) -> Option { From c7c3cd5f055b19306d69715b16dd41305035f8cd Mon Sep 17 00:00:00 2001 From: Zachary Dremann Date: Mon, 30 Sep 2024 23:19:59 -0400 Subject: [PATCH 4/6] do not expose a "datasets" benchmark binary the datasets.rs file is only used as a module in the lib benchmarks --- benchmarks/benches/{datasets.rs => datasets/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename benchmarks/benches/{datasets.rs => datasets/mod.rs} (100%) diff --git a/benchmarks/benches/datasets.rs b/benchmarks/benches/datasets/mod.rs similarity index 100% rename from benchmarks/benches/datasets.rs rename to benchmarks/benches/datasets/mod.rs From 385eff73d63d67e8ccc6d08f28a2c00c064fd5a5 Mon Sep 17 00:00:00 2001 From: Zachary Dremann Date: Mon, 30 Sep 2024 21:10:26 -0400 Subject: [PATCH 5/6] Speed up bitmap store iterator --- roaring/src/bitmap/store/bitmap_store.rs | 35 +++++++++++++----------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/roaring/src/bitmap/store/bitmap_store.rs b/roaring/src/bitmap/store/bitmap_store.rs index f349a2aa..a60f3a1a 100644 --- a/roaring/src/bitmap/store/bitmap_store.rs +++ b/roaring/src/bitmap/store/bitmap_store.rs @@ -1,5 +1,4 @@ use core::borrow::Borrow; -use core::cmp::Ordering; use core::fmt::{Display, Formatter}; use core::ops::{BitAndAssign, BitOrAssign, BitXorAssign, RangeInclusive, SubAssign}; @@ -427,24 +426,28 @@ impl> Iterator for BitmapIter { type Item = u16; fn next(&mut self) -> Option { - loop { - if self.value == 0 { - self.key += 1; - let cmp = self.key.cmp(&self.key_back); - // Match arms can be reordered, this ordering is perf sensitive - self.value = if cmp == Ordering::Less { - unsafe { *self.bits.borrow().get_unchecked(self.key) } - } else if cmp == Ordering::Equal { - self.value_back - } else { + if self.value == 0 { + 'get_val: { + if self.key >= self.key_back { return None; - }; - continue; + } + for key in self.key + 1..self.key_back { + self.value = unsafe { *self.bits.borrow().get_unchecked(key) }; + if self.value != 0 { + self.key = key; + break 'get_val; + } + } + self.key = self.key_back; + self.value = self.value_back; + if self.value == 0 { + return None; + } } - let index = self.value.trailing_zeros() as usize; - self.value &= self.value - 1; - return Some((64 * self.key + index) as u16); } + let index = self.value.trailing_zeros() as usize; + self.value &= self.value - 1; + Some((64 * self.key + index) as u16) } } From b39a3c11f5f4db4c84c253f3082338aa62fc7f0d Mon Sep 17 00:00:00 2001 From: Zachary Dremann Date: Mon, 30 Sep 2024 23:38:48 -0400 Subject: [PATCH 6/6] use u16 keys for bitmap store iterator --- roaring/src/bitmap/store/bitmap_store.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/roaring/src/bitmap/store/bitmap_store.rs b/roaring/src/bitmap/store/bitmap_store.rs index a60f3a1a..ceaeb5c2 100644 --- a/roaring/src/bitmap/store/bitmap_store.rs +++ b/roaring/src/bitmap/store/bitmap_store.rs @@ -403,9 +403,9 @@ impl Display for Error { impl std::error::Error for Error {} pub struct BitmapIter> { - key: usize, + key: u16, value: u64, - key_back: usize, + key_back: u16, value_back: u64, bits: B, } @@ -415,7 +415,7 @@ impl> BitmapIter { BitmapIter { key: 0, value: bits.borrow()[0], - key_back: BITMAP_LENGTH - 1, + key_back: BITMAP_LENGTH as u16 - 1, value_back: bits.borrow()[BITMAP_LENGTH - 1], bits, } @@ -432,7 +432,7 @@ impl> Iterator for BitmapIter { return None; } for key in self.key + 1..self.key_back { - self.value = unsafe { *self.bits.borrow().get_unchecked(key) }; + self.value = unsafe { *self.bits.borrow().get_unchecked(key as usize) }; if self.value != 0 { self.key = key; break 'get_val; @@ -445,9 +445,9 @@ impl> Iterator for BitmapIter { } } } - let index = self.value.trailing_zeros() as usize; + let index = self.value.trailing_zeros() as u16; self.value &= self.value - 1; - Some((64 * self.key + index) as u16) + Some(64 * self.key + index) } } @@ -461,13 +461,14 @@ impl> DoubleEndedIterator for BitmapIter { return None; } self.key_back -= 1; - self.value_back = unsafe { *self.bits.borrow().get_unchecked(self.key_back) }; + self.value_back = + unsafe { *self.bits.borrow().get_unchecked(self.key_back as usize) }; continue; } - let index_from_left = value.leading_zeros() as usize; + let index_from_left = value.leading_zeros() as u16; let index = 63 - index_from_left; *value &= !(1 << index); - return Some((64 * self.key_back + index) as u16); + return Some(64 * self.key_back + index); } } }