RFR 8078645: removeIf(filter) in ConcurrentHashMap removes entries for which filter is false

[Paul Sandoz]

On May 5, 2015, at 7:54 AM, Martin Buchholz <martinrb at google.com> wrote:
> 
> One query in ConcurrentSkipListMap, we have:
> 
> 2500             // else use iterator
> 2501             @SuppressWarnings("unchecked") Iterator<Map.Entry<Object,E>> it =
> 2502                     ((SubMap<Object,E>)m).entryIterator();
> 
> and then
> 
> 2578             // else use iterator
> 2579             Iterator<Map.Entry<K1,V1>> it = ((SubMap<K1,V1>)m).entryIterator();
> 
> why does only the former require the "unchecked" warning suppression?
> 
> Good question.

Yes, for values the key type parameter is "thrown" away and reconstituting as Object is not type safe.


>   I think it's a small but clear improvement to consistently use K,V on view subclasses, allowing a few @SuppressWarnings to be removed:
> 

AFAICT all but the above related suppress warning annotations could be removed if spliterator returning methods were on SubMap, rather than casting the result of the iterator based methods (see end of email for patch). I dunno if the 9 compiler got smarter or code was updated and the annotation was not removed.

Your patch (plus addition of SubMap spliterator returning methods) seems like a good change to bring in bulk-wise, or otherwise earlier on if fixing another bug.

Paul.

diff -r b029e73f9cbb src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java
--- a/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java	Tue May 05 09:43:27 2015 +0200
+++ b/src/java.base/share/classes/java/util/concurrent/ConcurrentSkipListMap.java	Tue May 05 11:10:41 2015 +0200
@@ -2400,12 +2400,12 @@
             Map.Entry<E,?> e = m.pollLastEntry();
             return (e == null) ? null : e.getKey();
         }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Iterator<E> iterator() {
             if (m instanceof ConcurrentSkipListMap)
-                return ((ConcurrentSkipListMap<E,Object>)m).keyIterator();
+                return ((ConcurrentSkipListMap<E,>)m).keyIterator();
             else
-                return ((ConcurrentSkipListMap.SubMap<E,Object>)m).keyIterator();
+                return ((ConcurrentSkipListMap.SubMap<E,?>)m).keyIterator();
         }
         public boolean equals(Object o) {
             if (o == this)
@@ -2451,12 +2451,12 @@
         public NavigableSet<E> descendingSet() {
             return new KeySet<E>(m.descendingMap());
         }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Spliterator<E> spliterator() {
             if (m instanceof ConcurrentSkipListMap)
                 return ((ConcurrentSkipListMap<E,?>)m).keySpliterator();
             else
-                return (Spliterator<E>)((SubMap<E,?>)m).keyIterator();
+                return ((SubMap<E,?>)m).keySpliterator();
         }
     }
 
@@ -2465,7 +2465,7 @@
         Values(ConcurrentNavigableMap<?, E> map) {
             m = map;
         }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Iterator<E> iterator() {
             if (m instanceof ConcurrentSkipListMap)
                 return ((ConcurrentSkipListMap<?,E>)m).valueIterator();
@@ -2486,12 +2486,12 @@
         }
         public Object[] toArray()     { return toList(this).toArray();  }
         public <T> T[] toArray(T[] a) { return toList(this).toArray(a); }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Spliterator<E> spliterator() {
             if (m instanceof ConcurrentSkipListMap)
                 return ((ConcurrentSkipListMap<?,E>)m).valueSpliterator();
             else
-                return (Spliterator<E>)((SubMap<?,E>)m).valueIterator();
+                return ((SubMap<?,E>)m).valueSpliterator();
         }
         public boolean removeIf(Predicate<? super E> filter) {
             if (filter == null) throw new NullPointerException();
@@ -2516,7 +2516,7 @@
         EntrySet(ConcurrentNavigableMap<K1, V1> map) {
             m = map;
         }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Iterator<Map.Entry<K1,V1>> iterator() {
             if (m instanceof ConcurrentSkipListMap)
                 return ((ConcurrentSkipListMap<K1,V1>)m).entryIterator();
@@ -2563,13 +2563,12 @@
         }
         public Object[] toArray()     { return toList(this).toArray();  }
         public <T> T[] toArray(T[] a) { return toList(this).toArray(a); }
-        @SuppressWarnings("unchecked")
+//        @SuppressWarnings("unchecked")
         public Spliterator<Map.Entry<K1,V1>> spliterator() {
             if (m instanceof ConcurrentSkipListMap)
                 return ((ConcurrentSkipListMap<K1,V1>)m).entrySpliterator();
             else
-                return (Spliterator<Map.Entry<K1,V1>>)
-                    ((SubMap<K1,V1>)m).entryIterator();
+                return ((SubMap<K1,V1>)m).entrySpliterator();
         }
         public boolean removeIf(Predicate<? super Entry<K1, V1>> filter) {
             if (filter == null) throw new NullPointerException();
@@ -3112,14 +3111,26 @@
             return new SubMapKeyIterator();
         }
 
+        Spliterator<K> keySpliterator() {
+            return new SubMapKeyIterator();
+        }
+
         Iterator<V> valueIterator() {
             return new SubMapValueIterator();
         }
 
+        Spliterator<V> valueSpliterator() {
+            return new SubMapValueIterator();
+        }
+
         Iterator<Map.Entry<K,V>> entryIterator() {
             return new SubMapEntryIterator();
         }
 
+        Spliterator<Entry<K,V>> entrySpliterator() {
+            return new SubMapEntryIterator();
+        }
+
         /**
          * Variant of main Iter class to traverse through submaps.
          * Also serves as back-up Spliterator for views