From c089006409537087d57e60fc4e24b9f8449b841c Mon Sep 17 00:00:00 2001 From: saik0 Date: Sun, 6 Feb 2022 17:47:56 -0800 Subject: [PATCH 1/3] make len constant time --- src/bitmap/arbitrary.rs | 3 +- src/bitmap/inherent.rs | 32 +++++++--- src/bitmap/iter.rs | 16 ++--- src/bitmap/mod.rs | 1 + src/bitmap/ops.rs | 116 +++++++++++++++++++++++++++-------- src/bitmap/serialization.rs | 5 +- src/treemap/inherent.rs | 24 +++++--- src/treemap/iter.rs | 4 +- src/treemap/mod.rs | 1 + src/treemap/ops.rs | 16 +++++ src/treemap/serialization.rs | 9 ++- 11 files changed, 172 insertions(+), 55 deletions(-) diff --git a/src/bitmap/arbitrary.rs b/src/bitmap/arbitrary.rs index a1fabdab..07ecd4bc 100644 --- a/src/bitmap/arbitrary.rs +++ b/src/bitmap/arbitrary.rs @@ -178,7 +178,8 @@ mod test { container.ensure_correct_store(); container }).collect::>(); - RoaringBitmap { containers } + let len = containers.iter().map(|c| c.len()).sum(); + RoaringBitmap { len, containers } } } diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index aad59318..4653fa09 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -15,7 +15,7 @@ impl RoaringBitmap { /// let mut rb = RoaringBitmap::new(); /// ``` pub fn new() -> RoaringBitmap { - RoaringBitmap { containers: Vec::new() } + RoaringBitmap { len: 0, containers: Vec::new() } } /// Adds a value to the set. @@ -41,7 +41,9 @@ impl RoaringBitmap { &mut self.containers[loc] } }; - container.insert(index) + let inserted = container.insert(index); + self.len += inserted as u64; + inserted } /// Search for the specific container by the given key. @@ -90,7 +92,9 @@ impl RoaringBitmap { // If the end range value is in the same container, just call into // the one container. if start_container_key == end_container_key { - return self.containers[first_index].insert_range(start_index..=end_index); + let inserted = self.containers[first_index].insert_range(start_index..=end_index); + self.len += inserted; + return inserted; } // For the first container, insert start_index..=u16::MAX, with @@ -115,7 +119,7 @@ impl RoaringBitmap { let last_index = self.find_container_by_key(end_container_key); inserted += self.containers[last_index].insert_range(0..=end_index); - + self.len += inserted; inserted } @@ -139,7 +143,7 @@ impl RoaringBitmap { pub fn push(&mut self, value: u32) -> bool { let (key, index) = util::split(value); - match self.containers.last_mut() { + let pushed = match self.containers.last_mut() { Some(container) if container.key == key => container.push(index), Some(container) if container.key > key => false, _otherwise => { @@ -148,7 +152,9 @@ impl RoaringBitmap { self.containers.push(container); true } - } + }; + self.len += pushed as u64; + pushed } /// @@ -172,6 +178,7 @@ impl RoaringBitmap { self.containers.push(container); } } + self.len += 1; } /// Removes a value from the set. Returns `true` if the value was present in the set. @@ -189,7 +196,7 @@ impl RoaringBitmap { /// ``` pub fn remove(&mut self, value: u32) -> bool { let (key, index) = util::split(value); - match self.containers.binary_search_by_key(&key, |c| c.key) { + let removed = match self.containers.binary_search_by_key(&key, |c| c.key) { Ok(loc) => { if self.containers[loc].remove(index) { if self.containers[loc].len() == 0 { @@ -201,7 +208,9 @@ impl RoaringBitmap { } } _ => false, - } + }; + self.len -= removed as u64; + removed } /// Removes a range of values. @@ -244,6 +253,7 @@ impl RoaringBitmap { } index += 1; } + self.len -= removed; removed } @@ -282,6 +292,7 @@ impl RoaringBitmap { /// assert_eq!(rb.contains(1), false); /// ``` pub fn clear(&mut self) { + self.len = 0; self.containers.clear(); } @@ -320,7 +331,7 @@ impl RoaringBitmap { /// assert_eq!(rb.len(), 2); /// ``` pub fn len(&self) -> u64 { - self.containers.iter().map(|container| container.len()).sum() + self.len } /// Returns the minimum value in the set (if the set is non-empty). @@ -435,10 +446,11 @@ impl Default for RoaringBitmap { impl Clone for RoaringBitmap { fn clone(&self) -> Self { - RoaringBitmap { containers: self.containers.clone() } + RoaringBitmap { len: self.len, containers: self.containers.clone() } } fn clone_from(&mut self, other: &Self) { + self.len = other.len; self.containers.clone_from(&other.containers); } } diff --git a/src/bitmap/iter.rs b/src/bitmap/iter.rs index 75d1fa58..44f21cbd 100644 --- a/src/bitmap/iter.rs +++ b/src/bitmap/iter.rs @@ -17,16 +17,16 @@ pub struct IntoIter { } impl Iter<'_> { - fn new(containers: &[Container]) -> Iter { - let size_hint = containers.iter().map(|c| c.len()).sum(); - Iter { inner: containers.iter().flatten(), size_hint } + fn new(bitmap: &RoaringBitmap) -> Iter { + let size_hint = bitmap.len(); + Iter { inner: bitmap.containers.iter().flatten(), size_hint } } } impl IntoIter { - fn new(containers: Vec) -> IntoIter { - let size_hint = containers.iter().map(|c| c.len()).sum(); - IntoIter { inner: containers.into_iter().flatten(), size_hint } + fn new(bitmap: RoaringBitmap) -> IntoIter { + let size_hint = bitmap.len(); + IntoIter { inner: bitmap.containers.into_iter().flatten(), size_hint } } } @@ -95,7 +95,7 @@ impl RoaringBitmap { /// assert_eq!(iter.next(), None); /// ``` pub fn iter(&self) -> Iter { - Iter::new(&self.containers) + Iter::new(self) } } @@ -113,7 +113,7 @@ impl IntoIterator for RoaringBitmap { type IntoIter = IntoIter; fn into_iter(self) -> IntoIter { - IntoIter::new(self.containers) + IntoIter::new(self) } } diff --git a/src/bitmap/mod.rs b/src/bitmap/mod.rs index f99f73eb..db4b7240 100644 --- a/src/bitmap/mod.rs +++ b/src/bitmap/mod.rs @@ -36,4 +36,5 @@ pub use self::iter::Iter; #[derive(PartialEq)] pub struct RoaringBitmap { containers: Vec, + len: u64, } diff --git a/src/bitmap/ops.rs b/src/bitmap/ops.rs index 54b5a951..4b4d753c 100644 --- a/src/bitmap/ops.rs +++ b/src/bitmap/ops.rs @@ -1,3 +1,5 @@ +#![allow(clippy::suspicious_op_assign_impl)] // allow for +/- len in op assign + use std::mem; use std::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Sub, SubAssign}; @@ -193,16 +195,27 @@ impl BitOr<&RoaringBitmap> for &RoaringBitmap { fn bitor(self, rhs: &RoaringBitmap) -> RoaringBitmap { let mut containers = Vec::new(); + let mut len = 0; for pair in Pairs::new(&self.containers, &rhs.containers) { match pair { - (Some(lhs), None) => containers.push(lhs.clone()), - (None, Some(rhs)) => containers.push(rhs.clone()), - (Some(lhs), Some(rhs)) => containers.push(BitOr::bitor(lhs, rhs)), + (Some(lhs), None) => { + len += lhs.len(); + containers.push(lhs.clone()); + } + (None, Some(rhs)) => { + len += rhs.len(); + containers.push(rhs.clone()); + } + (Some(lhs), Some(rhs)) => { + let container = BitOr::bitor(lhs, rhs); + len += container.len(); + containers.push(container); + } (None, None) => break, } } - RoaringBitmap { containers } + RoaringBitmap { len, containers } } } @@ -214,11 +227,19 @@ impl BitOrAssign for RoaringBitmap { mem::swap(self, &mut rhs); } - for container in rhs.containers { - let key = container.key; + for rhs_cont in rhs.containers { + let key = rhs_cont.key; match self.containers.binary_search_by_key(&key, |c| c.key) { - Err(loc) => self.containers.insert(loc, container), - Ok(loc) => BitOrAssign::bitor_assign(&mut self.containers[loc], container), + Err(loc) => { + self.len += rhs_cont.len(); + self.containers.insert(loc, rhs_cont); + } + Ok(loc) => { + let lhs_cont = &mut self.containers[loc]; + self.len -= lhs_cont.len(); + BitOrAssign::bitor_assign(lhs_cont, rhs_cont); + self.len += lhs_cont.len(); + } } } } @@ -227,11 +248,19 @@ impl BitOrAssign for RoaringBitmap { impl BitOrAssign<&RoaringBitmap> for RoaringBitmap { /// An `union` between two sets. fn bitor_assign(&mut self, rhs: &RoaringBitmap) { - for container in &rhs.containers { - let key = container.key; + for rhs_cont in &rhs.containers { + let key = rhs_cont.key; match self.containers.binary_search_by_key(&key, |c| c.key) { - Err(loc) => self.containers.insert(loc, container.clone()), - Ok(loc) => BitOrAssign::bitor_assign(&mut self.containers[loc], container), + Err(loc) => { + self.len += rhs_cont.len(); + self.containers.insert(loc, rhs_cont.clone()); + } + Ok(loc) => { + let lhs_cont = &mut self.containers[loc]; + self.len -= lhs_cont.len(); + BitOrAssign::bitor_assign(lhs_cont, rhs_cont); + self.len += lhs_cont.len(); + } } } } @@ -273,16 +302,18 @@ impl BitAnd<&RoaringBitmap> for &RoaringBitmap { fn bitand(self, rhs: &RoaringBitmap) -> RoaringBitmap { let mut containers = Vec::new(); + let mut len = 0; for pair in Pairs::new(&self.containers, &rhs.containers) { if let (Some(lhs), Some(rhs)) = pair { let container = BitAnd::bitand(lhs, rhs); if container.len() != 0 { + len += container.len(); containers.push(container); } } } - RoaringBitmap { containers } + RoaringBitmap { len, containers } } } @@ -294,34 +325,42 @@ impl BitAndAssign for RoaringBitmap { mem::swap(self, &mut rhs); } + let mut len = self.len; RetainMut::retain_mut(&mut self.containers, |cont| { let key = cont.key; + len -= cont.len(); match rhs.containers.binary_search_by_key(&key, |c| c.key) { Ok(loc) => { let rhs_cont = &mut rhs.containers[loc]; let rhs_cont = mem::replace(rhs_cont, Container::new(rhs_cont.key)); BitAndAssign::bitand_assign(cont, rhs_cont); + len += cont.len(); cont.len() != 0 } Err(_) => false, } - }) + }); + self.len = len; } } impl BitAndAssign<&RoaringBitmap> for RoaringBitmap { /// An `intersection` between two sets. fn bitand_assign(&mut self, rhs: &RoaringBitmap) { + let mut len = self.len; RetainMut::retain_mut(&mut self.containers, |cont| { let key = cont.key; + len -= cont.len(); match rhs.containers.binary_search_by_key(&key, |c| c.key) { Ok(loc) => { BitAndAssign::bitand_assign(cont, &rhs.containers[loc]); + len += cont.len(); cont.len() != 0 } Err(_) => false, } - }) + }); + self.len = len; } } @@ -361,13 +400,18 @@ impl Sub<&RoaringBitmap> for &RoaringBitmap { fn sub(self, rhs: &RoaringBitmap) -> RoaringBitmap { let mut containers = Vec::new(); + let mut len = 0; for pair in Pairs::new(&self.containers, &rhs.containers) { match pair { - (Some(lhs), None) => containers.push(lhs.clone()), + (Some(lhs), None) => { + len += lhs.len(); + containers.push(lhs.clone()) + } (None, Some(_)) => (), (Some(lhs), Some(rhs)) => { let container = Sub::sub(lhs, rhs); if container.len() != 0 { + len += container.len(); containers.push(container); } } @@ -375,7 +419,7 @@ impl Sub<&RoaringBitmap> for &RoaringBitmap { } } - RoaringBitmap { containers } + RoaringBitmap { len, containers } } } @@ -389,15 +433,19 @@ impl SubAssign for RoaringBitmap { impl SubAssign<&RoaringBitmap> for RoaringBitmap { /// A `difference` between two sets. fn sub_assign(&mut self, rhs: &RoaringBitmap) { + let mut len = self.len; RetainMut::retain_mut(&mut self.containers, |cont| { match rhs.containers.binary_search_by_key(&cont.key, |c| c.key) { Ok(loc) => { + len -= cont.len(); SubAssign::sub_assign(cont, &rhs.containers[loc]); + len += cont.len(); cont.len() != 0 } Err(_) => true, } - }) + }); + self.len = len; } } @@ -437,13 +485,21 @@ impl BitXor<&RoaringBitmap> for &RoaringBitmap { fn bitxor(self, rhs: &RoaringBitmap) -> RoaringBitmap { let mut containers = Vec::new(); + let mut len = 0; for pair in Pairs::new(&self.containers, &rhs.containers) { match pair { - (Some(lhs), None) => containers.push(lhs.clone()), - (None, Some(rhs)) => containers.push(rhs.clone()), + (Some(lhs), None) => { + containers.push(lhs.clone()); + len += lhs.len(); + } + (None, Some(rhs)) => { + containers.push(rhs.clone()); + len += rhs.len(); + } (Some(lhs), Some(rhs)) => { let container = BitXor::bitxor(lhs, rhs); if container.len() != 0 { + len += container.len(); containers.push(container); } } @@ -451,7 +507,7 @@ impl BitXor<&RoaringBitmap> for &RoaringBitmap { } } - RoaringBitmap { containers } + RoaringBitmap { len, containers } } } @@ -461,13 +517,20 @@ impl BitXorAssign for RoaringBitmap { for pair in Pairs::new(mem::take(&mut self.containers), rhs.containers) { match pair { (Some(mut lhs), Some(rhs)) => { + self.len -= lhs.len(); BitXorAssign::bitxor_assign(&mut lhs, rhs); + self.len += lhs.len(); if lhs.len() != 0 { self.containers.push(lhs); } } - (Some(lhs), None) => self.containers.push(lhs), - (None, Some(rhs)) => self.containers.push(rhs), + (Some(lhs), None) => { + self.containers.push(lhs); + } + (None, Some(rhs)) => { + self.len += rhs.len(); + self.containers.push(rhs); + } (None, None) => break, } } @@ -480,13 +543,18 @@ impl BitXorAssign<&RoaringBitmap> for RoaringBitmap { for pair in Pairs::new(mem::take(&mut self.containers), &rhs.containers) { match pair { (Some(mut lhs), Some(rhs)) => { + self.len -= lhs.len(); BitXorAssign::bitxor_assign(&mut lhs, rhs); + self.len += lhs.len(); if lhs.len() != 0 { self.containers.push(lhs); } } (Some(lhs), None) => self.containers.push(lhs), - (None, Some(rhs)) => self.containers.push(rhs.clone()), + (None, Some(rhs)) => { + self.len += rhs.len(); + self.containers.push(rhs.clone()) + } (None, None) => break, } } diff --git a/src/bitmap/serialization.rs b/src/bitmap/serialization.rs index 93cd7c5c..27331c92 100644 --- a/src/bitmap/serialization.rs +++ b/src/bitmap/serialization.rs @@ -144,12 +144,15 @@ impl RoaringBitmap { drop(offsets); // Not useful when deserializing into memory } + let mut bitmap_len = 0; let mut containers = Vec::with_capacity(size); for _ in 0..size { let key = description_bytes.read_u16::()?; let len = u64::from(description_bytes.read_u16::()?) + 1; + bitmap_len += len; + let store = if len <= 4096 { let mut values = vec![0; len as usize]; reader.read_exact(cast_slice_mut(&mut values))?; @@ -169,6 +172,6 @@ impl RoaringBitmap { containers.push(Container { key, store }); } - Ok(RoaringBitmap { containers }) + Ok(RoaringBitmap { len: bitmap_len, containers }) } } diff --git a/src/treemap/inherent.rs b/src/treemap/inherent.rs index 72f1d56d..45d8828f 100644 --- a/src/treemap/inherent.rs +++ b/src/treemap/inherent.rs @@ -16,7 +16,7 @@ impl RoaringTreemap { /// let mut rb = RoaringTreemap::new(); /// ``` pub fn new() -> RoaringTreemap { - RoaringTreemap { map: BTreeMap::new() } + RoaringTreemap { len: 0, map: BTreeMap::new() } } /// Adds a value to the set. Returns `true` if the value was not already present in the set. @@ -33,7 +33,9 @@ impl RoaringTreemap { /// ``` pub fn insert(&mut self, value: u64) -> bool { let (hi, lo) = util::split(value); - self.map.entry(hi).or_insert_with(RoaringBitmap::new).insert(lo) + let inserted = self.map.entry(hi).or_insert_with(RoaringBitmap::new).insert(lo); + self.len += inserted as u64; + inserted } /// Pushes `value` in the treemap only if it is greater than the current maximum value. @@ -55,7 +57,9 @@ impl RoaringTreemap { /// ``` pub fn push(&mut self, value: u64) -> bool { let (hi, lo) = util::split(value); - self.map.entry(hi).or_insert_with(RoaringBitmap::new).push(lo) + let pushed = self.map.entry(hi).or_insert_with(RoaringBitmap::new).push(lo); + self.len += pushed as u64; + pushed } /// @@ -80,6 +84,7 @@ impl RoaringTreemap { self.map.insert(hi, rb); } } + self.len += 1; } /// Removes a value from the set. Returns `true` if the value was present in the set. @@ -97,7 +102,7 @@ impl RoaringTreemap { /// ``` pub fn remove(&mut self, value: u64) -> bool { let (hi, lo) = util::split(value); - match self.map.entry(hi) { + let removed = match self.map.entry(hi) { Entry::Vacant(_) => false, Entry::Occupied(mut ent) => { if ent.get_mut().remove(lo) { @@ -109,7 +114,9 @@ impl RoaringTreemap { false } } - } + }; + self.len -= removed as u64; + removed } /// Removes a range of values. @@ -155,6 +162,7 @@ impl RoaringTreemap { self.map.remove(&key); } + self.len -= removed; removed } @@ -193,6 +201,7 @@ impl RoaringTreemap { /// assert_eq!(rb.contains(1), false); /// ``` pub fn clear(&mut self) { + self.len = 0; self.map.clear(); } @@ -231,7 +240,7 @@ impl RoaringTreemap { /// assert_eq!(rb.len(), 2); /// ``` pub fn len(&self) -> u64 { - self.map.values().map(RoaringBitmap::len).sum() + self.len } /// Returns the minimum value in the set (if the set is non-empty). @@ -344,10 +353,11 @@ impl Default for RoaringTreemap { impl Clone for RoaringTreemap { fn clone(&self) -> Self { - RoaringTreemap { map: self.map.clone() } + RoaringTreemap { len: self.len, map: self.map.clone() } } fn clone_from(&mut self, other: &Self) { + self.len = other.len; self.map.clone_from(&other.map); } } diff --git a/src/treemap/iter.rs b/src/treemap/iter.rs index 3cf59c85..a174c4e0 100644 --- a/src/treemap/iter.rs +++ b/src/treemap/iter.rs @@ -182,7 +182,9 @@ impl RoaringTreemap { /// assert_eq!(clone, original); /// ``` pub fn from_bitmaps>(iterator: I) -> Self { - RoaringTreemap { map: iterator.into_iter().collect() } + let mut len = 0; + let map = iterator.into_iter().inspect(|(_, rb)| len += rb.len()).collect(); + RoaringTreemap { len, map } } } diff --git a/src/treemap/mod.rs b/src/treemap/mod.rs index 3230ee28..317dffc1 100644 --- a/src/treemap/mod.rs +++ b/src/treemap/mod.rs @@ -34,4 +34,5 @@ pub use self::iter::{IntoIter, Iter}; #[derive(PartialEq)] pub struct RoaringTreemap { map: BTreeMap, + len: u64, } diff --git a/src/treemap/ops.rs b/src/treemap/ops.rs index 78cd3573..3de91a96 100644 --- a/src/treemap/ops.rs +++ b/src/treemap/ops.rs @@ -207,10 +207,13 @@ impl BitOrAssign for RoaringTreemap { for (key, other_rb) in rhs.map { match self.map.entry(key) { Entry::Vacant(ent) => { + self.len += other_rb.len(); ent.insert(other_rb); } Entry::Occupied(mut ent) => { + self.len -= ent.get().len(); BitOrAssign::bitor_assign(ent.get_mut(), other_rb); + self.len += ent.get().len(); } } } @@ -223,10 +226,13 @@ impl BitOrAssign<&RoaringTreemap> for RoaringTreemap { for (key, other_rb) in &rhs.map { match self.map.entry(*key) { Entry::Vacant(ent) => { + self.len += other_rb.len(); ent.insert(other_rb.clone()); } Entry::Occupied(mut ent) => { + self.len -= ent.get().len(); BitOrAssign::bitor_assign(ent.get_mut(), other_rb); + self.len += ent.get().len(); } } } @@ -292,9 +298,11 @@ impl BitAndAssign<&RoaringTreemap> for RoaringTreemap { fn bitand_assign(&mut self, rhs: &RoaringTreemap) { let mut keys_to_remove: Vec = Vec::new(); for (key, self_rb) in &mut self.map { + self.len -= self_rb.len(); match rhs.map.get(key) { Some(other_rb) => { BitAndAssign::bitand_assign(self_rb, other_rb); + self.len += self_rb.len(); if self_rb.is_empty() { keys_to_remove.push(*key); } @@ -361,7 +369,9 @@ impl SubAssign<&RoaringTreemap> for RoaringTreemap { match self.map.entry(*key) { Entry::Vacant(_entry) => (), Entry::Occupied(mut entry) => { + self.len -= entry.get().len(); SubAssign::sub_assign(entry.get_mut(), rhs_rb); + self.len += entry.get().len(); if entry.get().is_empty() { entry.remove_entry(); } @@ -419,10 +429,13 @@ impl BitXorAssign for RoaringTreemap { for (key, other_rb) in rhs.map { match self.map.entry(key) { Entry::Vacant(entry) => { + self.len += other_rb.len(); entry.insert(other_rb); } Entry::Occupied(mut entry) => { + self.len -= entry.get().len(); BitXorAssign::bitxor_assign(entry.get_mut(), other_rb); + self.len += entry.get().len(); if entry.get().is_empty() { entry.remove_entry(); } @@ -438,10 +451,13 @@ impl BitXorAssign<&RoaringTreemap> for RoaringTreemap { for (key, other_rb) in &rhs.map { match self.map.entry(*key) { Entry::Vacant(entry) => { + self.len += other_rb.len(); entry.insert(other_rb.clone()); } Entry::Occupied(mut entry) => { + self.len -= entry.get().len(); BitXorAssign::bitxor_assign(entry.get_mut(), other_rb); + self.len += entry.get().len(); if entry.get().is_empty() { entry.remove_entry(); } diff --git a/src/treemap/serialization.rs b/src/treemap/serialization.rs index 7e12ae88..195cbfe2 100644 --- a/src/treemap/serialization.rs +++ b/src/treemap/serialization.rs @@ -1,6 +1,7 @@ use super::RoaringTreemap; use crate::RoaringBitmap; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; +use std::collections::BTreeMap; use std::{io, mem::size_of}; impl RoaringTreemap { @@ -71,15 +72,17 @@ impl RoaringTreemap { pub fn deserialize_from(mut reader: R) -> io::Result { let size = reader.read_u64::()?; - let mut s = Self::new(); + let mut len = 0; + let mut map = BTreeMap::new(); for _ in 0..size { let key = reader.read_u32::()?; let bitmap = RoaringBitmap::deserialize_from(&mut reader)?; + len += bitmap.len(); - s.map.insert(key, bitmap); + map.insert(key, bitmap); } - Ok(s) + Ok(RoaringTreemap { len, map }) } } From 5509a9778721a92d0319eea37bb63aa533118320 Mon Sep 17 00:00:00 2001 From: saik0 Date: Mon, 7 Feb 2022 01:05:27 -0800 Subject: [PATCH 2/3] move clippy allow to module level --- src/bitmap/mod.rs | 3 +++ src/bitmap/ops.rs | 2 -- src/bitmap/store/array_store/mod.rs | 4 ---- src/bitmap/store/bitmap_store.rs | 2 -- src/bitmap/store/mod.rs | 2 -- 5 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/bitmap/mod.rs b/src/bitmap/mod.rs index db4b7240..451a1477 100644 --- a/src/bitmap/mod.rs +++ b/src/bitmap/mod.rs @@ -1,3 +1,6 @@ +#![allow(clippy::suspicious_op_assign_impl)] // allow for +/- len in op assign +#![allow(clippy::suspicious_arithmetic_impl)] + mod arbitrary; mod container; mod fmt; diff --git a/src/bitmap/ops.rs b/src/bitmap/ops.rs index 4b4d753c..f0a94450 100644 --- a/src/bitmap/ops.rs +++ b/src/bitmap/ops.rs @@ -1,5 +1,3 @@ -#![allow(clippy::suspicious_op_assign_impl)] // allow for +/- len in op assign - use std::mem; use std::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Sub, SubAssign}; diff --git a/src/bitmap/store/array_store/mod.rs b/src/bitmap/store/array_store/mod.rs index 64d42add..665824b2 100644 --- a/src/bitmap/store/array_store/mod.rs +++ b/src/bitmap/store/array_store/mod.rs @@ -255,7 +255,6 @@ impl BitOr for &ArrayStore { type Output = ArrayStore; fn bitor(self, rhs: Self) -> Self::Output { - #[allow(clippy::suspicious_arithmetic_impl)] let capacity = self.vec.len() + rhs.vec.len(); let mut visitor = VecWriter::new(capacity); #[cfg(feature = "simd")] @@ -280,7 +279,6 @@ impl BitAnd for &ArrayStore { } impl BitAndAssign<&Self> for ArrayStore { - #[allow(clippy::suspicious_op_assign_impl)] fn bitand_assign(&mut self, rhs: &Self) { #[cfg(feature = "simd")] { @@ -319,7 +317,6 @@ impl Sub for &ArrayStore { } impl SubAssign<&Self> for ArrayStore { - #[allow(clippy::suspicious_op_assign_impl)] fn sub_assign(&mut self, rhs: &Self) { #[cfg(feature = "simd")] { @@ -348,7 +345,6 @@ impl BitXor for &ArrayStore { type Output = ArrayStore; fn bitxor(self, rhs: Self) -> Self::Output { - #[allow(clippy::suspicious_arithmetic_impl)] let capacity = self.vec.len() + rhs.vec.len(); let mut visitor = VecWriter::new(capacity); #[cfg(feature = "simd")] diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 240cc798..74aef6c4 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -379,14 +379,12 @@ impl BitAndAssign<&Self> for BitmapStore { } impl SubAssign<&Self> for BitmapStore { - #[allow(clippy::suspicious_op_assign_impl)] fn sub_assign(&mut self, rhs: &Self) { op_bitmaps(self, rhs, |l, r| *l &= !r); } } impl SubAssign<&ArrayStore> for BitmapStore { - #[allow(clippy::suspicious_op_assign_impl)] fn sub_assign(&mut self, rhs: &ArrayStore) { for &index in rhs.iter() { let (key, bit) = (key(index), bit(index)); diff --git a/src/bitmap/store/mod.rs b/src/bitmap/store/mod.rs index fcefb3da..7e0f5f9c 100644 --- a/src/bitmap/store/mod.rs +++ b/src/bitmap/store/mod.rs @@ -248,7 +248,6 @@ impl BitAnd<&Store> for &Store { } impl BitAndAssign for Store { - #[allow(clippy::suspicious_op_assign_impl)] fn bitand_assign(&mut self, mut rhs: Store) { match (self, &mut rhs) { (&mut Array(ref mut vec1), &mut Array(ref mut vec2)) => { @@ -272,7 +271,6 @@ impl BitAndAssign for Store { } impl BitAndAssign<&Store> for Store { - #[allow(clippy::suspicious_op_assign_impl)] fn bitand_assign(&mut self, rhs: &Store) { match (self, rhs) { (&mut Array(ref mut vec1), &Array(ref vec2)) => { From 17340b519cb39c874c1b1dd3015986f207e390f0 Mon Sep 17 00:00:00 2001 From: saik0 Date: Mon, 7 Feb 2022 08:38:39 -0800 Subject: [PATCH 3/3] use cached len in select --- src/bitmap/inherent.rs | 8 +++++--- src/treemap/inherent.rs | 8 +++++--- tests/rank.rs | 2 ++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 4653fa09..d6cb743e 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -386,8 +386,6 @@ impl RoaringBitmap { /// assert_eq!(rb.rank(10), 2) /// ``` pub fn rank(&self, value: u32) -> u64 { - // if len becomes cached for RoaringBitmap: return len if len > value - let (key, index) = util::split(value); match self.containers.binary_search_by_key(&key, |c| c.key) { @@ -402,7 +400,7 @@ impl RoaringBitmap { } } - /// Returns the nth integer in the bitmap (if the set is non-empty) + /// Returns the nth integer in the bitmap or `None` if n > len() /// /// # Examples /// @@ -421,6 +419,10 @@ impl RoaringBitmap { /// assert_eq!(rb.select(2), Some(100)); /// ``` pub fn select(&self, n: u32) -> Option { + if n as u64 >= self.len() { + return None; + } + let mut n = n as u64; for container in &self.containers { diff --git a/src/treemap/inherent.rs b/src/treemap/inherent.rs index 45d8828f..75ee8dea 100644 --- a/src/treemap/inherent.rs +++ b/src/treemap/inherent.rs @@ -302,8 +302,6 @@ impl RoaringTreemap { /// assert_eq!(rb.rank(10), 2) /// ``` pub fn rank(&self, value: u64) -> u64 { - // if len becomes cached for RoaringTreemap: return len if len > value - let (hi, lo) = util::split(value); let mut iter = self.map.range(..=hi).rev(); @@ -313,7 +311,7 @@ impl RoaringTreemap { + iter.map(|(_, bitmap)| bitmap.len()).sum::() } - /// Returns the nth integer in the treemap (if the set is non-empty) + /// Returns the nth integer in the treemap or `None` if n > len() /// /// # Examples /// @@ -333,6 +331,10 @@ impl RoaringTreemap { /// assert_eq!(rb.select(3), None); /// ``` pub fn select(&self, mut n: u64) -> Option { + if n as u64 >= self.len() { + return None; + } + for (&key, bitmap) in &self.map { let len = bitmap.len(); if len > n { diff --git a/tests/rank.rs b/tests/rank.rs index 94883779..c1fcf2aa 100644 --- a/tests/rank.rs +++ b/tests/rank.rs @@ -20,6 +20,8 @@ fn rank() { // Bitmap container at key assert_eq!(bitmap.rank(200_000), 2001); + assert_eq!(bitmap.rank(209_998), 11_999); + assert_eq!(bitmap.rank(209_999), 12_000); assert_eq!(bitmap.rank(210_000), 12_000); }