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

Martin Buchholz martinrb at google.com
Tue May 5 17:39:22 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 ]

I'd prefer to go the other way, deleting those trivial methods entirely, utilizing the rarely used .new syntax.

Index: ConcurrentSkipListMap.java

RCS file: /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/ConcurrentSkipListMap.java,v retrieving revision 1.147 diff -u -r1.147 ConcurrentSkipListMap.java --- ConcurrentSkipListMap.java 5 May 2015 16:36:32 -0000 1.147 +++ ConcurrentSkipListMap.java 5 May 2015 17:34:53 -0000 @@ -2311,20 +2311,6 @@ } }

• // Factory methods for iterators needed by ConcurrentSkipListSet etc
• Iterator keyIterator() {

•    return new KeyIterator();

• }
• Iterator valueIterator() {

•    return new ValueIterator();

• }
• Iterator<Map.Entry<K,V>> entryIterator() {

•    return new EntryIterator();

• }

• /* ---------------- View Classes -------------- */
  
  /*

@@ -2367,9 +2353,9 @@ } public Iterator iterator() { if (m instanceof ConcurrentSkipListMap)

•            return ((ConcurrentSkipListMap<K,V>)m).keyIterator();

•            return ((ConcurrentSkipListMap<K,V>)m).new KeyIterator();
         else

•            return

((ConcurrentSkipListMap.SubMap<K,V>)m).keyIterator();

•            return ((SubMap<K,V>)m).new SubMapKeyIterator();
     }
     public boolean equals(Object o) {
         if (o == this)

@@ -2415,12 +2401,12 @@ public NavigableSet descendingSet() { return new KeySet<>(m.descendingMap()); }

•    @SuppressWarnings("unchecked")

•     public Spliterator<K> spliterator() {
          if (m instanceof ConcurrentSkipListMap)
              return ((ConcurrentSkipListMap<K,V>)m).keySpliterator();
          else

•            return (Spliterator<K>)((SubMap<K,V>)m).keyIterator();

•            return ((SubMap<K,V>)m).new SubMapKeyIterator();
     }

  }

@@ -2431,9 +2417,9 @@ } public Iterator iterator() { if (m instanceof ConcurrentSkipListMap)

•            return ((ConcurrentSkipListMap<K,V>)m).valueIterator();

•            return ((ConcurrentSkipListMap<K,V>)m).new ValueIterator();
         else

•            return ((SubMap<K,V>)m).valueIterator();

•            return ((SubMap<K,V>)m).new SubMapValueIterator();
     }
     public int size() { return m.size(); }
     public boolean isEmpty() { return m.isEmpty(); }

@@ -2441,12 +2427,12 @@ public void clear() { m.clear(); } public Object[] toArray() { return toList(this).toArray(); } public T[] toArray(T[] a) { return toList(this).toArray(a); }

•    @SuppressWarnings("unchecked")

•     public Spliterator<V> spliterator() {
          if (m instanceof ConcurrentSkipListMap)
              return ((ConcurrentSkipListMap<K,V>)m).valueSpliterator();
          else

•            return (Spliterator<V>)((SubMap<K,V>)m).valueIterator();

•            return ((SubMap<K,V>)m).new SubMapValueIterator();
     }
  
     public boolean removeIf(Predicate<? super V> filter) {

@@ -2454,7 +2440,8 @@ if (m instanceof ConcurrentSkipListMap) return ((ConcurrentSkipListMap<K,V>)m).removeValueIf(filter); // else use iterator

•        Iterator<Map.Entry<K,V>> it = ((SubMap<K,V>)m).entryIterator();

•        Iterator<Map.Entry<K,V>> it =

•            ((SubMap<K,V>)m).new SubMapEntryIterator();
         boolean removed = false;
         while (it.hasNext()) {
             Map.Entry<K,V> e = it.next();

@@ -2473,9 +2460,9 @@ } public Iterator<Map.Entry<K,V>> iterator() { if (m instanceof ConcurrentSkipListMap)

•            return ((ConcurrentSkipListMap<K,V>)m).entryIterator();

•            return ((ConcurrentSkipListMap<K,V>)m).new EntryIterator();
         else

•            return ((SubMap<K,V>)m).entryIterator();

•            return ((SubMap<K,V>)m).new SubMapEntryIterator();
     }
  
     public boolean contains(Object o) {

@@ -2517,20 +2504,20 @@ } public Object[] toArray() { return toList(this).toArray(); } public T[] toArray(T[] a) { return toList(this).toArray(a); }

•    @SuppressWarnings("unchecked")

•     public Spliterator<Map.Entry<K,V>> spliterator() {
          if (m instanceof ConcurrentSkipListMap)
              return ((ConcurrentSkipListMap<K,V>)m).entrySpliterator();
          else

•            return (Spliterator<Map.Entry<K,V>>)

•                ((SubMap<K,V>)m).entryIterator();

•            return ((SubMap<K,V>)m).new SubMapEntryIterator();
     }
     public boolean removeIf(Predicate<? super Entry<K,V>> filter) {
         if (filter == null) throw new NullPointerException();
         if (m instanceof ConcurrentSkipListMap)
             return

((ConcurrentSkipListMap<K,V>)m).removeEntryIf(filter); // else use iterator

•        Iterator<Map.Entry<K,V>> it = ((SubMap<K,V>)m).entryIterator();

•        Iterator<Map.Entry<K,V>> it =

•            ((SubMap<K,V>)m).new SubMapEntryIterator();
         boolean removed = false;
         while (it.hasNext()) {
             Map.Entry<K,V> e = it.next();

@@ -3062,18 +3049,6 @@ return descendingMap().navigableKeySet(); }

•    Iterator<K> keyIterator() {

•        return new SubMapKeyIterator();

•    }

•    Iterator<V> valueIterator() {

•        return new SubMapValueIterator();

•    }

•    Iterator<Map.Entry<K,V>> entryIterator() {

•        return new SubMapEntryIterator();

•    }

•     /**
       * Variant of main Iter class to traverse through submaps.
       * Also serves as back-up Spliterator for views

Index: ConcurrentSkipListSet.java

RCS file: /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/ConcurrentSkipListSet.java,v retrieving revision 1.47 diff -u -r1.47 ConcurrentSkipListSet.java --- ConcurrentSkipListSet.java 15 Jan 2015 18:34:18 -0000 1.47 +++ ConcurrentSkipListSet.java 5 May 2015 17:34:53 -0000 @@ -474,7 +474,7 @@ if (m instanceof ConcurrentSkipListMap) return ((ConcurrentSkipListMap<E,?>)m).keySpliterator(); else

•        return

(Spliterator)((ConcurrentSkipListMap.SubMap<E,?>)m).keyIterator();

•        return ((ConcurrentSkipListMap.SubMap<E,?>)m).new

SubMapKeyIterator(); }

 // Support for resetting map in clone

On Tue, May 5, 2015 at 2:12 AM, Paul Sandoz <paul.sandoz at oracle.com> wrote:

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)((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)((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 keySpliterator() { + return new SubMapKeyIterator(); + } + Iterator valueIterator() { return new SubMapValueIterator(); } + Spliterator 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