Fix Bug #677 - Optimize removeIf() for UnifiedMap. (#786)

Fix Bug #677 - Optimize removeIf() for UnifiedMap.
eclipse · Jan 15, 2020 · 7ce7a19 · 7ce7a19
2 parents 9084111 + 1596cfe
commit 7ce7a19
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 7 deletions.
diff --git a/RELEASE_NOTE_DRAFT.md b/RELEASE_NOTE_DRAFT.md
@@ -21,6 +21,7 @@ Central theme of this release is to fix issues identified for SimRel repository.
 * Fixed generated Eclipse features for p2 repository to ensure correct EPLv1 license is downloaded.
 * Fixed generated Eclipse features for p2 repository to ensure correct signatures on artifacts.
 * Fixed repository path for Eclipse features.
+* Optimize removeIf() for UnifiedMap.
 
 # Build Changes
 -----------------

diff --git a/eclipse-collections/src/main/java/org/eclipse/collections/impl/map/mutable/UnifiedMap.java b/eclipse-collections/src/main/java/org/eclipse/collections/impl/map/mutable/UnifiedMap.java
@@ -1038,17 +1038,48 @@ public V removeKey(K key)
     @Override
     public boolean removeIf(Predicate2<? super K, ? super V> predicate)
     {
-        int previousSize = this.size();
-        Iterator<Entry<K, V>> iterator = this.entrySet().iterator();
-        while (iterator.hasNext())
+        int previousOccupied = this.occupied;
+        for (int index = 0; index < this.table.length; index += 2)
         {
-            Entry<K, V> entry = iterator.next();
-            if (predicate.accept(entry.getKey(), entry.getValue()))
+            Object cur = this.table[index];
+            if (cur == null)
+            {
+                continue;
+            }
+            if (cur == CHAINED_KEY)
+            {
+                Object[] chain = (Object[]) this.table[index + 1];
+                for (int chIndex = 0; chIndex < chain.length; )
+                {
+                    if (chain[chIndex] == null)
+                    {
+                        break;
+                    }
+                    K key = this.nonSentinel(chain[chIndex]);
+                    V value = (V) chain[chIndex + 1];
+                    if (predicate.accept(key, value))
+                    {
+                        this.overwriteWithLastElementFromChain(chain, index, chIndex);
+                    }
+                    else
+                    {
+                        chIndex += 2;
+                    }
+                }
+            }
+            else
             {
-                iterator.remove();
+                K key = this.nonSentinel(cur);
+                V value = (V) this.table[index + 1];
+                if (predicate.accept(key, value))
+                {
+                    this.table[index] = null;
+                    this.table[index + 1] = null;
+                    this.occupied--;
+                }
             }
         }
-        return previousSize > this.size();
+        return previousOccupied > this.occupied;
     }
 
     private void chainedForEachEntry(Object[] chain, Procedure2<? super K, ? super V> procedure)

diff --git a/...a8/src/test/java/org/eclipse/collections/test/map/mutable/MutableMapIterableTestCase.java b/...a8/src/test/java/org/eclipse/collections/test/map/mutable/MutableMapIterableTestCase.java
@@ -21,6 +21,7 @@
 import org.eclipse.collections.impl.factory.Bags;
 import org.eclipse.collections.impl.list.Interval;
 import org.eclipse.collections.impl.tuple.ImmutableEntry;
+import org.eclipse.collections.test.CollisionsTestCase;
 import org.eclipse.collections.test.map.MapIterableTestCase;
 import org.junit.Test;
 
@@ -147,6 +148,18 @@ default void MutableMapIterable_removeIf()
         assertTrue(map2.removeIf((each, value) -> each % 2 != 0));
         assertFalse(map2.removeIf((each, value) -> each % 2 != 0));
         assertEquals(this.newWithKeysValues(4, "four", 8, "Eight"), map2);
+
+        MutableMapIterable<Integer, String> map3 = this.newWithKeysValues(CollisionsTestCase.COLLISION_1, "0", CollisionsTestCase.COLLISION_2, "17", CollisionsTestCase.COLLISION_3, "34", 100, "100");
+        assertTrue(map3.removeIf((key, value) -> CollisionsTestCase.COLLISION_1.equals(key) || CollisionsTestCase.COLLISION_2.equals(key) || CollisionsTestCase.COLLISION_3.equals(key)));
+        assertEquals(this.newWithKeysValues(100, "100"), map3);
+
+        MutableMapIterable<Integer, String> map4 = this.newWithKeysValues(CollisionsTestCase.COLLISION_1, "0", CollisionsTestCase.COLLISION_2, "17", CollisionsTestCase.COLLISION_3, "34", 100, "100");
+        assertTrue(map4.removeIf(Predicates2.alwaysTrue()));
+        assertEquals(this.newWithKeysValues(), map4);
+
+        MutableMapIterable<Integer, String> map5 = this.newWithKeysValues(CollisionsTestCase.COLLISION_1, "0", CollisionsTestCase.COLLISION_2, "17", CollisionsTestCase.COLLISION_3, "34", 100, "100");
+        assertTrue(map5.removeIf((key, value) -> CollisionsTestCase.COLLISION_1.equals(key) || CollisionsTestCase.COLLISION_3.equals(key)));
+        assertEquals(this.newWithKeysValues(CollisionsTestCase.COLLISION_2, "17", 100, "100"), map5);
     }
 
     @Test