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

Paul Sandoz paul.sandoz at oracle.com
Tue May 5 09:12:33 UTC 2015

• Previous message: RFR 8078645: removeIf(filter) in ConcurrentHashMap removes entries for which filter is false
• Next message: RFR 8078645: removeIf(filter) in ConcurrentHashMap removes entries for which filter is false
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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 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 descendingSet() { return new KeySet(m.descendingMap()); }

•    @SuppressWarnings("unchecked")

+// @SuppressWarnings("unchecked") public Spliterator 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 iterator() { if (m instanceof ConcurrentSkipListMap) return ((ConcurrentSkipListMap<?,E>)m).valueIterator(); @@ -2486,12 +2486,12 @@ } public Object[] toArray() { return toList(this).toArray(); } public T[] toArray(T[] a) { return toList(this).toArray(a); }

•    @SuppressWarnings("unchecked")

+// @SuppressWarnings("unchecked") public Spliterator 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[] 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

• Previous message: RFR 8078645: removeIf(filter) in ConcurrentHashMap removes entries for which filter is false
• Next message: RFR 8078645: removeIf(filter) in ConcurrentHashMap removes entries for which filter is false
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

More information about the core-libs-dev mailing list