From 060fb30ef792b334dd2e28851ca0e28505528f80 Mon Sep 17 00:00:00 2001 From: psandoz Date: Fri, 9 Aug 2013 11:44:38 +0200 Subject: [PATCH] 8022326: Spliterator for values of j.u.c.ConcurrentSkipListMap does not report ORDERED Reviewed-by: martin, chegar --- jdk/src/share/classes/java/util/TreeMap.java | 7 +- .../concurrent/ConcurrentSkipListMap.java | 35 +++- .../SpliteratorCharacteristics.java | 150 ++++++++++++------ 3 files changed, 130 insertions(+), 62 deletions(-) diff --git a/jdk/src/share/classes/java/util/TreeMap.java b/jdk/src/share/classes/java/util/TreeMap.java index 352ac237ee..5c61d4f71e 100644 --- a/jdk/src/share/classes/java/util/TreeMap.java +++ b/jdk/src/share/classes/java/util/TreeMap.java @@ -2944,16 +2944,11 @@ public class TreeMap @Override public Comparator> getComparator() { - // Since SORTED is reported and Map.Entry elements are not comparable - // then a non-null comparator needs to be returned + // Adapt or create a key-based comparator if (tree.comparator != null) { - // Adapt the existing non-null comparator to compare entries - // by key return Map.Entry.comparingByKey(tree.comparator); } else { - // Return a comparator of entries by key, with K assumed to be - // of Comparable return (Comparator> & Serializable) (e1, e2) -> { @SuppressWarnings("unchecked") Comparable k1 = (Comparable) e1.getKey(); diff --git a/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java b/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java index 99faf16683..ff75003e4c 100644 --- a/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java +++ b/jdk/src/share/classes/java/util/concurrent/ConcurrentSkipListMap.java @@ -34,6 +34,7 @@ */ package java.util.concurrent; +import java.io.Serializable; import java.util.AbstractCollection; import java.util.AbstractMap; import java.util.AbstractSet; @@ -44,11 +45,15 @@ import java.util.Comparator; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.NavigableMap; import java.util.NavigableSet; import java.util.NoSuchElementException; import java.util.Set; import java.util.SortedMap; +import java.util.SortedSet; import java.util.Spliterator; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.ConcurrentNavigableMap; import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.BiConsumer; @@ -108,9 +113,7 @@ import java.util.function.Function; * @since 1.6 */ public class ConcurrentSkipListMap extends AbstractMap - implements ConcurrentNavigableMap, - Cloneable, - java.io.Serializable { + implements ConcurrentNavigableMap, Cloneable, Serializable { /* * This class implements a tree-like two-dimensionally linked skip * list in which the index levels are represented in separate @@ -1412,6 +1415,8 @@ public class ConcurrentSkipListMap extends AbstractMap /** * Saves this map to a stream (that is, serializes it). * + * @param s the stream + * @throws java.io.IOException if an I/O error occurs * @serialData The key (Object) and value (Object) for each * key-value mapping represented by the map, followed by * {@code null}. The key-value mappings are emitted in key-order @@ -1436,6 +1441,10 @@ public class ConcurrentSkipListMap extends AbstractMap /** * Reconstitutes this map from a stream (that is, deserializes it). + * @param s the stream + * @throws ClassNotFoundException if the class of a serialized object + * could not be found + * @throws java.io.IOException if an I/O error occurs */ @SuppressWarnings("unchecked") private void readObject(final java.io.ObjectInputStream s) @@ -2548,8 +2557,7 @@ public class ConcurrentSkipListMap extends AbstractMap * @serial include */ static final class SubMap extends AbstractMap - implements ConcurrentNavigableMap, Cloneable, - java.io.Serializable { + implements ConcurrentNavigableMap, Cloneable, Serializable { private static final long serialVersionUID = -7647078645895051609L; /** Underlying map */ @@ -3180,6 +3188,7 @@ public class ConcurrentSkipListMap extends AbstractMap public long estimateSize() { return Long.MAX_VALUE; } + } final class SubMapValueIterator extends SubMapIter { @@ -3434,7 +3443,8 @@ public class ConcurrentSkipListMap extends AbstractMap } public int characteristics() { - return Spliterator.CONCURRENT | Spliterator.NONNULL; + return Spliterator.CONCURRENT | Spliterator.ORDERED | + Spliterator.NONNULL; } } @@ -3528,8 +3538,17 @@ public class ConcurrentSkipListMap extends AbstractMap } public final Comparator> getComparator() { - return comparator == null ? null : - Map.Entry.comparingByKey(comparator); + // Adapt or create a key-based comparator + if (comparator != null) { + return Map.Entry.comparingByKey(comparator); + } + else { + return (Comparator> & Serializable) (e1, e2) -> { + @SuppressWarnings("unchecked") + Comparable k1 = (Comparable) e1.getKey(); + return k1.compareTo(e2.getKey()); + }; + } } } diff --git a/jdk/test/java/util/Spliterator/SpliteratorCharacteristics.java b/jdk/test/java/util/Spliterator/SpliteratorCharacteristics.java index d37a2ac903..bb0b7ecd97 100644 --- a/jdk/test/java/util/Spliterator/SpliteratorCharacteristics.java +++ b/jdk/test/java/util/Spliterator/SpliteratorCharacteristics.java @@ -23,7 +23,7 @@ /** * @test - * @bug 8020156 8020009 + * @bug 8020156 8020009 8022326 * @run testng SpliteratorCharacteristics */ @@ -32,79 +32,133 @@ import org.testng.annotations.Test; import java.util.Arrays; import java.util.Collection; import java.util.Comparator; +import java.util.Map; +import java.util.Set; +import java.util.SortedMap; +import java.util.SortedSet; import java.util.Spliterator; import java.util.TreeMap; import java.util.TreeSet; +import java.util.concurrent.ConcurrentSkipListMap; +import java.util.concurrent.ConcurrentSkipListSet; import static org.testng.Assert.*; @Test public class SpliteratorCharacteristics { + // TreeMap + public void testTreeMap() { - TreeMap tm = new TreeMap<>(); - tm.put(1, "4"); - tm.put(2, "3"); - tm.put(3, "2"); - tm.put(4, "1"); + assertSortedMapCharacteristics(new TreeMap<>(), + Spliterator.SIZED | Spliterator.DISTINCT | + Spliterator.SORTED | Spliterator.ORDERED); + } + + public void testTreeMapWithComparator() { + assertSortedMapCharacteristics(new TreeMap<>(Comparator.reverseOrder()), + Spliterator.SIZED | Spliterator.DISTINCT | + Spliterator.SORTED | Spliterator.ORDERED); + } - assertCharacteristics(tm.keySet(), - Spliterator.SIZED | Spliterator.DISTINCT | - Spliterator.SORTED | Spliterator.ORDERED); - assertNullComparator(tm.keySet()); - assertCharacteristics(tm.values(), - Spliterator.SIZED | Spliterator.ORDERED); - assertISEComparator(tm.values()); + // TreeSet - assertCharacteristics(tm.entrySet(), - Spliterator.SIZED | Spliterator.DISTINCT | - Spliterator.SORTED | Spliterator.ORDERED); - assertNotNullComparator(tm.entrySet()); + public void testTreeSet() { + assertSortedSetCharacteristics(new TreeSet<>(), + Spliterator.SIZED | Spliterator.DISTINCT | + Spliterator.SORTED | Spliterator.ORDERED); } - public void testTreeMapWithComparator() { - TreeMap tm = new TreeMap<>(Comparator.reverseOrder()); - tm.put(1, "4"); - tm.put(2, "3"); - tm.put(3, "2"); - tm.put(4, "1"); + public void testTreeSetWithComparator() { + assertSortedSetCharacteristics(new TreeSet<>(Comparator.reverseOrder()), + Spliterator.SIZED | Spliterator.DISTINCT | + Spliterator.SORTED | Spliterator.ORDERED); + } - assertCharacteristics(tm.keySet(), - Spliterator.SIZED | Spliterator.DISTINCT | - Spliterator.SORTED | Spliterator.ORDERED); - assertNotNullComparator(tm.keySet()); - assertCharacteristics(tm.values(), - Spliterator.SIZED | Spliterator.ORDERED); - assertISEComparator(tm.values()); + // ConcurrentSkipListMap - assertCharacteristics(tm.entrySet(), - Spliterator.SIZED | Spliterator.DISTINCT | - Spliterator.SORTED | Spliterator.ORDERED); - assertNotNullComparator(tm.entrySet()); + public void testConcurrentSkipListMap() { + assertSortedMapCharacteristics(new ConcurrentSkipListMap<>(), + Spliterator.CONCURRENT | Spliterator.NONNULL | + Spliterator.DISTINCT | Spliterator.SORTED | + Spliterator.ORDERED); + } + + public void testConcurrentSkipListMapWithComparator() { + assertSortedMapCharacteristics(new ConcurrentSkipListMap<>(Comparator.reverseOrder()), + Spliterator.CONCURRENT | Spliterator.NONNULL | + Spliterator.DISTINCT | Spliterator.SORTED | + Spliterator.ORDERED); } - public void testTreeSet() { - TreeSet ts = new TreeSet<>(); - ts.addAll(Arrays.asList(1, 2, 3, 4)); - assertCharacteristics(ts, - Spliterator.SIZED | Spliterator.DISTINCT | - Spliterator.SORTED | Spliterator.ORDERED); - assertNullComparator(ts); + // ConcurrentSkipListSet + + public void testConcurrentSkipListSet() { + assertSortedSetCharacteristics(new ConcurrentSkipListSet<>(), + Spliterator.CONCURRENT | Spliterator.NONNULL | + Spliterator.DISTINCT | Spliterator.SORTED | + Spliterator.ORDERED); + } + + public void testConcurrentSkipListSetWithComparator() { + assertSortedSetCharacteristics(new ConcurrentSkipListSet<>(Comparator.reverseOrder()), + Spliterator.CONCURRENT | Spliterator.NONNULL | + Spliterator.DISTINCT | Spliterator.SORTED | + Spliterator.ORDERED); } - public void testTreeSetWithComparator() { - TreeSet ts = new TreeSet<>(Comparator.reverseOrder()); - ts.addAll(Arrays.asList(1, 2, 3, 4)); - assertCharacteristics(ts, - Spliterator.SIZED | Spliterator.DISTINCT | - Spliterator.SORTED | Spliterator.ORDERED); - assertNotNullComparator(ts); + // + + void assertSortedMapCharacteristics(SortedMap m, int keyCharacteristics) { + initMap(m); + + boolean hasComparator = m.comparator() != null; + + Set keys = m.keySet(); + assertCharacteristics(keys, keyCharacteristics); + if (hasComparator) { + assertNotNullComparator(keys); + } + else { + assertNullComparator(keys); + } + + assertCharacteristics(m.values(), + keyCharacteristics & ~(Spliterator.DISTINCT | Spliterator.SORTED)); + assertISEComparator(m.values()); + + assertCharacteristics(m.entrySet(), keyCharacteristics); + assertNotNullComparator(m.entrySet()); + } + + void assertSortedSetCharacteristics(SortedSet s, int keyCharacteristics) { + initSet(s); + + boolean hasComparator = s.comparator() != null; + + assertCharacteristics(s, keyCharacteristics); + if (hasComparator) { + assertNotNullComparator(s); + } + else { + assertNullComparator(s); + } + } + + void initMap(Map m) { + m.put(1, "4"); + m.put(2, "3"); + m.put(3, "2"); + m.put(4, "1"); } + void initSet(Set s) { + s.addAll(Arrays.asList(1, 2, 3, 4)); + } void assertCharacteristics(Collection c, int expectedCharacteristics) { assertCharacteristics(c.spliterator(), expectedCharacteristics); -- GitLab