From dcc5eeb01700336e75ea5ca03f8036ed38365524 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Sun, 7 May 2023 12:40:07 +0900 Subject: [PATCH 01/44] Feat remove first --- src/bitmap/container.rs | 6 +++++ src/bitmap/inherent.rs | 36 +++++++++++++++++++++++++++++ src/bitmap/store/array_store/mod.rs | 13 +++++++++++ src/bitmap/store/bitmap_store.rs | 35 ++++++++++++++++++++++++++++ src/bitmap/store/mod.rs | 7 ++++++ 5 files changed, 97 insertions(+) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index e6d0cf84..2f55fc7b 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -94,6 +94,12 @@ impl Container { result } + pub fn remove_first(&mut self, n: usize) -> bool { + let result = self.store.remove_first(n); + self.ensure_correct_store(); + result + } + pub fn contains(&self, index: u16) -> bool { self.store.contains(index) } diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 60e9e619..ff1c7ba6 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -578,6 +578,42 @@ impl RoaringBitmap { None } + + /// Removes the specified number of elements from the top. + /// + /// # Examples + /// + /// ```rust + /// use roaring::RoaringBitmap; + /// + /// let mut rb = RoaringBitmap::from_iter([1, 65535, 65536, 65589]); + /// rb.remove_first(2); + /// assert_eq!(rb, RoaringBitmap::from_iter([65536, 65589])); + /// rb.remove_first(1); + /// assert_eq!(rb, RoaringBitmap::from_iter([65589])); + pub fn remove_first(&mut self, n: usize) -> bool { + // remove containers up to the front of the target + let loc = self.select(n as u32).unwrap(); + let old_len = self.len(); + let remove_key = loc / (u16::MAX as usize + 1) as u32; + for i in 0..remove_key { + match self.containers.binary_search_by_key(&i, |c| c.key.into()) { + Ok(loc) => { + self.containers.remove(loc); + } + _ => {} + } + } + let removed_len = (old_len - self.len()) as usize; + + // remove data in containers if there are still targets for deletion + if removed_len < n { + // container immediately before should have been deleted, so the target is 0 index + let result = self.containers[0].remove_first(n - removed_len); + return result; + } + false + } } impl Default for RoaringBitmap { diff --git a/src/bitmap/store/array_store/mod.rs b/src/bitmap/store/array_store/mod.rs index ca6ee206..4f08d31b 100644 --- a/src/bitmap/store/array_store/mod.rs +++ b/src/bitmap/store/array_store/mod.rs @@ -106,6 +106,12 @@ impl ArrayStore { (pos_end - pos_start) as u64 } + pub fn remove_first(&mut self, n: usize) -> bool { + self.vec.rotate_left(n.into()); + self.vec.truncate(self.vec.len() - n as usize); + true + } + pub fn contains(&self, index: u16) -> bool { self.vec.binary_search(&index).is_ok() } @@ -558,4 +564,11 @@ mod tests { assert_eq!(into_vec(store), want); } + + #[test] + fn test_bitmap_remove_first() { + let mut store = Store::Array(ArrayStore::from_vec_unchecked(vec![1, 2, 130, 500])); + store.remove_first(3); + assert_eq!(into_vec(store), vec![500]); + } } diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 55a53c57..6212ed36 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -302,6 +302,21 @@ impl BitmapStore { pub fn as_array(&self) -> &[u64; BITMAP_LENGTH] { &self.bits } + + /// Set N bits that are currently 1 bit from the lower bit to 0. + pub fn remove_first(&mut self, mut clear_bits: usize) -> bool { + if self.bits[0].count_ones() > clear_bits as u32 { + let mut mask = 1; + while clear_bits > 0 { + if self.bits[0] & mask == mask { + self.bits[0] &= !mask; + clear_bits -= 1; + } + mask <<= 1; + } + } + true + } } // this can be done in 3 instructions on x86-64 with bmi2 with: tzcnt(pdep(1 << rank, value)) @@ -490,3 +505,23 @@ impl BitXorAssign<&ArrayStore> for BitmapStore { self.len = len as u64; } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_bitmap_remove_first() { + let mut store = BitmapStore::new(); + let range = RangeInclusive::new(1, 3); + store.insert_range(range); + let range_second = RangeInclusive::new(5, 65535); + // store.bits[0] = 0b1111111111111111111111111111111111111111111111111111111111101110 + store.insert_range(range_second); + store.remove_first(2); + assert_eq!( + store.bits[0], + 0b1111111111111111111111111111111111111111111111111111111111101000 + ); + } +} diff --git a/src/bitmap/store/mod.rs b/src/bitmap/store/mod.rs index 0ebc150b..db59a593 100644 --- a/src/bitmap/store/mod.rs +++ b/src/bitmap/store/mod.rs @@ -96,6 +96,13 @@ impl Store { } } + pub fn remove_first(&mut self, index: usize) -> bool { + match self { + Array(vec) => vec.remove_first(index), + Bitmap(bits) => bits.remove_first(index), + } + } + pub fn contains(&self, index: u16) -> bool { match self { Array(vec) => vec.contains(index), From 2009f6e8db08f81d52f5e914c805084f84eab272 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Sun, 7 May 2023 22:12:18 +0900 Subject: [PATCH 02/44] Fix process --- src/bitmap/inherent.rs | 66 +++++++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index ff1c7ba6..49ff86a0 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -586,33 +586,35 @@ impl RoaringBitmap { /// ```rust /// use roaring::RoaringBitmap; /// - /// let mut rb = RoaringBitmap::from_iter([1, 65535, 65536, 65589]); + /// let mut rb = RoaringBitmap::from_iter([1, 5, 7, 9]); /// rb.remove_first(2); - /// assert_eq!(rb, RoaringBitmap::from_iter([65536, 65589])); - /// rb.remove_first(1); - /// assert_eq!(rb, RoaringBitmap::from_iter([65589])); - pub fn remove_first(&mut self, n: usize) -> bool { + /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); + /// rb = RoaringBitmap::from_iter([1, 3, 7, 9]); + /// rb.remove_first(2); + /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); + pub fn remove_first(&mut self, mut n: usize) { // remove containers up to the front of the target - let loc = self.select(n as u32).unwrap(); - let old_len = self.len(); - let remove_key = loc / (u16::MAX as usize + 1) as u32; - for i in 0..remove_key { - match self.containers.binary_search_by_key(&i, |c| c.key.into()) { - Ok(loc) => { - self.containers.remove(loc); - } - _ => {} + let position = self.containers.iter().position(|container| { + let container_len = container.len() as usize; + if container_len < n { + n -= container_len; + false + } else { + true + } + }); + if let Some(position) = position { + if position > 0 { + self.containers.rotate_left(position); + self.containers.truncate(position); } } - let removed_len = (old_len - self.len()) as usize; - // remove data in containers if there are still targets for deletion - if removed_len < n { + if n > 0 { // container immediately before should have been deleted, so the target is 0 index - let result = self.containers[0].remove_first(n - removed_len); - return result; + self.containers[0].remove_first(n); } - false + } } } @@ -764,4 +766,28 @@ mod tests { assert_eq!(bitmap.containers.len(), 1); assert_eq!(bitmap.containers[0].key, 1); } + + #[test] + fn remove_first() { + let mut bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); + bitmap.remove_first(3); + assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); + + bitmap = RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11]); + bitmap.remove_first(3); + assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); + + let mut rb = RoaringBitmap::from_iter([1, 3]); + rb.remove_first(1); + println!("{:?}", rb); + assert_eq!(rb.len(), 1); + } + + #[test] + fn remove_first_for_bit() { + let mut bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..(1_u32 << 16) + 3); + bitmap.remove_first(65537); + println!("{:?}", bitmap); + } } From 389da10f12652416bb11af8a7cd2c373d54c012e Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 8 May 2023 01:16:29 +0900 Subject: [PATCH 03/44] Fix remove first function --- src/bitmap/container.rs | 16 +++++++++++++-- src/bitmap/inherent.rs | 32 +++++++++++++++++++++-------- src/bitmap/store/array_store/mod.rs | 7 +++++-- src/bitmap/store/bitmap_store.rs | 2 +- src/bitmap/store/mod.rs | 2 +- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index 2f55fc7b..7e2fc312 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -94,8 +94,20 @@ impl Container { result } - pub fn remove_first(&mut self, n: usize) -> bool { - let result = self.store.remove_first(n); + pub fn remove_first(&mut self, n: usize) { + match &self.store { + Store::Bitmap(bits) => { + if bits.len() - n as u64 <= ARRAY_LIMIT { + let mut replace_array = Vec::with_capacity(bits.len() as usize - n); + replace_array.extend(bits.clone().into_iter().skip(n)); + self.store = Store::Array(store::ArrayStore::from_vec_unchecked(replace_array)); + } else { + self.store.remove_first(n) + } + } + Store::Array(_) => self.store.remove_first(n), + }; + } self.ensure_correct_store(); result } diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 49ff86a0..1c961b48 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -768,26 +768,42 @@ mod tests { } #[test] - fn remove_first() { + fn remove_first_for_vec() { let mut bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); bitmap.remove_first(3); + assert_eq!(bitmap.len(), 3); assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); bitmap = RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11]); bitmap.remove_first(3); + assert_eq!(bitmap.len(), 3); assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); - let mut rb = RoaringBitmap::from_iter([1, 3]); - rb.remove_first(1); - println!("{:?}", rb); - assert_eq!(rb.len(), 1); + bitmap = RoaringBitmap::from_iter([1, 3]); + bitmap.remove_first(1); + assert_eq!(bitmap.len(), 1); + assert_eq!(bitmap, RoaringBitmap::from_iter([3])); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..(1_u32 << 16) + 5); + bitmap.remove_first(65537); + assert_eq!(bitmap.len(), 4); + assert_eq!(bitmap, RoaringBitmap::from_iter([65537, 65538, 65539, 65540])); } #[test] fn remove_first_for_bit() { let mut bitmap = RoaringBitmap::new(); - bitmap.insert_range(0..(1_u32 << 16) + 3); - bitmap.remove_first(65537); - println!("{:?}", bitmap); + bitmap.insert_range(0..4098); + bitmap.remove_first(4095); + assert_eq!(bitmap.len(), 3); + // removed bit to vec + assert_eq!(bitmap, RoaringBitmap::from_iter([4095, 4096, 4097])); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..(1_u32 << 16)); + bitmap.remove_first(65536); + assert_eq!(bitmap.len(), 0); + } } } diff --git a/src/bitmap/store/array_store/mod.rs b/src/bitmap/store/array_store/mod.rs index 4f08d31b..c6ad7f5b 100644 --- a/src/bitmap/store/array_store/mod.rs +++ b/src/bitmap/store/array_store/mod.rs @@ -106,10 +106,13 @@ impl ArrayStore { (pos_end - pos_start) as u64 } - pub fn remove_first(&mut self, n: usize) -> bool { + pub fn remove_first(&mut self, n: usize) { self.vec.rotate_left(n.into()); self.vec.truncate(self.vec.len() - n as usize); - true + } + + pub fn remove_last(&mut self, n: usize) { + self.vec.truncate(self.vec.len() - n as usize); } pub fn contains(&self, index: u16) -> bool { diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 6212ed36..3ba116b1 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -304,7 +304,7 @@ impl BitmapStore { } /// Set N bits that are currently 1 bit from the lower bit to 0. - pub fn remove_first(&mut self, mut clear_bits: usize) -> bool { + pub fn remove_first(&mut self, mut clear_bits: usize) { if self.bits[0].count_ones() > clear_bits as u32 { let mut mask = 1; while clear_bits > 0 { diff --git a/src/bitmap/store/mod.rs b/src/bitmap/store/mod.rs index db59a593..b094606b 100644 --- a/src/bitmap/store/mod.rs +++ b/src/bitmap/store/mod.rs @@ -96,7 +96,7 @@ impl Store { } } - pub fn remove_first(&mut self, index: usize) -> bool { + pub fn remove_first(&mut self, index: usize) { match self { Array(vec) => vec.remove_first(index), Bitmap(bits) => bits.remove_first(index), From 758bb6de3bc3d504ebd6763ed441f68425e81958 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 8 May 2023 14:12:48 +0900 Subject: [PATCH 04/44] Fix bit switching process of remove_first --- src/bitmap/store/bitmap_store.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 3ba116b1..1cee249f 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -305,14 +305,30 @@ impl BitmapStore { /// Set N bits that are currently 1 bit from the lower bit to 0. pub fn remove_first(&mut self, mut clear_bits: usize) { - if self.bits[0].count_ones() > clear_bits as u32 { + if self.len() < clear_bits as u64 { + return; + } + let mut removed: u64 = 0; + let min = self.min().unwrap(); + let key = key(min); + for index in key..BITMAP_LENGTH { let mut mask = 1; - while clear_bits > 0 { - if self.bits[0] & mask == mask { - self.bits[0] &= !mask; + let mut count = self.bits[index].count_ones(); + while count > 0 { + if self.bits[index] & mask == mask { + self.bits[index] &= !mask; clear_bits -= 1; + removed += 1; + if clear_bits <= 0 { + self.len -= removed; + return; + } } mask <<= 1; + count -= 1; + } + } + } } } true From db3df9e4b9468e0cc2e94c83f90d4c931709b268 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 8 May 2023 14:13:23 +0900 Subject: [PATCH 05/44] Feat remove last --- src/bitmap/container.rs | 17 ++++++++- src/bitmap/inherent.rs | 59 +++++++++++++++++++++++++++-- src/bitmap/store/array_store/mod.rs | 7 ++++ src/bitmap/store/bitmap_store.rs | 26 ++++++++++++- src/bitmap/store/mod.rs | 7 ++++ 5 files changed, 110 insertions(+), 6 deletions(-) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index 7e2fc312..2dec2de7 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -108,8 +108,21 @@ impl Container { Store::Array(_) => self.store.remove_first(n), }; } - self.ensure_correct_store(); - result + + pub fn remove_last(&mut self, n: usize) { + match &self.store { + Store::Bitmap(bits) => { + if bits.len() - n as u64 <= ARRAY_LIMIT { + let mut replace_array = Vec::with_capacity(bits.len() as usize - n); + replace_array.extend(bits.clone().into_iter()); + replace_array.truncate(replace_array.len() - n); + self.store = Store::Array(store::ArrayStore::from_vec_unchecked(replace_array)); + } else { + self.store.remove_last(n) + } + } + Store::Array(_) => self.store.remove_last(n), + }; } pub fn contains(&self, index: u16) -> bool { diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 1c961b48..1bc3bd1f 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -615,6 +615,29 @@ impl RoaringBitmap { self.containers[0].remove_first(n); } } + + /// Removes the specified number of elements from the tail. + /// todo doc test + pub fn remove_last(&mut self, mut n: usize) { + let container_len = self.containers.len(); + let potision = self.containers.iter().enumerate().rev().find_map(|(index, container)| { + let container_len = container.len() as usize; + if container_len >= n { + Some(index) + } else { + n -= container_len; + None + } + }); + if let Some(position) = potision { + let remove_container = container_len - position - 1; + if remove_container > 0 { + self.containers.truncate(remove_container); + } + if n > 0 { + self.containers[position].remove_last(n); + } + } } } @@ -801,9 +824,39 @@ mod tests { assert_eq!(bitmap, RoaringBitmap::from_iter([4095, 4096, 4097])); bitmap = RoaringBitmap::new(); - bitmap.insert_range(0..(1_u32 << 16)); - bitmap.remove_first(65536); - assert_eq!(bitmap.len(), 0); + bitmap.insert_range(0..6000); + bitmap.remove_first(999); + assert_eq!(bitmap.len(), 5001); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..8000); + bitmap.remove_first(10); + assert_eq!(bitmap.len(), 7990); + } + + #[test] + fn remove_last_for_bit() { + let mut bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..5000); + bitmap.remove_last(1000); + assert_eq!(bitmap.len(), 4000); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..6000); + bitmap.remove_last(1000); + assert_eq!(bitmap.len(), 5000); } + + #[test] + fn remove_last_for_vec() { + let mut bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); + bitmap.remove_last(2); + assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7])); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..(1_u32 << 16) + 5); + bitmap.remove_last(65537); + assert_eq!(bitmap.len(), 4); + assert_eq!(bitmap, RoaringBitmap::from_iter([0, 1, 2, 3])); } } diff --git a/src/bitmap/store/array_store/mod.rs b/src/bitmap/store/array_store/mod.rs index c6ad7f5b..f1105fc8 100644 --- a/src/bitmap/store/array_store/mod.rs +++ b/src/bitmap/store/array_store/mod.rs @@ -574,4 +574,11 @@ mod tests { store.remove_first(3); assert_eq!(into_vec(store), vec![500]); } + + #[test] + fn test_bitmap_remove_last() { + let mut store = Store::Array(ArrayStore::from_vec_unchecked(vec![1, 2, 130, 500])); + store.remove_last(2); + assert_eq!(into_vec(store), vec![1, 2]); + } } diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 1cee249f..ec5cc471 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -329,9 +329,33 @@ impl BitmapStore { } } } + + /// Set N bits that are currently 1 bit from the lower bit to 0. + pub fn remove_last(&mut self, mut clear_bits: usize) { + if self.len() < clear_bits as u64 { + return; + } + let mut removed: u64 = 0; + let max = self.max().unwrap(); + let key = key(max); + for index in (0..=key).rev() { + let bit = self.bits[index].count_ones(); + let mut mask = 1 << bit - 1; + let mut count = self.bits[index].count_ones(); + while count > 0 { + if self.bits[index] & mask == mask { + self.bits[index] &= !mask; + clear_bits -= 1; + removed += 1; + if clear_bits <= 0 { + self.len -= removed; + return; + } + } + mask >>= 1; + count -= 1; } } - true } } diff --git a/src/bitmap/store/mod.rs b/src/bitmap/store/mod.rs index b094606b..a20d849c 100644 --- a/src/bitmap/store/mod.rs +++ b/src/bitmap/store/mod.rs @@ -103,6 +103,13 @@ impl Store { } } + pub fn remove_last(&mut self, index: usize) { + match self { + Array(vec) => vec.remove_last(index), + Bitmap(bits) => bits.remove_last(index), + } + } + pub fn contains(&self, index: u16) -> bool { match self { Array(vec) => vec.contains(index), From e8e9be6a3ce24beeb4191493e75f4f4e170784d7 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 8 May 2023 16:38:08 +0900 Subject: [PATCH 06/44] Fix container remove target --- src/bitmap/inherent.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 1bc3bd1f..9b7ecb9f 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -617,7 +617,17 @@ impl RoaringBitmap { } /// Removes the specified number of elements from the tail. - /// todo doc test + /// + /// # Examples + /// + /// ```rust + /// use roaring::RoaringBitmap; + /// + /// let mut rb = RoaringBitmap::from_iter([1, 5, 7, 9]); + /// rb.remove_last(2); + /// assert_eq!(rb, RoaringBitmap::from_iter([1, 5])); + /// rb.remove_last(1); + /// assert_eq!(rb, RoaringBitmap::from_iter([1])); pub fn remove_last(&mut self, mut n: usize) { let container_len = self.containers.len(); let potision = self.containers.iter().enumerate().rev().find_map(|(index, container)| { @@ -632,7 +642,7 @@ impl RoaringBitmap { if let Some(position) = potision { let remove_container = container_len - position - 1; if remove_container > 0 { - self.containers.truncate(remove_container); + self.containers.truncate(self.containers.len() - remove_container); } if n > 0 { self.containers[position].remove_last(n); @@ -845,6 +855,11 @@ mod tests { bitmap.insert_range(0..6000); bitmap.remove_last(1000); assert_eq!(bitmap.len(), 5000); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..200000); + bitmap.remove_last(196000); + assert_eq!(bitmap.len(), 4000); } #[test] From 6a3ffb55deb895f213ca3f89ae4850662d880974 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 8 May 2023 17:48:42 +0900 Subject: [PATCH 07/44] Fix where counts are subtracted and add test --- src/bitmap/inherent.rs | 7 +++++++ src/bitmap/store/bitmap_store.rs | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 9b7ecb9f..69605b06 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -842,6 +842,13 @@ mod tests { bitmap.insert_range(0..8000); bitmap.remove_first(10); assert_eq!(bitmap.len(), 7990); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..2); + bitmap.insert_range(4..7); + bitmap.insert_range(1000..6000); + bitmap.remove_first(30); + assert_eq!(bitmap.len(), 4975); } #[test] diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index ec5cc471..3c6d55e2 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -319,13 +319,13 @@ impl BitmapStore { self.bits[index] &= !mask; clear_bits -= 1; removed += 1; + count -= 1; if clear_bits <= 0 { self.len -= removed; return; } } mask <<= 1; - count -= 1; } } } @@ -347,13 +347,13 @@ impl BitmapStore { self.bits[index] &= !mask; clear_bits -= 1; removed += 1; + count -= 1; if clear_bits <= 0 { self.len -= removed; return; } } mask >>= 1; - count -= 1; } } } From 43a88e99b09f19de42ff6ee32d9ac4341b4d9d2d Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Tue, 9 May 2023 21:31:48 +0900 Subject: [PATCH 08/44] Add handle of cases where the target to be deleted is larger than the length of the container --- src/bitmap/inherent.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 69605b06..5dd9375c 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -593,6 +593,9 @@ impl RoaringBitmap { /// rb.remove_first(2); /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); pub fn remove_first(&mut self, mut n: usize) { + if n > self.len() as usize { + return; + } // remove containers up to the front of the target let position = self.containers.iter().position(|container| { let container_len = container.len() as usize; @@ -631,6 +634,9 @@ impl RoaringBitmap { pub fn remove_last(&mut self, mut n: usize) { let container_len = self.containers.len(); let potision = self.containers.iter().enumerate().rev().find_map(|(index, container)| { + if n > self.len() as usize { + return; + } let container_len = container.len() as usize; if container_len >= n { Some(index) From e92df34a92446e2ca49deea916396f8e7a0bd828 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Tue, 9 May 2023 21:33:13 +0900 Subject: [PATCH 09/44] Fix to use rposition for remove_last --- src/bitmap/inherent.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 5dd9375c..25a76645 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -632,24 +632,21 @@ impl RoaringBitmap { /// rb.remove_last(1); /// assert_eq!(rb, RoaringBitmap::from_iter([1])); pub fn remove_last(&mut self, mut n: usize) { - let container_len = self.containers.len(); - let potision = self.containers.iter().enumerate().rev().find_map(|(index, container)| { if n > self.len() as usize { return; } + // remove containers up to the back of the target + let position = self.containers.iter().rposition(|container| { let container_len = container.len() as usize; - if container_len >= n { - Some(index) - } else { + if container_len < n { n -= container_len; - None + false + } else { + true } }); - if let Some(position) = potision { - let remove_container = container_len - position - 1; - if remove_container > 0 { - self.containers.truncate(self.containers.len() - remove_container); - } + if let Some(position) = position { + self.containers.truncate(position + 1); if n > 0 { self.containers[position].remove_last(n); } From 85a8f0264d3db7e6ad2230ca71cd55e24016148a Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Tue, 9 May 2023 21:33:27 +0900 Subject: [PATCH 10/44] Add test --- src/bitmap/inherent.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 25a76645..e0eef4dd 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -816,15 +816,18 @@ mod tests { assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); bitmap = RoaringBitmap::from_iter([1, 3]); - bitmap.remove_first(1); - assert_eq!(bitmap.len(), 1); - assert_eq!(bitmap, RoaringBitmap::from_iter([3])); + bitmap.remove_first(2); + assert_eq!(bitmap.len(), 0); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..(1_u32 << 16) + 5); bitmap.remove_first(65537); assert_eq!(bitmap.len(), 4); assert_eq!(bitmap, RoaringBitmap::from_iter([65537, 65538, 65539, 65540])); + + bitmap = RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11]); + bitmap.remove_first(7); + assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11])); } #[test] @@ -846,6 +849,12 @@ mod tests { bitmap.remove_first(10); assert_eq!(bitmap.len(), 7990); + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..200000); + bitmap.remove_first(2000); + assert_eq!(bitmap.len(), 198000); + assert_eq!(bitmap, RoaringBitmap::from_iter(2000..200000)); + bitmap = RoaringBitmap::new(); bitmap.insert_range(0..2); bitmap.insert_range(4..7); @@ -870,6 +879,12 @@ mod tests { bitmap.insert_range(0..200000); bitmap.remove_last(196000); assert_eq!(bitmap.len(), 4000); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..200000); + bitmap.remove_last(2000); + assert_eq!(bitmap.len(), 198000); + assert_eq!(bitmap, RoaringBitmap::from_iter(0..198000)); } #[test] @@ -878,10 +893,18 @@ mod tests { bitmap.remove_last(2); assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7])); + bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); + bitmap.remove_last(6); + assert_eq!(bitmap.len(), 0); + bitmap = RoaringBitmap::new(); bitmap.insert_range(0..(1_u32 << 16) + 5); bitmap.remove_last(65537); assert_eq!(bitmap.len(), 4); assert_eq!(bitmap, RoaringBitmap::from_iter([0, 1, 2, 3])); + + let mut bitmap = RoaringBitmap::from_iter([1, 2, 3]); + bitmap.remove_last(4); + assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3])); } } From ede6cdea17606735824b9cbf317ea2bb916e1ec9 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Wed, 10 May 2023 23:57:01 +0900 Subject: [PATCH 11/44] Add test to delete 0 elements --- src/bitmap/inherent.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index e0eef4dd..f85cb37c 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -819,6 +819,11 @@ mod tests { bitmap.remove_first(2); assert_eq!(bitmap.len(), 0); + bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); + bitmap.remove_first(0); + assert_eq!(bitmap.len(), 6); + assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11])); + bitmap = RoaringBitmap::new(); bitmap.insert_range(0..(1_u32 << 16) + 5); bitmap.remove_first(65537); @@ -861,6 +866,11 @@ mod tests { bitmap.insert_range(1000..6000); bitmap.remove_first(30); assert_eq!(bitmap.len(), 4975); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..65535); + bitmap.remove_first(0); + assert_eq!(bitmap.len(), 65535); } #[test] @@ -885,6 +895,11 @@ mod tests { bitmap.remove_last(2000); assert_eq!(bitmap.len(), 198000); assert_eq!(bitmap, RoaringBitmap::from_iter(0..198000)); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..65535); + bitmap.remove_last(0); + assert_eq!(bitmap.len(), 65535); } #[test] @@ -897,6 +912,11 @@ mod tests { bitmap.remove_last(6); assert_eq!(bitmap.len(), 0); + bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); + bitmap.remove_last(0); + assert_eq!(bitmap.len(), 6); + assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11])); + bitmap = RoaringBitmap::new(); bitmap.insert_range(0..(1_u32 << 16) + 5); bitmap.remove_last(65537); From 33b540c19f0702c4690c69203fe112d8e86f2d21 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Thu, 11 May 2023 00:36:25 +0900 Subject: [PATCH 12/44] Remove unnecessary if statement --- src/bitmap/inherent.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index f85cb37c..bd6f8f3c 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -606,11 +606,11 @@ impl RoaringBitmap { true } }); - if let Some(position) = position { - if position > 0 { - self.containers.rotate_left(position); - self.containers.truncate(position); - } + // It is checked at the beginning of the function, so it is usually never an Err + let position = position.expect("there are no containers to delete"); + if position > 0 { + self.containers.rotate_left(position); + self.containers.truncate(position); } // remove data in containers if there are still targets for deletion if n > 0 { @@ -645,11 +645,11 @@ impl RoaringBitmap { true } }); - if let Some(position) = position { - self.containers.truncate(position + 1); - if n > 0 { - self.containers[position].remove_last(n); - } + // It is checked at the beginning of the function, so it is usually never an Err + let position = position.expect("there are no containers to delete"); + self.containers.truncate(position + 1); + if n > 0 { + self.containers[position].remove_last(n); } } } From f7601db7a6c83a74e0d588c0a90b374a8b92a4a0 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Thu, 11 May 2023 01:25:26 +0900 Subject: [PATCH 13/44] Add all bit clear --- src/bitmap/store/bitmap_store.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 3c6d55e2..905fe71e 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -306,6 +306,7 @@ impl BitmapStore { /// Set N bits that are currently 1 bit from the lower bit to 0. pub fn remove_first(&mut self, mut clear_bits: usize) { if self.len() < clear_bits as u64 { + *self = Self::default(); return; } let mut removed: u64 = 0; @@ -333,6 +334,7 @@ impl BitmapStore { /// Set N bits that are currently 1 bit from the lower bit to 0. pub fn remove_last(&mut self, mut clear_bits: usize) { if self.len() < clear_bits as u64 { + *self = Self::default(); return; } let mut removed: u64 = 0; From 1cd29e37c5017fef7a3d9418650069571a4fb292 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Thu, 11 May 2023 01:26:43 +0900 Subject: [PATCH 14/44] Change conditional expression --- src/bitmap/store/bitmap_store.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 905fe71e..a7a6b094 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -321,8 +321,7 @@ impl BitmapStore { clear_bits -= 1; removed += 1; count -= 1; - if clear_bits <= 0 { - self.len -= removed; + if clear_bits == 0 { return; } } @@ -350,8 +349,7 @@ impl BitmapStore { clear_bits -= 1; removed += 1; count -= 1; - if clear_bits <= 0 { - self.len -= removed; + if clear_bits == 0 { return; } } From 7f02d44e8d97933b28d23011f83ca892ce715252 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Thu, 11 May 2023 01:27:51 +0900 Subject: [PATCH 15/44] Remove hard to follow variables --- src/bitmap/store/bitmap_store.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index a7a6b094..0a6ab647 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -309,7 +309,7 @@ impl BitmapStore { *self = Self::default(); return; } - let mut removed: u64 = 0; + self.len -= clear_bits as u64; let min = self.min().unwrap(); let key = key(min); for index in key..BITMAP_LENGTH { @@ -319,7 +319,6 @@ impl BitmapStore { if self.bits[index] & mask == mask { self.bits[index] &= !mask; clear_bits -= 1; - removed += 1; count -= 1; if clear_bits == 0 { return; @@ -336,7 +335,7 @@ impl BitmapStore { *self = Self::default(); return; } - let mut removed: u64 = 0; + self.len -= clear_bits as u64; let max = self.max().unwrap(); let key = key(max); for index in (0..=key).rev() { @@ -347,7 +346,6 @@ impl BitmapStore { if self.bits[index] & mask == mask { self.bits[index] &= !mask; clear_bits -= 1; - removed += 1; count -= 1; if clear_bits == 0 { return; From ad195572b0015f2e4cfc89f0c7391c5c8aa927ab Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Thu, 11 May 2023 01:29:47 +0900 Subject: [PATCH 16/44] Change the part that loops unnecessary --- src/bitmap/store/bitmap_store.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 0a6ab647..519011da 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -315,6 +315,11 @@ impl BitmapStore { for index in key..BITMAP_LENGTH { let mut mask = 1; let mut count = self.bits[index].count_ones(); + if clear_bits > count as usize { + self.bits[index] = 0; + clear_bits -= count as usize; + continue; + } while count > 0 { if self.bits[index] & mask == mask { self.bits[index] &= !mask; @@ -342,6 +347,11 @@ impl BitmapStore { let bit = self.bits[index].count_ones(); let mut mask = 1 << bit - 1; let mut count = self.bits[index].count_ones(); + if clear_bits > count as usize { + self.bits[index] = 0; + clear_bits -= count as usize; + continue; + } while count > 0 { if self.bits[index] & mask == mask { self.bits[index] &= !mask; From 4b2384bde64412c23f36e32ff10eb5437055e647 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Sun, 7 May 2023 12:40:07 +0900 Subject: [PATCH 17/44] Feat remove first --- src/bitmap/container.rs | 6 +++++ src/bitmap/inherent.rs | 36 +++++++++++++++++++++++++++++ src/bitmap/store/array_store/mod.rs | 13 +++++++++++ src/bitmap/store/bitmap_store.rs | 35 ++++++++++++++++++++++++++++ src/bitmap/store/mod.rs | 7 ++++++ 5 files changed, 97 insertions(+) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index 9cbab634..7d9afc73 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -94,6 +94,12 @@ impl Container { result } + pub fn remove_first(&mut self, n: usize) -> bool { + let result = self.store.remove_first(n); + self.ensure_correct_store(); + result + } + pub fn contains(&self, index: u16) -> bool { self.store.contains(index) } diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 60e9e619..ff1c7ba6 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -578,6 +578,42 @@ impl RoaringBitmap { None } + + /// Removes the specified number of elements from the top. + /// + /// # Examples + /// + /// ```rust + /// use roaring::RoaringBitmap; + /// + /// let mut rb = RoaringBitmap::from_iter([1, 65535, 65536, 65589]); + /// rb.remove_first(2); + /// assert_eq!(rb, RoaringBitmap::from_iter([65536, 65589])); + /// rb.remove_first(1); + /// assert_eq!(rb, RoaringBitmap::from_iter([65589])); + pub fn remove_first(&mut self, n: usize) -> bool { + // remove containers up to the front of the target + let loc = self.select(n as u32).unwrap(); + let old_len = self.len(); + let remove_key = loc / (u16::MAX as usize + 1) as u32; + for i in 0..remove_key { + match self.containers.binary_search_by_key(&i, |c| c.key.into()) { + Ok(loc) => { + self.containers.remove(loc); + } + _ => {} + } + } + let removed_len = (old_len - self.len()) as usize; + + // remove data in containers if there are still targets for deletion + if removed_len < n { + // container immediately before should have been deleted, so the target is 0 index + let result = self.containers[0].remove_first(n - removed_len); + return result; + } + false + } } impl Default for RoaringBitmap { diff --git a/src/bitmap/store/array_store/mod.rs b/src/bitmap/store/array_store/mod.rs index 543663df..7e0c3796 100644 --- a/src/bitmap/store/array_store/mod.rs +++ b/src/bitmap/store/array_store/mod.rs @@ -110,6 +110,12 @@ impl ArrayStore { (pos_end - pos_start) as u64 } + pub fn remove_first(&mut self, n: usize) -> bool { + self.vec.rotate_left(n.into()); + self.vec.truncate(self.vec.len() - n as usize); + true + } + pub fn contains(&self, index: u16) -> bool { self.vec.binary_search(&index).is_ok() } @@ -562,4 +568,11 @@ mod tests { assert_eq!(into_vec(store), want); } + + #[test] + fn test_bitmap_remove_first() { + let mut store = Store::Array(ArrayStore::from_vec_unchecked(vec![1, 2, 130, 500])); + store.remove_first(3); + assert_eq!(into_vec(store), vec![500]); + } } diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 55a53c57..6212ed36 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -302,6 +302,21 @@ impl BitmapStore { pub fn as_array(&self) -> &[u64; BITMAP_LENGTH] { &self.bits } + + /// Set N bits that are currently 1 bit from the lower bit to 0. + pub fn remove_first(&mut self, mut clear_bits: usize) -> bool { + if self.bits[0].count_ones() > clear_bits as u32 { + let mut mask = 1; + while clear_bits > 0 { + if self.bits[0] & mask == mask { + self.bits[0] &= !mask; + clear_bits -= 1; + } + mask <<= 1; + } + } + true + } } // this can be done in 3 instructions on x86-64 with bmi2 with: tzcnt(pdep(1 << rank, value)) @@ -490,3 +505,23 @@ impl BitXorAssign<&ArrayStore> for BitmapStore { self.len = len as u64; } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_bitmap_remove_first() { + let mut store = BitmapStore::new(); + let range = RangeInclusive::new(1, 3); + store.insert_range(range); + let range_second = RangeInclusive::new(5, 65535); + // store.bits[0] = 0b1111111111111111111111111111111111111111111111111111111111101110 + store.insert_range(range_second); + store.remove_first(2); + assert_eq!( + store.bits[0], + 0b1111111111111111111111111111111111111111111111111111111111101000 + ); + } +} diff --git a/src/bitmap/store/mod.rs b/src/bitmap/store/mod.rs index 9172a01a..e690663a 100644 --- a/src/bitmap/store/mod.rs +++ b/src/bitmap/store/mod.rs @@ -106,6 +106,13 @@ impl Store { } } + pub fn remove_first(&mut self, index: usize) -> bool { + match self { + Array(vec) => vec.remove_first(index), + Bitmap(bits) => bits.remove_first(index), + } + } + pub fn contains(&self, index: u16) -> bool { match self { Array(vec) => vec.contains(index), From 0a2be31475d5c872c54d3d52881d6ff2bedd9eb4 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Sun, 7 May 2023 22:12:18 +0900 Subject: [PATCH 18/44] Fix process --- src/bitmap/inherent.rs | 66 +++++++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index ff1c7ba6..49ff86a0 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -586,33 +586,35 @@ impl RoaringBitmap { /// ```rust /// use roaring::RoaringBitmap; /// - /// let mut rb = RoaringBitmap::from_iter([1, 65535, 65536, 65589]); + /// let mut rb = RoaringBitmap::from_iter([1, 5, 7, 9]); /// rb.remove_first(2); - /// assert_eq!(rb, RoaringBitmap::from_iter([65536, 65589])); - /// rb.remove_first(1); - /// assert_eq!(rb, RoaringBitmap::from_iter([65589])); - pub fn remove_first(&mut self, n: usize) -> bool { + /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); + /// rb = RoaringBitmap::from_iter([1, 3, 7, 9]); + /// rb.remove_first(2); + /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); + pub fn remove_first(&mut self, mut n: usize) { // remove containers up to the front of the target - let loc = self.select(n as u32).unwrap(); - let old_len = self.len(); - let remove_key = loc / (u16::MAX as usize + 1) as u32; - for i in 0..remove_key { - match self.containers.binary_search_by_key(&i, |c| c.key.into()) { - Ok(loc) => { - self.containers.remove(loc); - } - _ => {} + let position = self.containers.iter().position(|container| { + let container_len = container.len() as usize; + if container_len < n { + n -= container_len; + false + } else { + true + } + }); + if let Some(position) = position { + if position > 0 { + self.containers.rotate_left(position); + self.containers.truncate(position); } } - let removed_len = (old_len - self.len()) as usize; - // remove data in containers if there are still targets for deletion - if removed_len < n { + if n > 0 { // container immediately before should have been deleted, so the target is 0 index - let result = self.containers[0].remove_first(n - removed_len); - return result; + self.containers[0].remove_first(n); } - false + } } } @@ -764,4 +766,28 @@ mod tests { assert_eq!(bitmap.containers.len(), 1); assert_eq!(bitmap.containers[0].key, 1); } + + #[test] + fn remove_first() { + let mut bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); + bitmap.remove_first(3); + assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); + + bitmap = RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11]); + bitmap.remove_first(3); + assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); + + let mut rb = RoaringBitmap::from_iter([1, 3]); + rb.remove_first(1); + println!("{:?}", rb); + assert_eq!(rb.len(), 1); + } + + #[test] + fn remove_first_for_bit() { + let mut bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..(1_u32 << 16) + 3); + bitmap.remove_first(65537); + println!("{:?}", bitmap); + } } From cca8f22bcfa43c6e01ec99f9ef8735703facaf79 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 8 May 2023 01:16:29 +0900 Subject: [PATCH 19/44] Fix remove first function --- src/bitmap/container.rs | 16 +++++++++++++-- src/bitmap/inherent.rs | 32 +++++++++++++++++++++-------- src/bitmap/store/array_store/mod.rs | 7 +++++-- src/bitmap/store/bitmap_store.rs | 2 +- src/bitmap/store/mod.rs | 2 +- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index 7d9afc73..c2cbfa55 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -94,8 +94,20 @@ impl Container { result } - pub fn remove_first(&mut self, n: usize) -> bool { - let result = self.store.remove_first(n); + pub fn remove_first(&mut self, n: usize) { + match &self.store { + Store::Bitmap(bits) => { + if bits.len() - n as u64 <= ARRAY_LIMIT { + let mut replace_array = Vec::with_capacity(bits.len() as usize - n); + replace_array.extend(bits.clone().into_iter().skip(n)); + self.store = Store::Array(store::ArrayStore::from_vec_unchecked(replace_array)); + } else { + self.store.remove_first(n) + } + } + Store::Array(_) => self.store.remove_first(n), + }; + } self.ensure_correct_store(); result } diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 49ff86a0..1c961b48 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -768,26 +768,42 @@ mod tests { } #[test] - fn remove_first() { + fn remove_first_for_vec() { let mut bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); bitmap.remove_first(3); + assert_eq!(bitmap.len(), 3); assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); bitmap = RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11]); bitmap.remove_first(3); + assert_eq!(bitmap.len(), 3); assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); - let mut rb = RoaringBitmap::from_iter([1, 3]); - rb.remove_first(1); - println!("{:?}", rb); - assert_eq!(rb.len(), 1); + bitmap = RoaringBitmap::from_iter([1, 3]); + bitmap.remove_first(1); + assert_eq!(bitmap.len(), 1); + assert_eq!(bitmap, RoaringBitmap::from_iter([3])); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..(1_u32 << 16) + 5); + bitmap.remove_first(65537); + assert_eq!(bitmap.len(), 4); + assert_eq!(bitmap, RoaringBitmap::from_iter([65537, 65538, 65539, 65540])); } #[test] fn remove_first_for_bit() { let mut bitmap = RoaringBitmap::new(); - bitmap.insert_range(0..(1_u32 << 16) + 3); - bitmap.remove_first(65537); - println!("{:?}", bitmap); + bitmap.insert_range(0..4098); + bitmap.remove_first(4095); + assert_eq!(bitmap.len(), 3); + // removed bit to vec + assert_eq!(bitmap, RoaringBitmap::from_iter([4095, 4096, 4097])); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..(1_u32 << 16)); + bitmap.remove_first(65536); + assert_eq!(bitmap.len(), 0); + } } } diff --git a/src/bitmap/store/array_store/mod.rs b/src/bitmap/store/array_store/mod.rs index 7e0c3796..86b3e81a 100644 --- a/src/bitmap/store/array_store/mod.rs +++ b/src/bitmap/store/array_store/mod.rs @@ -110,10 +110,13 @@ impl ArrayStore { (pos_end - pos_start) as u64 } - pub fn remove_first(&mut self, n: usize) -> bool { + pub fn remove_first(&mut self, n: usize) { self.vec.rotate_left(n.into()); self.vec.truncate(self.vec.len() - n as usize); - true + } + + pub fn remove_last(&mut self, n: usize) { + self.vec.truncate(self.vec.len() - n as usize); } pub fn contains(&self, index: u16) -> bool { diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 6212ed36..3ba116b1 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -304,7 +304,7 @@ impl BitmapStore { } /// Set N bits that are currently 1 bit from the lower bit to 0. - pub fn remove_first(&mut self, mut clear_bits: usize) -> bool { + pub fn remove_first(&mut self, mut clear_bits: usize) { if self.bits[0].count_ones() > clear_bits as u32 { let mut mask = 1; while clear_bits > 0 { diff --git a/src/bitmap/store/mod.rs b/src/bitmap/store/mod.rs index e690663a..9e117020 100644 --- a/src/bitmap/store/mod.rs +++ b/src/bitmap/store/mod.rs @@ -106,7 +106,7 @@ impl Store { } } - pub fn remove_first(&mut self, index: usize) -> bool { + pub fn remove_first(&mut self, index: usize) { match self { Array(vec) => vec.remove_first(index), Bitmap(bits) => bits.remove_first(index), From da127811620ef8d30b94267fa4a80751e29b9ba7 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 8 May 2023 14:12:48 +0900 Subject: [PATCH 20/44] Fix bit switching process of remove_first --- src/bitmap/store/bitmap_store.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 3ba116b1..1cee249f 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -305,14 +305,30 @@ impl BitmapStore { /// Set N bits that are currently 1 bit from the lower bit to 0. pub fn remove_first(&mut self, mut clear_bits: usize) { - if self.bits[0].count_ones() > clear_bits as u32 { + if self.len() < clear_bits as u64 { + return; + } + let mut removed: u64 = 0; + let min = self.min().unwrap(); + let key = key(min); + for index in key..BITMAP_LENGTH { let mut mask = 1; - while clear_bits > 0 { - if self.bits[0] & mask == mask { - self.bits[0] &= !mask; + let mut count = self.bits[index].count_ones(); + while count > 0 { + if self.bits[index] & mask == mask { + self.bits[index] &= !mask; clear_bits -= 1; + removed += 1; + if clear_bits <= 0 { + self.len -= removed; + return; + } } mask <<= 1; + count -= 1; + } + } + } } } true From 88880f8fd5ed088a9c16ab9079762db7a9dfe2ff Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 8 May 2023 14:13:23 +0900 Subject: [PATCH 21/44] Feat remove last --- src/bitmap/container.rs | 17 ++++++++- src/bitmap/inherent.rs | 59 +++++++++++++++++++++++++++-- src/bitmap/store/array_store/mod.rs | 7 ++++ src/bitmap/store/bitmap_store.rs | 26 ++++++++++++- src/bitmap/store/mod.rs | 7 ++++ 5 files changed, 110 insertions(+), 6 deletions(-) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index c2cbfa55..5dba79d9 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -108,8 +108,21 @@ impl Container { Store::Array(_) => self.store.remove_first(n), }; } - self.ensure_correct_store(); - result + + pub fn remove_last(&mut self, n: usize) { + match &self.store { + Store::Bitmap(bits) => { + if bits.len() - n as u64 <= ARRAY_LIMIT { + let mut replace_array = Vec::with_capacity(bits.len() as usize - n); + replace_array.extend(bits.clone().into_iter()); + replace_array.truncate(replace_array.len() - n); + self.store = Store::Array(store::ArrayStore::from_vec_unchecked(replace_array)); + } else { + self.store.remove_last(n) + } + } + Store::Array(_) => self.store.remove_last(n), + }; } pub fn contains(&self, index: u16) -> bool { diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 1c961b48..1bc3bd1f 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -615,6 +615,29 @@ impl RoaringBitmap { self.containers[0].remove_first(n); } } + + /// Removes the specified number of elements from the tail. + /// todo doc test + pub fn remove_last(&mut self, mut n: usize) { + let container_len = self.containers.len(); + let potision = self.containers.iter().enumerate().rev().find_map(|(index, container)| { + let container_len = container.len() as usize; + if container_len >= n { + Some(index) + } else { + n -= container_len; + None + } + }); + if let Some(position) = potision { + let remove_container = container_len - position - 1; + if remove_container > 0 { + self.containers.truncate(remove_container); + } + if n > 0 { + self.containers[position].remove_last(n); + } + } } } @@ -801,9 +824,39 @@ mod tests { assert_eq!(bitmap, RoaringBitmap::from_iter([4095, 4096, 4097])); bitmap = RoaringBitmap::new(); - bitmap.insert_range(0..(1_u32 << 16)); - bitmap.remove_first(65536); - assert_eq!(bitmap.len(), 0); + bitmap.insert_range(0..6000); + bitmap.remove_first(999); + assert_eq!(bitmap.len(), 5001); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..8000); + bitmap.remove_first(10); + assert_eq!(bitmap.len(), 7990); + } + + #[test] + fn remove_last_for_bit() { + let mut bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..5000); + bitmap.remove_last(1000); + assert_eq!(bitmap.len(), 4000); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..6000); + bitmap.remove_last(1000); + assert_eq!(bitmap.len(), 5000); } + + #[test] + fn remove_last_for_vec() { + let mut bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); + bitmap.remove_last(2); + assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7])); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..(1_u32 << 16) + 5); + bitmap.remove_last(65537); + assert_eq!(bitmap.len(), 4); + assert_eq!(bitmap, RoaringBitmap::from_iter([0, 1, 2, 3])); } } diff --git a/src/bitmap/store/array_store/mod.rs b/src/bitmap/store/array_store/mod.rs index 86b3e81a..916f3708 100644 --- a/src/bitmap/store/array_store/mod.rs +++ b/src/bitmap/store/array_store/mod.rs @@ -578,4 +578,11 @@ mod tests { store.remove_first(3); assert_eq!(into_vec(store), vec![500]); } + + #[test] + fn test_bitmap_remove_last() { + let mut store = Store::Array(ArrayStore::from_vec_unchecked(vec![1, 2, 130, 500])); + store.remove_last(2); + assert_eq!(into_vec(store), vec![1, 2]); + } } diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 1cee249f..ec5cc471 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -329,9 +329,33 @@ impl BitmapStore { } } } + + /// Set N bits that are currently 1 bit from the lower bit to 0. + pub fn remove_last(&mut self, mut clear_bits: usize) { + if self.len() < clear_bits as u64 { + return; + } + let mut removed: u64 = 0; + let max = self.max().unwrap(); + let key = key(max); + for index in (0..=key).rev() { + let bit = self.bits[index].count_ones(); + let mut mask = 1 << bit - 1; + let mut count = self.bits[index].count_ones(); + while count > 0 { + if self.bits[index] & mask == mask { + self.bits[index] &= !mask; + clear_bits -= 1; + removed += 1; + if clear_bits <= 0 { + self.len -= removed; + return; + } + } + mask >>= 1; + count -= 1; } } - true } } diff --git a/src/bitmap/store/mod.rs b/src/bitmap/store/mod.rs index 9e117020..1559677c 100644 --- a/src/bitmap/store/mod.rs +++ b/src/bitmap/store/mod.rs @@ -113,6 +113,13 @@ impl Store { } } + pub fn remove_last(&mut self, index: usize) { + match self { + Array(vec) => vec.remove_last(index), + Bitmap(bits) => bits.remove_last(index), + } + } + pub fn contains(&self, index: u16) -> bool { match self { Array(vec) => vec.contains(index), From 9b200fb3ce1b9d28a770b51fe0a652b3ac4220d3 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 8 May 2023 16:38:08 +0900 Subject: [PATCH 22/44] Fix container remove target --- src/bitmap/inherent.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 1bc3bd1f..9b7ecb9f 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -617,7 +617,17 @@ impl RoaringBitmap { } /// Removes the specified number of elements from the tail. - /// todo doc test + /// + /// # Examples + /// + /// ```rust + /// use roaring::RoaringBitmap; + /// + /// let mut rb = RoaringBitmap::from_iter([1, 5, 7, 9]); + /// rb.remove_last(2); + /// assert_eq!(rb, RoaringBitmap::from_iter([1, 5])); + /// rb.remove_last(1); + /// assert_eq!(rb, RoaringBitmap::from_iter([1])); pub fn remove_last(&mut self, mut n: usize) { let container_len = self.containers.len(); let potision = self.containers.iter().enumerate().rev().find_map(|(index, container)| { @@ -632,7 +642,7 @@ impl RoaringBitmap { if let Some(position) = potision { let remove_container = container_len - position - 1; if remove_container > 0 { - self.containers.truncate(remove_container); + self.containers.truncate(self.containers.len() - remove_container); } if n > 0 { self.containers[position].remove_last(n); @@ -845,6 +855,11 @@ mod tests { bitmap.insert_range(0..6000); bitmap.remove_last(1000); assert_eq!(bitmap.len(), 5000); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..200000); + bitmap.remove_last(196000); + assert_eq!(bitmap.len(), 4000); } #[test] From 66664185e599a5b8f549941c5e1c0ceb2dba4248 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 8 May 2023 17:48:42 +0900 Subject: [PATCH 23/44] Fix where counts are subtracted and add test --- src/bitmap/inherent.rs | 7 +++++++ src/bitmap/store/bitmap_store.rs | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 9b7ecb9f..69605b06 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -842,6 +842,13 @@ mod tests { bitmap.insert_range(0..8000); bitmap.remove_first(10); assert_eq!(bitmap.len(), 7990); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..2); + bitmap.insert_range(4..7); + bitmap.insert_range(1000..6000); + bitmap.remove_first(30); + assert_eq!(bitmap.len(), 4975); } #[test] diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index ec5cc471..3c6d55e2 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -319,13 +319,13 @@ impl BitmapStore { self.bits[index] &= !mask; clear_bits -= 1; removed += 1; + count -= 1; if clear_bits <= 0 { self.len -= removed; return; } } mask <<= 1; - count -= 1; } } } @@ -347,13 +347,13 @@ impl BitmapStore { self.bits[index] &= !mask; clear_bits -= 1; removed += 1; + count -= 1; if clear_bits <= 0 { self.len -= removed; return; } } mask >>= 1; - count -= 1; } } } From ca654797ff453ea5b750770eed395ff3306d9e6a Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Tue, 9 May 2023 21:31:48 +0900 Subject: [PATCH 24/44] Add handle of cases where the target to be deleted is larger than the length of the container --- src/bitmap/inherent.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 69605b06..5dd9375c 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -593,6 +593,9 @@ impl RoaringBitmap { /// rb.remove_first(2); /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); pub fn remove_first(&mut self, mut n: usize) { + if n > self.len() as usize { + return; + } // remove containers up to the front of the target let position = self.containers.iter().position(|container| { let container_len = container.len() as usize; @@ -631,6 +634,9 @@ impl RoaringBitmap { pub fn remove_last(&mut self, mut n: usize) { let container_len = self.containers.len(); let potision = self.containers.iter().enumerate().rev().find_map(|(index, container)| { + if n > self.len() as usize { + return; + } let container_len = container.len() as usize; if container_len >= n { Some(index) From e6442aeea8af7c1ad18b154636427c41940753dd Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Tue, 9 May 2023 21:33:13 +0900 Subject: [PATCH 25/44] Fix to use rposition for remove_last --- src/bitmap/inherent.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 5dd9375c..25a76645 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -632,24 +632,21 @@ impl RoaringBitmap { /// rb.remove_last(1); /// assert_eq!(rb, RoaringBitmap::from_iter([1])); pub fn remove_last(&mut self, mut n: usize) { - let container_len = self.containers.len(); - let potision = self.containers.iter().enumerate().rev().find_map(|(index, container)| { if n > self.len() as usize { return; } + // remove containers up to the back of the target + let position = self.containers.iter().rposition(|container| { let container_len = container.len() as usize; - if container_len >= n { - Some(index) - } else { + if container_len < n { n -= container_len; - None + false + } else { + true } }); - if let Some(position) = potision { - let remove_container = container_len - position - 1; - if remove_container > 0 { - self.containers.truncate(self.containers.len() - remove_container); - } + if let Some(position) = position { + self.containers.truncate(position + 1); if n > 0 { self.containers[position].remove_last(n); } From 43392213dbc80e08e196a4de6393b6b9a1087462 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Tue, 9 May 2023 21:33:27 +0900 Subject: [PATCH 26/44] Add test --- src/bitmap/inherent.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 25a76645..e0eef4dd 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -816,15 +816,18 @@ mod tests { assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); bitmap = RoaringBitmap::from_iter([1, 3]); - bitmap.remove_first(1); - assert_eq!(bitmap.len(), 1); - assert_eq!(bitmap, RoaringBitmap::from_iter([3])); + bitmap.remove_first(2); + assert_eq!(bitmap.len(), 0); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..(1_u32 << 16) + 5); bitmap.remove_first(65537); assert_eq!(bitmap.len(), 4); assert_eq!(bitmap, RoaringBitmap::from_iter([65537, 65538, 65539, 65540])); + + bitmap = RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11]); + bitmap.remove_first(7); + assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11])); } #[test] @@ -846,6 +849,12 @@ mod tests { bitmap.remove_first(10); assert_eq!(bitmap.len(), 7990); + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..200000); + bitmap.remove_first(2000); + assert_eq!(bitmap.len(), 198000); + assert_eq!(bitmap, RoaringBitmap::from_iter(2000..200000)); + bitmap = RoaringBitmap::new(); bitmap.insert_range(0..2); bitmap.insert_range(4..7); @@ -870,6 +879,12 @@ mod tests { bitmap.insert_range(0..200000); bitmap.remove_last(196000); assert_eq!(bitmap.len(), 4000); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..200000); + bitmap.remove_last(2000); + assert_eq!(bitmap.len(), 198000); + assert_eq!(bitmap, RoaringBitmap::from_iter(0..198000)); } #[test] @@ -878,10 +893,18 @@ mod tests { bitmap.remove_last(2); assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7])); + bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); + bitmap.remove_last(6); + assert_eq!(bitmap.len(), 0); + bitmap = RoaringBitmap::new(); bitmap.insert_range(0..(1_u32 << 16) + 5); bitmap.remove_last(65537); assert_eq!(bitmap.len(), 4); assert_eq!(bitmap, RoaringBitmap::from_iter([0, 1, 2, 3])); + + let mut bitmap = RoaringBitmap::from_iter([1, 2, 3]); + bitmap.remove_last(4); + assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3])); } } From 216bb8bdbc1334f81f95881bf7c15cf08806608b Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Wed, 10 May 2023 23:57:01 +0900 Subject: [PATCH 27/44] Add test to delete 0 elements --- src/bitmap/inherent.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index e0eef4dd..f85cb37c 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -819,6 +819,11 @@ mod tests { bitmap.remove_first(2); assert_eq!(bitmap.len(), 0); + bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); + bitmap.remove_first(0); + assert_eq!(bitmap.len(), 6); + assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11])); + bitmap = RoaringBitmap::new(); bitmap.insert_range(0..(1_u32 << 16) + 5); bitmap.remove_first(65537); @@ -861,6 +866,11 @@ mod tests { bitmap.insert_range(1000..6000); bitmap.remove_first(30); assert_eq!(bitmap.len(), 4975); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..65535); + bitmap.remove_first(0); + assert_eq!(bitmap.len(), 65535); } #[test] @@ -885,6 +895,11 @@ mod tests { bitmap.remove_last(2000); assert_eq!(bitmap.len(), 198000); assert_eq!(bitmap, RoaringBitmap::from_iter(0..198000)); + + bitmap = RoaringBitmap::new(); + bitmap.insert_range(0..65535); + bitmap.remove_last(0); + assert_eq!(bitmap.len(), 65535); } #[test] @@ -897,6 +912,11 @@ mod tests { bitmap.remove_last(6); assert_eq!(bitmap.len(), 0); + bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); + bitmap.remove_last(0); + assert_eq!(bitmap.len(), 6); + assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11])); + bitmap = RoaringBitmap::new(); bitmap.insert_range(0..(1_u32 << 16) + 5); bitmap.remove_last(65537); From 72475321e7b1be511b2f3155e217c2c5a4b128a4 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Thu, 11 May 2023 00:36:25 +0900 Subject: [PATCH 28/44] Remove unnecessary if statement --- src/bitmap/inherent.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index f85cb37c..bd6f8f3c 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -606,11 +606,11 @@ impl RoaringBitmap { true } }); - if let Some(position) = position { - if position > 0 { - self.containers.rotate_left(position); - self.containers.truncate(position); - } + // It is checked at the beginning of the function, so it is usually never an Err + let position = position.expect("there are no containers to delete"); + if position > 0 { + self.containers.rotate_left(position); + self.containers.truncate(position); } // remove data in containers if there are still targets for deletion if n > 0 { @@ -645,11 +645,11 @@ impl RoaringBitmap { true } }); - if let Some(position) = position { - self.containers.truncate(position + 1); - if n > 0 { - self.containers[position].remove_last(n); - } + // It is checked at the beginning of the function, so it is usually never an Err + let position = position.expect("there are no containers to delete"); + self.containers.truncate(position + 1); + if n > 0 { + self.containers[position].remove_last(n); } } } From 8640495dbcdf0e3b000912e647c76b6e19e09746 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Thu, 11 May 2023 01:25:26 +0900 Subject: [PATCH 29/44] Add all bit clear --- src/bitmap/store/bitmap_store.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 3c6d55e2..905fe71e 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -306,6 +306,7 @@ impl BitmapStore { /// Set N bits that are currently 1 bit from the lower bit to 0. pub fn remove_first(&mut self, mut clear_bits: usize) { if self.len() < clear_bits as u64 { + *self = Self::default(); return; } let mut removed: u64 = 0; @@ -333,6 +334,7 @@ impl BitmapStore { /// Set N bits that are currently 1 bit from the lower bit to 0. pub fn remove_last(&mut self, mut clear_bits: usize) { if self.len() < clear_bits as u64 { + *self = Self::default(); return; } let mut removed: u64 = 0; From 7e3ef24078e625caf41289d738c933c33037c72f Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Thu, 11 May 2023 01:26:43 +0900 Subject: [PATCH 30/44] Change conditional expression --- src/bitmap/store/bitmap_store.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 905fe71e..a7a6b094 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -321,8 +321,7 @@ impl BitmapStore { clear_bits -= 1; removed += 1; count -= 1; - if clear_bits <= 0 { - self.len -= removed; + if clear_bits == 0 { return; } } @@ -350,8 +349,7 @@ impl BitmapStore { clear_bits -= 1; removed += 1; count -= 1; - if clear_bits <= 0 { - self.len -= removed; + if clear_bits == 0 { return; } } From 948d588cff49c396a910f8c4a0f3b466ffe8c9e1 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Thu, 11 May 2023 01:27:51 +0900 Subject: [PATCH 31/44] Remove hard to follow variables --- src/bitmap/store/bitmap_store.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index a7a6b094..0a6ab647 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -309,7 +309,7 @@ impl BitmapStore { *self = Self::default(); return; } - let mut removed: u64 = 0; + self.len -= clear_bits as u64; let min = self.min().unwrap(); let key = key(min); for index in key..BITMAP_LENGTH { @@ -319,7 +319,6 @@ impl BitmapStore { if self.bits[index] & mask == mask { self.bits[index] &= !mask; clear_bits -= 1; - removed += 1; count -= 1; if clear_bits == 0 { return; @@ -336,7 +335,7 @@ impl BitmapStore { *self = Self::default(); return; } - let mut removed: u64 = 0; + self.len -= clear_bits as u64; let max = self.max().unwrap(); let key = key(max); for index in (0..=key).rev() { @@ -347,7 +346,6 @@ impl BitmapStore { if self.bits[index] & mask == mask { self.bits[index] &= !mask; clear_bits -= 1; - removed += 1; count -= 1; if clear_bits == 0 { return; From 3abe4d5f1d4919761d5843a4687f56557f51f539 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Thu, 11 May 2023 01:29:47 +0900 Subject: [PATCH 32/44] Change the part that loops unnecessary --- src/bitmap/store/bitmap_store.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 0a6ab647..519011da 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -315,6 +315,11 @@ impl BitmapStore { for index in key..BITMAP_LENGTH { let mut mask = 1; let mut count = self.bits[index].count_ones(); + if clear_bits > count as usize { + self.bits[index] = 0; + clear_bits -= count as usize; + continue; + } while count > 0 { if self.bits[index] & mask == mask { self.bits[index] &= !mask; @@ -342,6 +347,11 @@ impl BitmapStore { let bit = self.bits[index].count_ones(); let mut mask = 1 << bit - 1; let mut count = self.bits[index].count_ones(); + if clear_bits > count as usize { + self.bits[index] = 0; + clear_bits -= count as usize; + continue; + } while count > 0 { if self.bits[index] & mask == mask { self.bits[index] &= !mask; From 5c7cb3b86317e4bcb361b57308da0766b2b55560 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 10 Jul 2023 23:33:46 +0900 Subject: [PATCH 33/44] fix test --- src/bitmap/inherent.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index bd6f8f3c..7e734685 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -589,7 +589,8 @@ impl RoaringBitmap { /// let mut rb = RoaringBitmap::from_iter([1, 5, 7, 9]); /// rb.remove_first(2); /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); - /// rb = RoaringBitmap::from_iter([1, 3, 7, 9]); + /// + /// let mut rb = RoaringBitmap::from_iter([1, 3, 7, 9]); /// rb.remove_first(2); /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); pub fn remove_first(&mut self, mut n: usize) { From b7c5b5a2acfeb33fab85784fb38cad76a6ee3367 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Mon, 10 Jul 2023 23:51:50 +0900 Subject: [PATCH 34/44] fixe to use u64 instead of usize --- src/bitmap/container.rs | 16 ++++++++-------- src/bitmap/inherent.rs | 12 ++++++------ src/bitmap/store/array_store/mod.rs | 6 +++--- src/bitmap/store/bitmap_store.rs | 22 +++++++++++----------- src/bitmap/store/mod.rs | 4 ++-- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index 5dba79d9..7ae5b1ac 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -94,12 +94,12 @@ impl Container { result } - pub fn remove_first(&mut self, n: usize) { + pub fn remove_first(&mut self, n: u64) { match &self.store { Store::Bitmap(bits) => { - if bits.len() - n as u64 <= ARRAY_LIMIT { - let mut replace_array = Vec::with_capacity(bits.len() as usize - n); - replace_array.extend(bits.clone().into_iter().skip(n)); + if bits.len() - n <= ARRAY_LIMIT { + let mut replace_array = Vec::with_capacity((bits.len() - n) as usize); + replace_array.extend(bits.clone().into_iter().skip(n as usize)); self.store = Store::Array(store::ArrayStore::from_vec_unchecked(replace_array)); } else { self.store.remove_first(n) @@ -109,13 +109,13 @@ impl Container { }; } - pub fn remove_last(&mut self, n: usize) { + pub fn remove_last(&mut self, n: u64) { match &self.store { Store::Bitmap(bits) => { - if bits.len() - n as u64 <= ARRAY_LIMIT { - let mut replace_array = Vec::with_capacity(bits.len() as usize - n); + if bits.len() - n <= ARRAY_LIMIT { + let mut replace_array = Vec::with_capacity((bits.len() - n) as usize); replace_array.extend(bits.clone().into_iter()); - replace_array.truncate(replace_array.len() - n); + replace_array.truncate(replace_array.len() - n as usize); self.store = Store::Array(store::ArrayStore::from_vec_unchecked(replace_array)); } else { self.store.remove_last(n) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 7e734685..e29d6cd8 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -593,13 +593,13 @@ impl RoaringBitmap { /// let mut rb = RoaringBitmap::from_iter([1, 3, 7, 9]); /// rb.remove_first(2); /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); - pub fn remove_first(&mut self, mut n: usize) { - if n > self.len() as usize { + pub fn remove_first(&mut self, mut n: u64) { + if n > self.len() { return; } // remove containers up to the front of the target let position = self.containers.iter().position(|container| { - let container_len = container.len() as usize; + let container_len = container.len(); if container_len < n { n -= container_len; false @@ -632,13 +632,13 @@ impl RoaringBitmap { /// assert_eq!(rb, RoaringBitmap::from_iter([1, 5])); /// rb.remove_last(1); /// assert_eq!(rb, RoaringBitmap::from_iter([1])); - pub fn remove_last(&mut self, mut n: usize) { - if n > self.len() as usize { + pub fn remove_last(&mut self, mut n: u64) { + if n > self.len() { return; } // remove containers up to the back of the target let position = self.containers.iter().rposition(|container| { - let container_len = container.len() as usize; + let container_len = container.len(); if container_len < n { n -= container_len; false diff --git a/src/bitmap/store/array_store/mod.rs b/src/bitmap/store/array_store/mod.rs index 916f3708..0fd4a848 100644 --- a/src/bitmap/store/array_store/mod.rs +++ b/src/bitmap/store/array_store/mod.rs @@ -110,12 +110,12 @@ impl ArrayStore { (pos_end - pos_start) as u64 } - pub fn remove_first(&mut self, n: usize) { - self.vec.rotate_left(n.into()); + pub fn remove_first(&mut self, n: u64) { + self.vec.rotate_left(n as usize); self.vec.truncate(self.vec.len() - n as usize); } - pub fn remove_last(&mut self, n: usize) { + pub fn remove_last(&mut self, n: u64) { self.vec.truncate(self.vec.len() - n as usize); } diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 519011da..10708d11 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -304,8 +304,8 @@ impl BitmapStore { } /// Set N bits that are currently 1 bit from the lower bit to 0. - pub fn remove_first(&mut self, mut clear_bits: usize) { - if self.len() < clear_bits as u64 { + pub fn remove_first(&mut self, mut clear_bits: u64) { + if self.len() < clear_bits { *self = Self::default(); return; } @@ -314,10 +314,10 @@ impl BitmapStore { let key = key(min); for index in key..BITMAP_LENGTH { let mut mask = 1; - let mut count = self.bits[index].count_ones(); - if clear_bits > count as usize { + let mut count = self.bits[index].count_ones() as u64; + if clear_bits > count { self.bits[index] = 0; - clear_bits -= count as usize; + clear_bits -= count; continue; } while count > 0 { @@ -335,21 +335,21 @@ impl BitmapStore { } /// Set N bits that are currently 1 bit from the lower bit to 0. - pub fn remove_last(&mut self, mut clear_bits: usize) { - if self.len() < clear_bits as u64 { + pub fn remove_last(&mut self, mut clear_bits: u64) { + if self.len() < clear_bits { *self = Self::default(); return; } - self.len -= clear_bits as u64; + self.len -= clear_bits; let max = self.max().unwrap(); let key = key(max); for index in (0..=key).rev() { let bit = self.bits[index].count_ones(); let mut mask = 1 << bit - 1; - let mut count = self.bits[index].count_ones(); - if clear_bits > count as usize { + let mut count = self.bits[index].count_ones() as u64; + if clear_bits > count { self.bits[index] = 0; - clear_bits -= count as usize; + clear_bits -= count; continue; } while count > 0 { diff --git a/src/bitmap/store/mod.rs b/src/bitmap/store/mod.rs index 1559677c..47deb261 100644 --- a/src/bitmap/store/mod.rs +++ b/src/bitmap/store/mod.rs @@ -106,14 +106,14 @@ impl Store { } } - pub fn remove_first(&mut self, index: usize) { + pub fn remove_first(&mut self, index: u64) { match self { Array(vec) => vec.remove_first(index), Bitmap(bits) => bits.remove_first(index), } } - pub fn remove_last(&mut self, index: usize) { + pub fn remove_last(&mut self, index: u64) { match self { Array(vec) => vec.remove_last(index), Bitmap(bits) => bits.remove_last(index), From f4080d9852236c6b313e4d91cea6f2a87e5644fa Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Tue, 11 Jul 2023 00:05:43 +0900 Subject: [PATCH 35/44] Rename remove_first to remove_front and remove_last to remove_back --- src/bitmap/container.rs | 12 ++--- src/bitmap/inherent.rs | 68 ++++++++++++++--------------- src/bitmap/store/array_store/mod.rs | 12 ++--- src/bitmap/store/bitmap_store.rs | 8 ++-- src/bitmap/store/mod.rs | 12 ++--- 5 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index 7ae5b1ac..298b9083 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -94,7 +94,7 @@ impl Container { result } - pub fn remove_first(&mut self, n: u64) { + pub fn remove_front(&mut self, n: u64) { match &self.store { Store::Bitmap(bits) => { if bits.len() - n <= ARRAY_LIMIT { @@ -102,14 +102,14 @@ impl Container { replace_array.extend(bits.clone().into_iter().skip(n as usize)); self.store = Store::Array(store::ArrayStore::from_vec_unchecked(replace_array)); } else { - self.store.remove_first(n) + self.store.remove_front(n) } } - Store::Array(_) => self.store.remove_first(n), + Store::Array(_) => self.store.remove_front(n), }; } - pub fn remove_last(&mut self, n: u64) { + pub fn remove_back(&mut self, n: u64) { match &self.store { Store::Bitmap(bits) => { if bits.len() - n <= ARRAY_LIMIT { @@ -118,10 +118,10 @@ impl Container { replace_array.truncate(replace_array.len() - n as usize); self.store = Store::Array(store::ArrayStore::from_vec_unchecked(replace_array)); } else { - self.store.remove_last(n) + self.store.remove_back(n) } } - Store::Array(_) => self.store.remove_last(n), + Store::Array(_) => self.store.remove_back(n), }; } diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index e29d6cd8..64f4195c 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -587,13 +587,13 @@ impl RoaringBitmap { /// use roaring::RoaringBitmap; /// /// let mut rb = RoaringBitmap::from_iter([1, 5, 7, 9]); - /// rb.remove_first(2); + /// rb.remove_front(2); /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); /// /// let mut rb = RoaringBitmap::from_iter([1, 3, 7, 9]); - /// rb.remove_first(2); + /// rb.remove_front(2); /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); - pub fn remove_first(&mut self, mut n: u64) { + pub fn remove_front(&mut self, mut n: u64) { if n > self.len() { return; } @@ -616,7 +616,7 @@ impl RoaringBitmap { // remove data in containers if there are still targets for deletion if n > 0 { // container immediately before should have been deleted, so the target is 0 index - self.containers[0].remove_first(n); + self.containers[0].remove_front(n); } } @@ -628,11 +628,11 @@ impl RoaringBitmap { /// use roaring::RoaringBitmap; /// /// let mut rb = RoaringBitmap::from_iter([1, 5, 7, 9]); - /// rb.remove_last(2); + /// rb.remove_back(2); /// assert_eq!(rb, RoaringBitmap::from_iter([1, 5])); - /// rb.remove_last(1); + /// rb.remove_back(1); /// assert_eq!(rb, RoaringBitmap::from_iter([1])); - pub fn remove_last(&mut self, mut n: u64) { + pub fn remove_back(&mut self, mut n: u64) { if n > self.len() { return; } @@ -650,7 +650,7 @@ impl RoaringBitmap { let position = position.expect("there are no containers to delete"); self.containers.truncate(position + 1); if n > 0 { - self.containers[position].remove_last(n); + self.containers[position].remove_back(n); } } } @@ -805,59 +805,59 @@ mod tests { } #[test] - fn remove_first_for_vec() { + fn remove_front_for_vec() { let mut bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); - bitmap.remove_first(3); + bitmap.remove_front(3); assert_eq!(bitmap.len(), 3); assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); bitmap = RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11]); - bitmap.remove_first(3); + bitmap.remove_front(3); assert_eq!(bitmap.len(), 3); assert_eq!(bitmap, RoaringBitmap::from_iter([7, 9, 11])); bitmap = RoaringBitmap::from_iter([1, 3]); - bitmap.remove_first(2); + bitmap.remove_front(2); assert_eq!(bitmap.len(), 0); bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); - bitmap.remove_first(0); + bitmap.remove_front(0); assert_eq!(bitmap.len(), 6); assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11])); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..(1_u32 << 16) + 5); - bitmap.remove_first(65537); + bitmap.remove_front(65537); assert_eq!(bitmap.len(), 4); assert_eq!(bitmap, RoaringBitmap::from_iter([65537, 65538, 65539, 65540])); bitmap = RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11]); - bitmap.remove_first(7); + bitmap.remove_front(7); assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11])); } #[test] - fn remove_first_for_bit() { + fn remove_front_for_bit() { let mut bitmap = RoaringBitmap::new(); bitmap.insert_range(0..4098); - bitmap.remove_first(4095); + bitmap.remove_front(4095); assert_eq!(bitmap.len(), 3); // removed bit to vec assert_eq!(bitmap, RoaringBitmap::from_iter([4095, 4096, 4097])); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..6000); - bitmap.remove_first(999); + bitmap.remove_front(999); assert_eq!(bitmap.len(), 5001); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..8000); - bitmap.remove_first(10); + bitmap.remove_front(10); assert_eq!(bitmap.len(), 7990); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..200000); - bitmap.remove_first(2000); + bitmap.remove_front(2000); assert_eq!(bitmap.len(), 198000); assert_eq!(bitmap, RoaringBitmap::from_iter(2000..200000)); @@ -865,67 +865,67 @@ mod tests { bitmap.insert_range(0..2); bitmap.insert_range(4..7); bitmap.insert_range(1000..6000); - bitmap.remove_first(30); + bitmap.remove_front(30); assert_eq!(bitmap.len(), 4975); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..65535); - bitmap.remove_first(0); + bitmap.remove_front(0); assert_eq!(bitmap.len(), 65535); } #[test] - fn remove_last_for_bit() { + fn remove_back_for_bit() { let mut bitmap = RoaringBitmap::new(); bitmap.insert_range(0..5000); - bitmap.remove_last(1000); + bitmap.remove_back(1000); assert_eq!(bitmap.len(), 4000); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..6000); - bitmap.remove_last(1000); + bitmap.remove_back(1000); assert_eq!(bitmap.len(), 5000); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..200000); - bitmap.remove_last(196000); + bitmap.remove_back(196000); assert_eq!(bitmap.len(), 4000); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..200000); - bitmap.remove_last(2000); + bitmap.remove_back(2000); assert_eq!(bitmap.len(), 198000); assert_eq!(bitmap, RoaringBitmap::from_iter(0..198000)); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..65535); - bitmap.remove_last(0); + bitmap.remove_back(0); assert_eq!(bitmap.len(), 65535); } #[test] - fn remove_last_for_vec() { + fn remove_back_for_vec() { let mut bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); - bitmap.remove_last(2); + bitmap.remove_back(2); assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7])); bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); - bitmap.remove_last(6); + bitmap.remove_back(6); assert_eq!(bitmap.len(), 0); bitmap = RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11]); - bitmap.remove_last(0); + bitmap.remove_back(0); assert_eq!(bitmap.len(), 6); assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3, 7, 9, 11])); bitmap = RoaringBitmap::new(); bitmap.insert_range(0..(1_u32 << 16) + 5); - bitmap.remove_last(65537); + bitmap.remove_back(65537); assert_eq!(bitmap.len(), 4); assert_eq!(bitmap, RoaringBitmap::from_iter([0, 1, 2, 3])); let mut bitmap = RoaringBitmap::from_iter([1, 2, 3]); - bitmap.remove_last(4); + bitmap.remove_back(4); assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3])); } } diff --git a/src/bitmap/store/array_store/mod.rs b/src/bitmap/store/array_store/mod.rs index 0fd4a848..5895eb37 100644 --- a/src/bitmap/store/array_store/mod.rs +++ b/src/bitmap/store/array_store/mod.rs @@ -110,12 +110,12 @@ impl ArrayStore { (pos_end - pos_start) as u64 } - pub fn remove_first(&mut self, n: u64) { + pub fn remove_front(&mut self, n: u64) { self.vec.rotate_left(n as usize); self.vec.truncate(self.vec.len() - n as usize); } - pub fn remove_last(&mut self, n: u64) { + pub fn remove_back(&mut self, n: u64) { self.vec.truncate(self.vec.len() - n as usize); } @@ -573,16 +573,16 @@ mod tests { } #[test] - fn test_bitmap_remove_first() { + fn test_bitmap_remove_front() { let mut store = Store::Array(ArrayStore::from_vec_unchecked(vec![1, 2, 130, 500])); - store.remove_first(3); + store.remove_front(3); assert_eq!(into_vec(store), vec![500]); } #[test] - fn test_bitmap_remove_last() { + fn test_bitmap_remove_back() { let mut store = Store::Array(ArrayStore::from_vec_unchecked(vec![1, 2, 130, 500])); - store.remove_last(2); + store.remove_back(2); assert_eq!(into_vec(store), vec![1, 2]); } } diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 10708d11..3b6eb0c2 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -304,7 +304,7 @@ impl BitmapStore { } /// Set N bits that are currently 1 bit from the lower bit to 0. - pub fn remove_first(&mut self, mut clear_bits: u64) { + pub fn remove_front(&mut self, mut clear_bits: u64) { if self.len() < clear_bits { *self = Self::default(); return; @@ -335,7 +335,7 @@ impl BitmapStore { } /// Set N bits that are currently 1 bit from the lower bit to 0. - pub fn remove_last(&mut self, mut clear_bits: u64) { + pub fn remove_back(&mut self, mut clear_bits: u64) { if self.len() < clear_bits { *self = Self::default(); return; @@ -559,14 +559,14 @@ mod tests { use super::*; #[test] - fn test_bitmap_remove_first() { + fn test_bitmap_remove_front() { let mut store = BitmapStore::new(); let range = RangeInclusive::new(1, 3); store.insert_range(range); let range_second = RangeInclusive::new(5, 65535); // store.bits[0] = 0b1111111111111111111111111111111111111111111111111111111111101110 store.insert_range(range_second); - store.remove_first(2); + store.remove_front(2); assert_eq!( store.bits[0], 0b1111111111111111111111111111111111111111111111111111111111101000 diff --git a/src/bitmap/store/mod.rs b/src/bitmap/store/mod.rs index 47deb261..0b8017e6 100644 --- a/src/bitmap/store/mod.rs +++ b/src/bitmap/store/mod.rs @@ -106,17 +106,17 @@ impl Store { } } - pub fn remove_first(&mut self, index: u64) { + pub fn remove_front(&mut self, index: u64) { match self { - Array(vec) => vec.remove_first(index), - Bitmap(bits) => bits.remove_first(index), + Array(vec) => vec.remove_front(index), + Bitmap(bits) => bits.remove_front(index), } } - pub fn remove_last(&mut self, index: u64) { + pub fn remove_back(&mut self, index: u64) { match self { - Array(vec) => vec.remove_last(index), - Bitmap(bits) => bits.remove_last(index), + Array(vec) => vec.remove_back(index), + Bitmap(bits) => bits.remove_back(index), } } From e690f2ac514de2288619cdf5df4012b9c3a81139 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Fri, 14 Jul 2023 01:19:30 +0900 Subject: [PATCH 36/44] Fix incorrect judgment condition. --- src/bitmap/inherent.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 64f4195c..9ceea831 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -600,7 +600,7 @@ impl RoaringBitmap { // remove containers up to the front of the target let position = self.containers.iter().position(|container| { let container_len = container.len(); - if container_len < n { + if container_len <= n { n -= container_len; false } else { @@ -639,7 +639,7 @@ impl RoaringBitmap { // remove containers up to the back of the target let position = self.containers.iter().rposition(|container| { let container_len = container.len(); - if container_len < n { + if container_len <= n { n -= container_len; false } else { From 5761b1c9bc2e648be5a5d425242d808d5b266984 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Fri, 14 Jul 2023 01:21:39 +0900 Subject: [PATCH 37/44] Fix use the Vec::drain method --- src/bitmap/inherent.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 9ceea831..7b94d18b 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -610,8 +610,7 @@ impl RoaringBitmap { // It is checked at the beginning of the function, so it is usually never an Err let position = position.expect("there are no containers to delete"); if position > 0 { - self.containers.rotate_left(position); - self.containers.truncate(position); + self.containers.drain(..position); } // remove data in containers if there are still targets for deletion if n > 0 { @@ -651,6 +650,7 @@ impl RoaringBitmap { self.containers.truncate(position + 1); if n > 0 { self.containers[position].remove_back(n); + self.containers.drain(position + 1..); } } } From 44c5a1926d59b1faa0dcc7822bce9997d7befecc Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Fri, 14 Jul 2023 01:23:57 +0900 Subject: [PATCH 38/44] Fix the panics and removed unnecessary comments. --- src/bitmap/inherent.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index 7b94d18b..d21ad922 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -607,8 +607,7 @@ impl RoaringBitmap { true } }); - // It is checked at the beginning of the function, so it is usually never an Err - let position = position.expect("there are no containers to delete"); + let position = position.unwrap_or(self.containers.len()); if position > 0 { self.containers.drain(..position); } From 6b54277b03167342e1eb99d7386dbcade19f0bc4 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Fri, 14 Jul 2023 01:26:23 +0900 Subject: [PATCH 39/44] Fix to delete all cases when "n" exceeds the container size --- src/bitmap/inherent.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index d21ad922..2670609c 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -594,9 +594,6 @@ impl RoaringBitmap { /// rb.remove_front(2); /// assert_eq!(rb, RoaringBitmap::from_iter([7, 9])); pub fn remove_front(&mut self, mut n: u64) { - if n > self.len() { - return; - } // remove containers up to the front of the target let position = self.containers.iter().position(|container| { let container_len = container.len(); @@ -612,7 +609,7 @@ impl RoaringBitmap { self.containers.drain(..position); } // remove data in containers if there are still targets for deletion - if n > 0 { + if n > 0 && !self.containers.is_empty() { // container immediately before should have been deleted, so the target is 0 index self.containers[0].remove_front(n); } @@ -631,9 +628,6 @@ impl RoaringBitmap { /// rb.remove_back(1); /// assert_eq!(rb, RoaringBitmap::from_iter([1])); pub fn remove_back(&mut self, mut n: u64) { - if n > self.len() { - return; - } // remove containers up to the back of the target let position = self.containers.iter().rposition(|container| { let container_len = container.len(); @@ -645,11 +639,13 @@ impl RoaringBitmap { } }); // It is checked at the beginning of the function, so it is usually never an Err - let position = position.expect("there are no containers to delete"); - self.containers.truncate(position + 1); - if n > 0 { - self.containers[position].remove_back(n); + if let Some(position) = position { self.containers.drain(position + 1..); + if n > 0 && !self.containers.is_empty() { + self.containers[position].remove_back(n); + } + } else { + self.containers.clear(); } } } @@ -832,7 +828,7 @@ mod tests { bitmap = RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11]); bitmap.remove_front(7); - assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 5, 7, 9, 11])); + assert_eq!(bitmap, RoaringBitmap::default()); } #[test] @@ -925,6 +921,6 @@ mod tests { let mut bitmap = RoaringBitmap::from_iter([1, 2, 3]); bitmap.remove_back(4); - assert_eq!(bitmap, RoaringBitmap::from_iter([1, 2, 3])); + assert_eq!(bitmap, RoaringBitmap::default()); } } From 521f5a7a908195177cb7dcabaae277ef58263662 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Fri, 14 Jul 2023 01:28:34 +0900 Subject: [PATCH 40/44] Remove unwanted clone --- src/bitmap/container.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index 298b9083..02eace23 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -99,7 +99,7 @@ impl Container { Store::Bitmap(bits) => { if bits.len() - n <= ARRAY_LIMIT { let mut replace_array = Vec::with_capacity((bits.len() - n) as usize); - replace_array.extend(bits.clone().into_iter().skip(n as usize)); + replace_array.extend(bits.iter().skip(n as usize)); self.store = Store::Array(store::ArrayStore::from_vec_unchecked(replace_array)); } else { self.store.remove_front(n) From d0e7322a07bf3d68369cd6bcf5a5ace62f7711b1 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Fri, 14 Jul 2023 01:32:44 +0900 Subject: [PATCH 41/44] Fix unwanted clone --- src/bitmap/container.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index 02eace23..dee6ee1f 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -114,8 +114,7 @@ impl Container { Store::Bitmap(bits) => { if bits.len() - n <= ARRAY_LIMIT { let mut replace_array = Vec::with_capacity((bits.len() - n) as usize); - replace_array.extend(bits.clone().into_iter()); - replace_array.truncate(replace_array.len() - n as usize); + replace_array.extend(bits.iter().take((bits.len() - n) as usize)); self.store = Store::Array(store::ArrayStore::from_vec_unchecked(replace_array)); } else { self.store.remove_back(n) From deaa66f32719c0750c795d8ef287cca6f9dd5eec Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Fri, 14 Jul 2023 01:37:16 +0900 Subject: [PATCH 42/44] Add clear method to avoid new allocations --- src/bitmap/store/bitmap_store.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 3b6eb0c2..b50f186e 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -303,10 +303,15 @@ impl BitmapStore { &self.bits } + pub fn clear(&mut self) { + self.bits.fill(0); + self.len = 0; + } + /// Set N bits that are currently 1 bit from the lower bit to 0. pub fn remove_front(&mut self, mut clear_bits: u64) { if self.len() < clear_bits { - *self = Self::default(); + self.clear(); return; } self.len -= clear_bits as u64; @@ -337,7 +342,7 @@ impl BitmapStore { /// Set N bits that are currently 1 bit from the lower bit to 0. pub fn remove_back(&mut self, mut clear_bits: u64) { if self.len() < clear_bits { - *self = Self::default(); + self.clear(); return; } self.len -= clear_bits; From f7a18aa5f3f44e80fc38291b2620654d0d3a9411 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Fri, 14 Jul 2023 01:41:19 +0900 Subject: [PATCH 43/44] Fix clippy warnings --- src/bitmap/store/bitmap_store.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index b50f186e..e0f287bc 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -314,7 +314,7 @@ impl BitmapStore { self.clear(); return; } - self.len -= clear_bits as u64; + self.len -= clear_bits; let min = self.min().unwrap(); let key = key(min); for index in key..BITMAP_LENGTH { @@ -350,7 +350,7 @@ impl BitmapStore { let key = key(max); for index in (0..=key).rev() { let bit = self.bits[index].count_ones(); - let mut mask = 1 << bit - 1; + let mut mask = 1 << (bit - 1); let mut count = self.bits[index].count_ones() as u64; if clear_bits > count { self.bits[index] = 0; From d63dd699c90cd0b75dff1ce083eef8a812a89728 Mon Sep 17 00:00:00 2001 From: shimatar0 Date: Fri, 14 Jul 2023 01:58:18 +0900 Subject: [PATCH 44/44] Refactor to clearly define the method to clear the bit --- src/bitmap/store/bitmap_store.rs | 76 ++++++++++++++++---------------- 1 file changed, 37 insertions(+), 39 deletions(-) diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index e0f287bc..68366643 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -315,26 +315,18 @@ impl BitmapStore { return; } self.len -= clear_bits; - let min = self.min().unwrap(); - let key = key(min); - for index in key..BITMAP_LENGTH { - let mut mask = 1; - let mut count = self.bits[index].count_ones() as u64; - if clear_bits > count { - self.bits[index] = 0; - clear_bits -= count; - continue; - } - while count > 0 { - if self.bits[index] & mask == mask { - self.bits[index] &= !mask; - clear_bits -= 1; - count -= 1; - if clear_bits == 0 { - return; - } + for word in self.bits.iter_mut() { + let count = word.count_ones() as u64; + if clear_bits < count { + for _ in 0..clear_bits { + *word = *word & (*word - 1); } - mask <<= 1; + return; + } + *word = 0; + clear_bits -= count; + if clear_bits == 0 { + return; } } } @@ -346,27 +338,18 @@ impl BitmapStore { return; } self.len -= clear_bits; - let max = self.max().unwrap(); - let key = key(max); - for index in (0..=key).rev() { - let bit = self.bits[index].count_ones(); - let mut mask = 1 << (bit - 1); - let mut count = self.bits[index].count_ones() as u64; - if clear_bits > count { - self.bits[index] = 0; - clear_bits -= count; - continue; - } - while count > 0 { - if self.bits[index] & mask == mask { - self.bits[index] &= !mask; - clear_bits -= 1; - count -= 1; - if clear_bits == 0 { - return; - } + for word in self.bits.iter_mut().rev() { + let count = word.count_ones() as u64; + if clear_bits < count { + for _ in 0..clear_bits { + *word &= !(1 << (63 - word.leading_zeros())); } - mask >>= 1; + return; + } + *word = 0; + clear_bits -= count; + if clear_bits == 0 { + return; } } } @@ -577,4 +560,19 @@ mod tests { 0b1111111111111111111111111111111111111111111111111111111111101000 ); } + + #[test] + fn test_bitmap_remove_back() { + let mut store = BitmapStore::new(); + let range = RangeInclusive::new(1, 3); + store.insert_range(range); + let range_second = RangeInclusive::new(5, 65535); + // store.bits[1023] = 0b1111111111111111111111111111111111111111111111111111111111111111 + store.insert_range(range_second); + store.remove_back(2); + assert_eq!( + store.bits[1023], + 0b11111111111111111111111111111111111111111111111111111111111111 + ); + } }