Merge #83

83: Use Vec::retain instead of Vec::remove r=Kerollmops a=Kerollmops

There are many places where we use [the `Vec::remove` method](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.remove). It can bring a lot of performances issues as every time this function is called, the right part of the Vec (after the removed element) is shifted to the left by one, when a lot of element must be removed, we should always use [`Vec::retain`](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.retain) which is more optimized for this in the sense that it will avoid calling a lot the copy function for when multiple elements must be removed at the same time.

I found that doing this change in the `Store::intersect_with` method for the Array-Array operation gave me much better performances (7m intersected with 3m and intersected with 4m again), moving from 240ms to 55ms. I am sure that we can optimize the current implementation more.

- [x] Stores intersections.
- [x] Stores differences.
- [ ] Stores symmetric differences.

Co-authored-by: Clément Renault <[email protected]>
Co-authored-by: Kerollmops <[email protected]>

src/bitmap/cmp.rs

@@ -118,7 +118,7 @@ impl<'a> Iterator for Pairs<'a> {
             Right,
             Both,
             None,
-        };
+        }
         let which = match (self.0.peek(), self.1.peek()) {
             (None, None) => Which::None,
             (Some(_), None) => Which::Left,

src/bitmap/store.rs

@@ -323,35 +323,23 @@ impl Store {
     pub fn intersect_with(&mut self, other: &Self) {
         match (self, other) {
             (&mut Array(ref mut vec1), &Array(ref vec2)) => {
-                let mut i1 = 0usize;
-                let mut iter2 = vec2.iter();
-                let mut current2 = iter2.next();
-                while i1 < vec1.len() {
-                    match current2.map(|c2| vec1[i1].cmp(c2)) {
-                        None | Some(Less) => {
-                            vec1.remove(i1);
-                        }
-                        Some(Greater) => {
-                            current2 = iter2.next();
-                        }
-                        Some(Equal) => {
-                            i1 += 1;
-                            current2 = iter2.next();
-                        }
-                    }
-                }
+                let mut i = 0;
+                vec1.retain(|x| {
+                    i += vec2
+                        .iter()
+                        .skip(i)
+                        .position(|y| y >= x)
+                        .unwrap_or(vec2.len());
+                    vec2.get(i).map_or(false, |y| x == y)
+                });
             }
             (&mut Bitmap(ref mut bits1), &Bitmap(ref bits2)) => {
                 for (index1, &index2) in bits1.iter_mut().zip(bits2.iter()) {
                     *index1 &= index2;
                 }
             }
             (&mut Array(ref mut vec), store @ &Bitmap(..)) => {
-                for i in (0..(vec.len())).rev() {
-                    if !store.contains(vec[i]) {
-                        vec.remove(i);
-                    }
-                }
+                vec.retain(|x| store.contains(*x));
             }
             (this @ &mut Bitmap(..), &Array(..)) => {
                 let mut new = other.clone();
@@ -364,24 +352,15 @@ impl Store {
     pub fn difference_with(&mut self, other: &Self) {
         match (self, other) {
             (&mut Array(ref mut vec1), &Array(ref vec2)) => {
-                let mut i1 = 0usize;
-                let mut iter2 = vec2.iter();
-                let mut current2 = iter2.next();
-                while i1 < vec1.len() {
-                    match current2.map(|c2| vec1[i1].cmp(c2)) {
-                        None => break,
-                        Some(Less) => {
-                            i1 += 1;
-                        }
-                        Some(Greater) => {
-                            current2 = iter2.next();
-                        }
-                        Some(Equal) => {
-                            vec1.remove(i1);
-                            current2 = iter2.next();
-                        }
-                    }
-                }
+                let mut i = 0;
+                vec1.retain(|x| {
+                    i += vec2
+                        .iter()
+                        .skip(i)
+                        .position(|y| y >= x)
+                        .unwrap_or(vec2.len());
+                    vec2.get(i).map_or(true, |y| x != y)
+                });
             }
             (ref mut this @ &mut Bitmap(..), &Array(ref vec2)) => {
                 for index in vec2.iter() {
@@ -394,11 +373,7 @@ impl Store {
                 }
             }
             (&mut Array(ref mut vec), store @ &Bitmap(..)) => {
-                for i in (0..vec.len()).rev() {
-                    if store.contains(vec[i]) {
-                        vec.remove(i);
-                    }
-                }
+                vec.retain(|x| !store.contains(*x));
             }
         }
     }

src/treemap/cmp.rs

@@ -115,7 +115,7 @@ impl<'a> Iterator for Pairs<'a> {
             Right,
             Both,
             None,
-        };
+        }
         let which = match (self.0.peek(), self.1.peek()) {
             (None, None) => Which::None,
             (Some(_), None) => Which::Left,