From 96e2b92fb134f79eb25b13a07b3193e86fa89873 Mon Sep 17 00:00:00 2001 From: Mike Duigou Date: Wed, 23 Oct 2013 14:32:41 -0700 Subject: [PATCH] 8024688: further split Map and ConcurrentMap defaults eliminating looping from Map defaults, Map.merge fixes and doc fixes Reviewed-by: psandoz, dholmes --- jdk/src/share/classes/java/util/HashMap.java | 37 +- jdk/src/share/classes/java/util/Map.java | 345 ++++++++---------- .../java/util/concurrent/ConcurrentMap.java | 335 ++++++++++++++++- jdk/test/java/util/Map/Defaults.java | 259 ++++++++----- 4 files changed, 676 insertions(+), 300 deletions(-) diff --git a/jdk/src/share/classes/java/util/HashMap.java b/jdk/src/share/classes/java/util/HashMap.java index db39a922242..1183952eb06 100644 --- a/jdk/src/share/classes/java/util/HashMap.java +++ b/jdk/src/share/classes/java/util/HashMap.java @@ -915,7 +915,7 @@ public class HashMap extends AbstractMap return removeNode(hash(key), key, null, false, true) != null; } public final Spliterator spliterator() { - return new KeySpliterator(HashMap.this, 0, -1, 0, 0); + return new KeySpliterator<>(HashMap.this, 0, -1, 0, 0); } public final void forEach(Consumer action) { Node[] tab; @@ -959,7 +959,7 @@ public class HashMap extends AbstractMap public final Iterator iterator() { return new ValueIterator(); } public final boolean contains(Object o) { return containsValue(o); } public final Spliterator spliterator() { - return new ValueSpliterator(HashMap.this, 0, -1, 0, 0); + return new ValueSpliterator<>(HashMap.this, 0, -1, 0, 0); } public final void forEach(Consumer action) { Node[] tab; @@ -1022,7 +1022,7 @@ public class HashMap extends AbstractMap return false; } public final Spliterator> spliterator() { - return new EntrySpliterator(HashMap.this, 0, -1, 0, 0); + return new EntrySpliterator<>(HashMap.this, 0, -1, 0, 0); } public final void forEach(Consumer> action) { Node[] tab; @@ -1042,19 +1042,23 @@ public class HashMap extends AbstractMap // Overrides of JDK8 Map extension methods + @Override public V getOrDefault(Object key, V defaultValue) { Node e; return (e = getNode(hash(key), key)) == null ? defaultValue : e.value; } + @Override public V putIfAbsent(K key, V value) { return putVal(hash(key), key, value, true, true); } + @Override public boolean remove(Object key, Object value) { return removeNode(hash(key), key, value, true, true) != null; } + @Override public boolean replace(K key, V oldValue, V newValue) { Node e; V v; if ((e = getNode(hash(key), key)) != null && @@ -1066,6 +1070,7 @@ public class HashMap extends AbstractMap return false; } + @Override public V replace(K key, V value) { Node e; if ((e = getNode(hash(key), key)) != null) { @@ -1077,6 +1082,7 @@ public class HashMap extends AbstractMap return null; } + @Override public V computeIfAbsent(K key, Function mappingFunction) { if (mappingFunction == null) @@ -1150,6 +1156,7 @@ public class HashMap extends AbstractMap return null; } + @Override public V compute(K key, BiFunction remappingFunction) { if (remappingFunction == null) @@ -1202,6 +1209,7 @@ public class HashMap extends AbstractMap return v; } + @Override public V merge(K key, V value, BiFunction remappingFunction) { if (remappingFunction == null) @@ -1230,7 +1238,11 @@ public class HashMap extends AbstractMap } } if (old != null) { - V v = remappingFunction.apply(old.value, value); + V v; + if (old.value != null) + v = remappingFunction.apply(old.value, value); + else + v = value; if (v != null) { old.value = v; afterNodeAccess(old); @@ -1254,6 +1266,7 @@ public class HashMap extends AbstractMap return value; } + @Override public void forEach(BiConsumer action) { Node[] tab; if (action == null) @@ -1269,6 +1282,7 @@ public class HashMap extends AbstractMap } } + @Override public void replaceAll(BiFunction function) { Node[] tab; if (function == null) @@ -1295,6 +1309,7 @@ public class HashMap extends AbstractMap * @return a shallow copy of this map */ @SuppressWarnings("unchecked") + @Override public Object clone() { HashMap result; try { @@ -1496,7 +1511,7 @@ public class HashMap extends AbstractMap public KeySpliterator trySplit() { int hi = getFence(), lo = index, mid = (lo + hi) >>> 1; return (lo >= mid || current != null) ? null : - new KeySpliterator(map, lo, index = mid, est >>>= 1, + new KeySpliterator<>(map, lo, index = mid, est >>>= 1, expectedModCount); } @@ -1568,7 +1583,7 @@ public class HashMap extends AbstractMap public ValueSpliterator trySplit() { int hi = getFence(), lo = index, mid = (lo + hi) >>> 1; return (lo >= mid || current != null) ? null : - new ValueSpliterator(map, lo, index = mid, est >>>= 1, + new ValueSpliterator<>(map, lo, index = mid, est >>>= 1, expectedModCount); } @@ -1639,7 +1654,7 @@ public class HashMap extends AbstractMap public EntrySpliterator trySplit() { int hi = getFence(), lo = index, mid = (lo + hi) >>> 1; return (lo >= mid || current != null) ? null : - new EntrySpliterator(map, lo, index = mid, est >>>= 1, + new EntrySpliterator<>(map, lo, index = mid, est >>>= 1, expectedModCount); } @@ -1714,22 +1729,22 @@ public class HashMap extends AbstractMap // Create a regular (non-tree) node Node newNode(int hash, K key, V value, Node next) { - return new Node(hash, key, value, next); + return new Node<>(hash, key, value, next); } // For conversion from TreeNodes to plain nodes Node replacementNode(Node p, Node next) { - return new Node(p.hash, p.key, p.value, next); + return new Node<>(p.hash, p.key, p.value, next); } // Create a tree bin node TreeNode newTreeNode(int hash, K key, V value, Node next) { - return new TreeNode(hash, key, value, next); + return new TreeNode<>(hash, key, value, next); } // For treeifyBin TreeNode replacementTreeNode(Node p, Node next) { - return new TreeNode(p.hash, p.key, p.value, next); + return new TreeNode<>(p.hash, p.key, p.value, next); } /** diff --git a/jdk/src/share/classes/java/util/Map.java b/jdk/src/share/classes/java/util/Map.java index 0adf9d45bc5..5c3f954c2a0 100644 --- a/jdk/src/share/classes/java/util/Map.java +++ b/jdk/src/share/classes/java/util/Map.java @@ -565,7 +565,8 @@ public interface Map { * Returns the value to which the specified key is mapped, or * {@code defaultValue} if this map contains no mapping for the key. * - *

The default implementation makes no guarantees about synchronization + * @implSpec + * The default implementation makes no guarantees about synchronization * or atomicity properties of this method. Any implementation providing * atomicity guarantees must override this method and document its * concurrency properties. @@ -596,22 +597,18 @@ public interface Map { * the order of entry set iteration (if an iteration order is specified.) * Exceptions thrown by the action are relayed to the caller. * - *

The default implementation should be overridden by implementations if - * they can provide a more performant implementation than an iterator-based - * one. - * - *

The default implementation makes no guarantees about synchronization - * or atomicity properties of this method. Any implementation providing - * atomicity guarantees must override this method and document its - * concurrency properties. - * - * @implSpec The default implementation is equivalent to, for this - * {@code map}: + * @implSpec + * The default implementation is equivalent to, for this {@code map}: * 

 {@code
      * for ((Map.Entry entry : map.entrySet())
      *     action.accept(entry.getKey(), entry.getValue());
      * }
* + * The default implementation makes no guarantees about synchronization + * or atomicity properties of this method. Any implementation providing + * atomicity guarantees must override this method and document its + * concurrency properties. + * * @param action The action to be performed for each entry * @throws NullPointerException if the specified action is null * @throws ConcurrentModificationException if an entry is found to be @@ -640,11 +637,6 @@ public interface Map { * function throws an exception. Exceptions thrown by the function are * relayed to the caller. * - *

The default implementation makes no guarantees about synchronization - * or atomicity properties of this method. Any implementation providing - * atomicity guarantees must override this method and document its - * concurrency properties. - * * @implSpec *

The default implementation is equivalent to, for this {@code map}: * 

 {@code
@@ -652,6 +644,11 @@ public interface Map {
      *     entry.setValue(function.apply(entry.getKey(), entry.getValue()));
      * }
* + *

The default implementation makes no guarantees about synchronization + * or atomicity properties of this method. Any implementation providing + * atomicity guarantees must override this method and document its + * concurrency properties. + * * @param function the function to apply to each entry * @throws UnsupportedOperationException if the {@code set} operation * is not supported by this map's entry set iterator. @@ -703,22 +700,23 @@ public interface Map { * to {@code null}) associates it with the given value and returns * {@code null}, else returns the current value. * - *

The default implementation makes no guarantees about synchronization - * or atomicity properties of this method. Any implementation providing - * atomicity guarantees must override this method and document its - * concurrency properties. - * * @implSpec * The default implementation is equivalent to, for this {@code * map}: * * 

 {@code
-     * if (map.get(key) == null)
-     *     return map.put(key, value);
-     * else
-     *     return map.get(key);
+     * V v = map.get(key);
+     * if (v == null)
+     *     v = map.put(key, value);
+     *
+     * return v;
      * }
* + *

The default implementation makes no guarantees about synchronization + * or atomicity properties of this method. Any implementation providing + * atomicity guarantees must override this method and document its + * concurrency properties. + * * @param key key with which the specified value is to be associated * @param value value to be associated with the specified key * @return the previous value associated with the specified key, or @@ -738,16 +736,12 @@ public interface Map { * @throws IllegalArgumentException if some property of the specified key * or value prevents it from being stored in this map * (optional) - * @throws ConcurrentModificationException if a modification of the map is - * detected during insertion of the value. * @since 1.8 */ default V putIfAbsent(K key, V value) { V v = get(key); if (v == null) { - if (put(key, value) != null) { - throw new ConcurrentModificationException(); - } + v = put(key, value); } return v; @@ -757,11 +751,6 @@ public interface Map { * Removes the entry for the specified key only if it is currently * mapped to the specified value. * - *

The default implementation makes no guarantees about synchronization - * or atomicity properties of this method. Any implementation providing - * atomicity guarantees must override this method and document its - * concurrency properties. - * * @implSpec * The default implementation is equivalent to, for this {@code map}: * @@ -773,6 +762,11 @@ public interface Map { * return false; * } * + *

The default implementation makes no guarantees about synchronization + * or atomicity properties of this method. Any implementation providing + * atomicity guarantees must override this method and document its + * concurrency properties. + * * @param key key with which the specified value is associated * @param value value expected to be associated with the specified key * @return {@code true} if the value was removed @@ -801,11 +795,6 @@ public interface Map { * Replaces the entry for the specified key only if currently * mapped to the specified value. * - *

The default implementation makes no guarantees about synchronization - * or atomicity properties of this method. Any implementation providing - * atomicity guarantees must override this method and document its - * concurrency properties. - * * @implSpec * The default implementation is equivalent to, for this {@code map}: * @@ -821,6 +810,11 @@ public interface Map { * for maps that do not support null values if oldValue is null unless * newValue is also null. * + *

The default implementation makes no guarantees about synchronization + * or atomicity properties of this method. Any implementation providing + * atomicity guarantees must override this method and document its + * concurrency properties. + * * @param key key with which the specified value is associated * @param oldValue value expected to be associated with the specified key * @param newValue value to be associated with the specified key @@ -853,11 +847,6 @@ public interface Map { * Replaces the entry for the specified key only if it is * currently mapped to some value. * - *

The default implementation makes no guarantees about synchronization - * or atomicity properties of this method. Any implementation providing - * atomicity guarantees must override this method and document its - * concurrency properties. - * * @implSpec * The default implementation is equivalent to, for this {@code map}: * @@ -868,6 +857,11 @@ public interface Map { * return null; * } * + *

The default implementation makes no guarantees about synchronization + * or atomicity properties of this method. Any implementation providing + * atomicity guarantees must override this method and document its + * concurrency properties. + * * @param key key with which the specified value is associated * @param value value to be associated with the specified key * @return the previous value associated with the specified key, or @@ -888,14 +882,17 @@ public interface Map { * @since 1.8 */ default V replace(K key, V value) { - return containsKey(key) ? put(key, value) : null; + V curValue; + if (((curValue = get(key)) != null) || containsKey(key)) { + curValue = put(key, value); + } + return curValue; } /** - * If the specified key is not already associated with a value (or - * is mapped to {@code null}), attempts to compute its value using - * the given mapping function and enters it into this map unless - * {@code null}. + * If the specified key is not already associated with a value (or is mapped + * to {@code null}), attempts to compute its value using the given mapping + * function and enters it into this map unless {@code null}. * *

If the function returns {@code null} no mapping is recorded. If * the function itself throws an (unchecked) exception, the @@ -907,35 +904,42 @@ public interface Map { * map.computeIfAbsent(key, k -> new Value(f(k))); * } * + *

Or to implement a multi-value map, {@code Map>}, + * supporting multiple values per key: + * + * 

 {@code
+     * map.computeIfAbsent(key, k -> new HashSet()).add(v);
+     * }
+ * + * + * @implSpec + * The default implementation is equivalent to the following steps for this + * {@code map}, then returning the current value or {@code null} if now + * absent: + * + * 
 {@code
+     * if (map.get(key) == null) {
+     *     V newValue = mappingFunction.apply(key);
+     *     if (newValue != null)
+     *         map.put(key, newValue);
+     * }
+     * }
+ * *

The default implementation makes no guarantees about synchronization * or atomicity properties of this method. Any implementation providing * atomicity guarantees must override this method and document its * concurrency properties. In particular, all implementations of * subinterface {@link java.util.concurrent.ConcurrentMap} must document * whether the function is applied once atomically only if the value is not - * present. Any class that permits null values must document - * whether and how this method distinguishes absence from null mappings. - * - * @implSpec - * The default implementation is equivalent to the following - * steps for this {@code map}, then returning the current value or - * {@code null} if now absent: - * - * 

 {@code
-     * if (map.get(key) == null) {
-     *     V newValue = mappingFunction.apply(key);
-     *     if (newValue != null)
-     *         map.putIfAbsent(key, newValue);
-     * }
-     * }
+ * present. * * @param key key with which the specified value is to be associated * @param mappingFunction the function to compute a value * @return the current (existing or computed) value associated with * the specified key, or null if the computed value is null * @throws NullPointerException if the specified key is null and - * this map does not support null keys, or the - * mappingFunction is null + * this map does not support null keys, or the mappingFunction + * is null * @throws UnsupportedOperationException if the {@code put} operation * is not supported by this map * (optional) @@ -947,10 +951,16 @@ public interface Map { default V computeIfAbsent(K key, Function mappingFunction) { Objects.requireNonNull(mappingFunction); - V v, newValue; - return ((v = get(key)) == null && - (newValue = mappingFunction.apply(key)) != null && - (v = putIfAbsent(key, newValue)) == null) ? newValue : v; + V v; + if ((v = get(key)) == null) { + V newValue; + if ((newValue = mappingFunction.apply(key)) != null) { + put(key, newValue); + return newValue; + } + } + + return v; } /** @@ -960,6 +970,22 @@ public interface Map { *

If the function returns {@code null}, the mapping is removed. If the * function itself throws an (unchecked) exception, the exception is * rethrown, and the current mapping is left unchanged. + * + * @implSpec + * The default implementation is equivalent to performing the following + * steps for this {@code map}, then returning the current value or + * {@code null} if now absent: + * + * 

 {@code
+     * if (map.get(key) != null) {
+     *     V oldValue = map.get(key);
+     *     V newValue = remappingFunction.apply(key, oldValue);
+     *     if (newValue != null)
+     *         map.put(key, newValue);
+     *     else
+     *         map.remove(key);
+     * }
+     * }
* *

The default implementation makes no guarantees about synchronization * or atomicity properties of this method. Any implementation providing @@ -967,27 +993,7 @@ public interface Map { * concurrency properties. In particular, all implementations of * subinterface {@link java.util.concurrent.ConcurrentMap} must document * whether the function is applied once atomically only if the value is not - * present. Any class that permits null values must document - * whether and how this method distinguishes absence from null mappings. - * - * @implSpec - * The default implementation is equivalent to performing the - * following steps for this {@code map}, then returning the - * current value or {@code null} if now absent: - * - * 

 {@code
-     * if (map.get(key) != null) {
-     *     V oldValue = map.get(key);
-     *     V newValue = remappingFunction.apply(key, oldValue);
-     *     if (newValue != null)
-     *         map.replace(key, oldValue, newValue);
-     *     else
-     *         map.remove(key, oldValue);
-     * }
-     * }
- * - * In concurrent contexts, the default implementation may retry - * these steps when multiple threads attempt updates. + * present. * * @param key key with which the specified value is to be associated * @param remappingFunction the function to compute a value @@ -1007,22 +1013,25 @@ public interface Map { BiFunction remappingFunction) { Objects.requireNonNull(remappingFunction); V oldValue; - while ((oldValue = get(key)) != null) { + if ((oldValue = get(key)) != null) { V newValue = remappingFunction.apply(key, oldValue); if (newValue != null) { - if (replace(key, oldValue, newValue)) - return newValue; - } else if (remove(key, oldValue)) + put(key, newValue); + return newValue; + } else { + remove(key); return null; + } + } else { + return null; } - return oldValue; } /** - * Attempts to compute a mapping for the specified key and its - * current mapped value (or {@code null} if there is no current - * mapping). For example, to either create or append a {@code - * String msg} to a value mapping: + * Attempts to compute a mapping for the specified key and its current + * mapped value (or {@code null} if there is no current mapping). For + * example, to either create or append a {@code String} msg to a value + * mapping: * * 
 {@code
      * map.compute(key, (k, v) -> (v == null) ? msg : v.concat(msg))}
@@ -1033,15 +1042,6 @@ public interface Map { * (unchecked) exception, the exception is rethrown, and the current mapping * is left unchanged. * - *

The default implementation makes no guarantees about synchronization - * or atomicity properties of this method. Any implementation providing - * atomicity guarantees must override this method and document its - * concurrency properties. In particular, all implementations of - * subinterface {@link java.util.concurrent.ConcurrentMap} must document - * whether the function is applied once atomically only if the value is not - * present. Any class that permits null values must document - * whether and how this method distinguishes absence from null mappings. - * * @implSpec * The default implementation is equivalent to performing the following * steps for this {@code map}, then returning the current value or @@ -1052,19 +1052,24 @@ public interface Map { * V newValue = remappingFunction.apply(key, oldValue); * if (oldValue != null ) { * if (newValue != null) - * map.replace(key, oldValue, newValue); + * map.put(key, newValue); * else - * map.remove(key, oldValue); + * map.remove(key); * } else { * if (newValue != null) - * map.putIfAbsent(key, newValue); + * map.put(key, newValue); * else * return null; * } * } * - * In concurrent contexts, the default implementation may retry - * these steps when multiple threads attempt updates. + *

The default implementation makes no guarantees about synchronization + * or atomicity properties of this method. Any implementation providing + * atomicity guarantees must override this method and document its + * concurrency properties. In particular, all implementations of + * subinterface {@link java.util.concurrent.ConcurrentMap} must document + * whether the function is applied once atomically only if the value is not + * present. * * @param key key with which the specified value is to be associated * @param remappingFunction the function to compute a value @@ -1084,44 +1089,22 @@ public interface Map { BiFunction remappingFunction) { Objects.requireNonNull(remappingFunction); V oldValue = get(key); - for (;;) { - V newValue = remappingFunction.apply(key, oldValue); - if (newValue == null) { - // delete mapping - if(oldValue != null || containsKey(key)) { - // something to remove - if (remove(key, oldValue)) { - // removed the old value as expected - return null; - } - // some other value replaced old value. try again. - oldValue = get(key); - } else { - // nothing to do. Leave things as they were. - return null; - } + V newValue = remappingFunction.apply(key, oldValue); + if (newValue == null) { + // delete mapping + if (oldValue != null || containsKey(key)) { + // something to remove + remove(key); + return null; } else { - // add or replace old mapping - if (oldValue != null) { - // replace - if (replace(key, oldValue, newValue)) { - // replaced as expected. - return newValue; - } - - // some other value replaced old value. try again. - oldValue = get(key); - } else { - // add (replace if oldValue was null) - if ((oldValue = putIfAbsent(key, newValue)) == null) { - // replaced - return newValue; - } - - // some other value replaced old value. try again. - } + // nothing to do. Leave things as they were. + return null; } + } else { + // add or replace old mapping + put(key, newValue); + return newValue; } } @@ -1143,15 +1126,6 @@ public interface Map { * (unchecked) exception, the exception is rethrown, and the current mapping * is left unchanged. * - *

The default implementation makes no guarantees about synchronization - * or atomicity properties of this method. Any implementation providing - * atomicity guarantees must override this method and document its - * concurrency properties. In particular, all implementations of - * subinterface {@link java.util.concurrent.ConcurrentMap} must document - * whether the function is applied once atomically only if the value is not - * present. Any class that permits null values must document - * whether and how this method distinguishes absence from null mappings. - * * @implSpec * The default implementation is equivalent to performing the * following steps for this {@code map}, then returning the @@ -1162,15 +1136,20 @@ public interface Map { * V newValue = (oldValue == null) ? value : * remappingFunction.apply(oldValue, value); * if (newValue == null) - * map.remove(key, oldValue); + * map.remove(key); * else if (oldValue == null) - * map.putIfAbsent(key, newValue); + * map.remove(key); * else - * map.replace(key, oldValue, newValue); + * map.put(key, newValue); * } * - * In concurrent contexts, the default implementation may retry - * these steps when multiple threads attempt updates. + *

The default implementation makes no guarantees about synchronization + * or atomicity properties of this method. Any implementation providing + * atomicity guarantees must override this method and document its + * concurrency properties. In particular, all implementations of + * subinterface {@link java.util.concurrent.ConcurrentMap} must document + * whether the function is applied once atomically only if the value is not + * present. * * @param key key with which the specified value is to be associated * @param value the value to use if absent @@ -1183,32 +1162,30 @@ public interface Map { * prevents it from being stored in this map * (optional) * @throws NullPointerException if the specified key is null and - * this map does not support null keys, or the - * remappingFunction is null + * this map does not support null keys, or the remappingFunction + * is null * @since 1.8 */ default V merge(K key, V value, BiFunction remappingFunction) { Objects.requireNonNull(remappingFunction); V oldValue = get(key); - for (;;) { - if (oldValue != null) { - V newValue = remappingFunction.apply(oldValue, value); - if (newValue != null) { - if (replace(key, oldValue, newValue)) - return newValue; - } else if (remove(key, oldValue)) { - return null; - } - oldValue = get(key); + if (oldValue != null) { + V newValue = remappingFunction.apply(oldValue, value); + if (newValue != null) { + put(key, newValue); + return newValue; } else { - if (value == null) { - return null; - } - - if ((oldValue = putIfAbsent(key, value)) == null) { - return value; - } + remove(key); + return null; + } + } else { + if (value == null) { + remove(key); + return null; + } else { + put(key, value); + return value; } } } diff --git a/jdk/src/share/classes/java/util/concurrent/ConcurrentMap.java b/jdk/src/share/classes/java/util/concurrent/ConcurrentMap.java index 6c902fa6868..3cb1fe7ea8f 100644 --- a/jdk/src/share/classes/java/util/concurrent/ConcurrentMap.java +++ b/jdk/src/share/classes/java/util/concurrent/ConcurrentMap.java @@ -36,7 +36,9 @@ package java.util.concurrent; import java.util.Map; import java.util.Objects; +import java.util.function.BiConsumer; import java.util.function.BiFunction; +import java.util.function.Function; /** * A {@link java.util.Map} providing thread safety and atomicity @@ -64,9 +66,13 @@ public interface ConcurrentMap extends Map { * {@inheritDoc} * * @implNote This implementation assumes that the ConcurrentMap cannot - * contain null values and get() returning null unambiguously means the key - * is absent. Implementations which support null values must override this - * default implementation. + * contain null values and {@code get()} returning null unambiguously means + * the key is absent. Implementations which support null values + * must override this default implementation. + * + * @throws ClassCastException {@inheritDoc} + * @throws NullPointerException {@inheritDoc} + * @since 1.8 */ @Override default V getOrDefault(Object key, V defaultValue) { @@ -74,6 +80,41 @@ public interface ConcurrentMap extends Map { return ((v = get(key)) != null) ? v : defaultValue; } + /** + * {@inheritDoc} + * + * @implSpec The default implementation is equivalent to, for this + * {@code map}: + * 

 {@code
+     * for ((Map.Entry entry : map.entrySet())
+     *     action.accept(entry.getKey(), entry.getValue());
+     * }
+ * + * @implNote The default implementation assumes that + * {@code IllegalStateException} thrown by {@code getKey()} or + * {@code getValue()} indicates that the entry has been removed and cannot + * be processed. Operation continues for subsequent entries. + * + * @throws NullPointerException {@inheritDoc} + * @since 1.8 + */ + @Override + default void forEach(BiConsumer action) { + Objects.requireNonNull(action); + for (Map.Entry entry : entrySet()) { + K k; + V v; + try { + k = entry.getKey(); + v = entry.getValue(); + } catch(IllegalStateException ise) { + // this usually means the entry is no longer in the map. + continue; + } + action.accept(k, v); + } + } + /** * If the specified key is not already associated * with a value, associate it with the given value. @@ -82,10 +123,14 @@ public interface ConcurrentMap extends Map { * if (!map.containsKey(key)) * return map.put(key, value); * else - * return map.get(key);} + * return map.get(key); + * } * * except that the action is performed atomically. * + * @implNote This implementation intentionally re-abstracts the + * inappropriate default provided in {@code Map}. + * * @param key key with which the specified value is to be associated * @param value value to be associated with the specified key * @return the previous value associated with the specified key, or @@ -102,7 +147,7 @@ public interface ConcurrentMap extends Map { * @throws IllegalArgumentException if some property of the specified key * or value prevents it from being stored in this map */ - V putIfAbsent(K key, V value); + V putIfAbsent(K key, V value); /** * Removes the entry for a key only if currently mapped to a given value. @@ -112,10 +157,14 @@ public interface ConcurrentMap extends Map { * map.remove(key); * return true; * } else - * return false;} + * return false; + * } * * except that the action is performed atomically. * + * @implNote This implementation intentionally re-abstracts the + * inappropriate default provided in {@code Map}. + * * @param key key with which the specified value is associated * @param value value expected to be associated with the specified key * @return {@code true} if the value was removed @@ -138,10 +187,14 @@ public interface ConcurrentMap extends Map { * map.put(key, newValue); * return true; * } else - * return false;} + * return false; + * } * * except that the action is performed atomically. * + * @implNote This implementation intentionally re-abstracts the + * inappropriate default provided in {@code Map}. + * * @param key key with which the specified value is associated * @param oldValue value expected to be associated with the specified key * @param newValue value to be associated with the specified key @@ -164,10 +217,14 @@ public interface ConcurrentMap extends Map { * if (map.containsKey(key)) { * return map.put(key, value); * } else - * return null;} + * return null; + * } * * except that the action is performed atomically. * + * @implNote This implementation intentionally re-abstracts the + * inappropriate default provided in {@code Map}. + * * @param key key with which the specified value is associated * @param value value to be associated with the specified key * @return the previous value associated with the specified key, or @@ -189,10 +246,30 @@ public interface ConcurrentMap extends Map { /** * {@inheritDoc} * - * @implNote This implementation assumes that the ConcurrentMap cannot - * contain null values and get() returning null unambiguously means the key - * is absent. Implementations which support null values - * must override this default implementation. + * @implSpec + *

The default implementation is equivalent to, for this {@code map}: + * 

 {@code
+     * for ((Map.Entry entry : map.entrySet())
+     *     do {
+     *        K k = entry.getKey();
+     *        V v = entry.getValue();
+     *     } while(!replace(k, v, function.apply(k, v)));
+     * }
+ * + * The default implementation may retry these steps when multiple + * threads attempt updates including potentially calling the function + * repeatedly for a given key. + * + *

This implementation assumes that the ConcurrentMap cannot contain null + * values and {@code get()} returning null unambiguously means the key is + * absent. Implementations which support null values must + * override this default implementation. + * + * @throws UnsupportedOperationException {@inheritDoc} + * @throws NullPointerException {@inheritDoc} + * @throws ClassCastException {@inheritDoc} + * @throws IllegalArgumentException {@inheritDoc} + * @since 1.8 */ @Override default void replaceAll(BiFunction function) { @@ -200,11 +277,243 @@ public interface ConcurrentMap extends Map { forEach((k,v) -> { while(!replace(k, v, function.apply(k, v))) { // v changed or k is gone - if( (v = get(k)) == null) { + if ( (v = get(k)) == null) { // k is no longer in the map. break; } } }); } + + /** + * {@inheritDoc} + * + * @implSpec + * The default implementation is equivalent to the following steps for this + * {@code map}, then returning the current value or {@code null} if now + * absent: + * + * 

 {@code
+     * if (map.get(key) == null) {
+     *     V newValue = mappingFunction.apply(key);
+     *     if (newValue != null)
+     *         return map.putIfAbsent(key, newValue);
+     * }
+     * }
+ * + * The default implementation may retry these steps when multiple + * threads attempt updates including potentially calling the mapping + * function multiple times. + * + *

This implementation assumes that the ConcurrentMap cannot contain null + * values and {@code get()} returning null unambiguously means the key is + * absent. Implementations which support null values must + * override this default implementation. + * + * @throws UnsupportedOperationException {@inheritDoc} + * @throws ClassCastException {@inheritDoc} + * @throws NullPointerException {@inheritDoc} + * @since 1.8 + */ + @Override + default V computeIfAbsent(K key, + Function mappingFunction) { + Objects.requireNonNull(mappingFunction); + V v, newValue; + return ((v = get(key)) == null && + (newValue = mappingFunction.apply(key)) != null && + (v = putIfAbsent(key, newValue)) == null) ? newValue : v; + } + + /** + * {@inheritDoc} + * + * @implSpec + * The default implementation is equivalent to performing the following + * steps for this {@code map}, then returning the current value or + * {@code null} if now absent. : + * + * 

 {@code
+     * if (map.get(key) != null) {
+     *     V oldValue = map.get(key);
+     *     V newValue = remappingFunction.apply(key, oldValue);
+     *     if (newValue != null)
+     *         map.replace(key, oldValue, newValue);
+     *     else
+     *         map.remove(key, oldValue);
+     * }
+     * }
+ * + * The default implementation may retry these steps when multiple threads + * attempt updates including potentially calling the remapping function + * multiple times. + * + *

This implementation assumes that the ConcurrentMap cannot contain null + * values and {@code get()} returning null unambiguously means the key is + * absent. Implementations which support null values must + * override this default implementation. + * + * @throws UnsupportedOperationException {@inheritDoc} + * @throws ClassCastException {@inheritDoc} + * @throws NullPointerException {@inheritDoc} + * @since 1.8 + */ + @Override + default V computeIfPresent(K key, + BiFunction remappingFunction) { + Objects.requireNonNull(remappingFunction); + V oldValue; + while((oldValue = get(key)) != null) { + V newValue = remappingFunction.apply(key, oldValue); + if (newValue != null) { + if (replace(key, oldValue, newValue)) + return newValue; + } else if (remove(key, oldValue)) + return null; + } + return oldValue; + } + + /** + * {@inheritDoc} + * + * @implSpec + * The default implementation is equivalent to performing the following + * steps for this {@code map}, then returning the current value or + * {@code null} if absent: + * + * 

 {@code
+     * V oldValue = map.get(key);
+     * V newValue = remappingFunction.apply(key, oldValue);
+     * if (oldValue != null ) {
+     *    if (newValue != null)
+     *       map.replace(key, oldValue, newValue);
+     *    else
+     *       map.remove(key, oldValue);
+     * } else {
+     *    if (newValue != null)
+     *       map.putIfAbsent(key, newValue);
+     *    else
+     *       return null;
+     * }
+     * }
+ * + * The default implementation may retry these steps when multiple + * threads attempt updates including potentially calling the remapping + * function multiple times. + * + *

This implementation assumes that the ConcurrentMap cannot contain null + * values and {@code get()} returning null unambiguously means the key is + * absent. Implementations which support null values must + * override this default implementation. + * + * @throws UnsupportedOperationException {@inheritDoc} + * @throws ClassCastException {@inheritDoc} + * @throws NullPointerException {@inheritDoc} + * @since 1.8 + */ + @Override + default V compute(K key, + BiFunction remappingFunction) { + Objects.requireNonNull(remappingFunction); + V oldValue = get(key); + for(;;) { + V newValue = remappingFunction.apply(key, oldValue); + if (newValue == null) { + // delete mapping + if (oldValue != null || containsKey(key)) { + // something to remove + if (remove(key, oldValue)) { + // removed the old value as expected + return null; + } + + // some other value replaced old value. try again. + oldValue = get(key); + } else { + // nothing to do. Leave things as they were. + return null; + } + } else { + // add or replace old mapping + if (oldValue != null) { + // replace + if (replace(key, oldValue, newValue)) { + // replaced as expected. + return newValue; + } + + // some other value replaced old value. try again. + oldValue = get(key); + } else { + // add (replace if oldValue was null) + if ((oldValue = putIfAbsent(key, newValue)) == null) { + // replaced + return newValue; + } + + // some other value replaced old value. try again. + } + } + } + } + + + /** + * {@inheritDoc} + * + * @implSpec + * The default implementation is equivalent to performing the + * following steps for this {@code map}, then returning the + * current value or {@code null} if absent: + * + * 

 {@code
+     * V oldValue = map.get(key);
+     * V newValue = (oldValue == null) ? value :
+     *              remappingFunction.apply(oldValue, value);
+     * if (newValue == null)
+     *     map.remove(key);
+     * else if (oldValue == null)
+     *     map.remove(key);
+     * else
+     *     map.put(key, newValue);
+     * }
+ * + *

The default implementation may retry these steps when multiple + * threads attempt updates including potentially calling the remapping + * function multiple times. + * + *

This implementation assumes that the ConcurrentMap cannot contain null + * values and {@code get()} returning null unambiguously means the key is + * absent. Implementations which support null values must + * override this default implementation. + * + * @throws UnsupportedOperationException {@inheritDoc} + * @throws ClassCastException {@inheritDoc} + * @throws NullPointerException {@inheritDoc} + * @since 1.8 + */ + @Override + default V merge(K key, V value, + BiFunction remappingFunction) { + Objects.requireNonNull(remappingFunction); + Objects.requireNonNull(value); + V oldValue = get(key); + for (;;) { + if (oldValue != null) { + V newValue = remappingFunction.apply(oldValue, value); + if (newValue != null) { + if (replace(key, oldValue, newValue)) + return newValue; + } else if (remove(key, oldValue)) { + return null; + } + oldValue = get(key); + } else { + if ((oldValue = putIfAbsent(key, value)) == null) { + return value; + } + } + } + } } diff --git a/jdk/test/java/util/Map/Defaults.java b/jdk/test/java/util/Map/Defaults.java index f14cd8aaa4b..46a49a63a51 100644 --- a/jdk/test/java/util/Map/Defaults.java +++ b/jdk/test/java/util/Map/Defaults.java @@ -23,7 +23,7 @@ /* * @test - * @bug 8010122 8004518 8024331 + * @bug 8010122 8004518 8024331 8024688 * @summary Test Map default methods * @author Mike Duigou * @run testng Defaults @@ -36,8 +36,8 @@ import java.util.Collection; import java.util.Collections; import java.util.EnumMap; import java.util.HashMap; -import java.util.HashSet; import java.util.Hashtable; +import java.util.HashSet; import java.util.IdentityHashMap; import java.util.Iterator; import java.util.LinkedHashMap; @@ -288,19 +288,11 @@ public class Defaults { assertSame(map.get(EXTRA_KEY), EXTRA_VALUE); } - @Test(expectedExceptions = {NullPointerException.class}) - public void testComputeIfAbsentNPEHashMap() { - Object value = new HashMap().computeIfAbsent(KEYS[1], null); - } - - @Test(expectedExceptions = {NullPointerException.class}) - public void testComputeIfAbsentNPEHashtable() { - Object value = new Hashtable().computeIfAbsent(KEYS[1], null); - } - - @Test(expectedExceptions = {NullPointerException.class}) - public void testComputeIfAbsentNPETreeMap() { - Object value = new TreeMap().computeIfAbsent(KEYS[1], null); + @Test(dataProvider = "Map rw=true keys=all values=all") + public void testComputeIfAbsentNullFunction(String description, Map map) { + assertThrows( () -> { map.computeIfAbsent(KEYS[1], null);}, + NullPointerException.class, + "Should throw NPE"); } @Test(dataProvider = "Map rw=true keys=withNull values=withNull") @@ -343,22 +335,14 @@ public class Defaults { assertSame(map.get(EXTRA_KEY), null); } - @Test(expectedExceptions = {NullPointerException.class}) - public void testComputeIfPresentNPEHashMap() { - Object value = new HashMap().computeIfPresent(KEYS[1], null); + @Test(dataProvider = "Map rw=true keys=all values=all") + public void testComputeIfPresentNullFunction(String description, Map map) { + assertThrows( () -> { map.computeIfPresent(KEYS[1], null);}, + NullPointerException.class, + "Should throw NPE"); } - @Test(expectedExceptions = {NullPointerException.class}) - public void testComputeIfPresentNPEHashtable() { - Object value = new Hashtable().computeIfPresent(KEYS[1], null); - } - - @Test(expectedExceptions = {NullPointerException.class}) - public void testComputeIfPresentNPETreeMap() { - Object value = new TreeMap().computeIfPresent(KEYS[1], null); - } - - @Test(dataProvider = "Map rw=true keys=withNull values=withNull") + @Test(dataProvider = "Map rw=true keys=withNull values=withNull") public void testComputeNulls(String description, Map map) { assertTrue(map.containsKey(null), "null key absent"); assertNull(map.get(null), "value not null"); @@ -444,78 +428,86 @@ public class Defaults { assertSame(map.get(EXTRA_KEY), EXTRA_VALUE); } - @Test(expectedExceptions = {NullPointerException.class}) - public void testComputeNPEHashMap() { - Object value = new HashMap().compute(KEYS[1], null); + @Test(dataProvider = "Map rw=true keys=all values=all") + public void testComputeNullFunction(String description, Map map) { + assertThrows( () -> { map.compute(KEYS[1], null);}, + NullPointerException.class, + "Should throw NPE"); } - @Test(expectedExceptions = {NullPointerException.class}) - public void testComputeNPEHashtable() { - Object value = new Hashtable().compute(KEYS[1], null); - } + @Test(dataProvider = "MergeCases") + private void testMerge(String description, Map map, Merging.Value oldValue, Merging.Value newValue, Merging.Merger merger, Merging.Value put, Merging.Value result) { + // add and check initial conditions. + switch(oldValue) { + case ABSENT : + map.remove(EXTRA_KEY); + assertFalse(map.containsKey(EXTRA_KEY), "key not absent"); + break; + case NULL : + map.put(EXTRA_KEY, null); + assertTrue(map.containsKey(EXTRA_KEY), "key absent"); + assertNull(map.get(EXTRA_KEY), "wrong value"); + break; + case OLDVALUE : + map.put(EXTRA_KEY, VALUES[1]); + assertTrue(map.containsKey(EXTRA_KEY), "key absent"); + assertSame(map.get(EXTRA_KEY), VALUES[1], "wrong value"); + break; + default: + fail("unexpected old value"); + } - @Test(expectedExceptions = {NullPointerException.class}) - public void testComputeNPETreeMap() { - Object value = new TreeMap().compute(KEYS[1], null); - } + String returned = map.merge(EXTRA_KEY, + newValue == Merging.Value.NULL ? (String) null : VALUES[2], + merger + ); - @Test(dataProvider = "Map rw=true keys=withNull values=withNull") - public void testMergeNulls(String description, Map map) { - assertTrue(map.containsKey(null), "null key absent"); - assertNull(map.get(null), "value not null"); - assertSame(map.merge(null, EXTRA_VALUE, (v, vv) -> { - assertNull(v); - assertSame(vv, EXTRA_VALUE); - return vv; - }), EXTRA_VALUE, description); - assertTrue(map.containsKey(null)); - assertSame(map.get(null), EXTRA_VALUE, description); + // check result + + switch(result) { + case NULL : + assertNull(returned, "wrong value"); + break; + case NEWVALUE : + assertSame(returned, VALUES[2], "wrong value"); + break; + case RESULT : + assertSame(returned, VALUES[3], "wrong value"); + break; + default: + fail("unexpected new value"); + } + + // check map + switch(put) { + case ABSENT : + assertFalse(map.containsKey(EXTRA_KEY), "key not absent"); + break; + case NULL : + assertTrue(map.containsKey(EXTRA_KEY), "key absent"); + assertNull(map.get(EXTRA_KEY), "wrong value"); + break; + case NEWVALUE : + assertTrue(map.containsKey(EXTRA_KEY), "key absent"); + assertSame(map.get(EXTRA_KEY), VALUES[2], "wrong value"); + break; + case RESULT : + assertTrue(map.containsKey(EXTRA_KEY), "key absent"); + assertSame(map.get(EXTRA_KEY), VALUES[3], "wrong value"); + break; + default: + fail("unexpected new value"); + } } @Test(dataProvider = "Map rw=true keys=all values=all") - public void testMerge(String description, Map map) { - assertTrue(map.containsKey(KEYS[1])); - Object value = map.get(KEYS[1]); - assertTrue(null == value || value == VALUES[1], description + String.valueOf(value)); - assertSame(map.merge(KEYS[1], EXTRA_VALUE, (v, vv) -> { - assertSame(v, value); - assertSame(vv, EXTRA_VALUE); - return vv; - }), EXTRA_VALUE, description); - assertSame(map.get(KEYS[1]), EXTRA_VALUE, description); - assertNull(map.merge(KEYS[1], EXTRA_VALUE, (v, vv) -> { - assertSame(v, EXTRA_VALUE); - assertSame(vv, EXTRA_VALUE); - return null; - }), description); - assertFalse(map.containsKey(KEYS[1])); - - assertFalse(map.containsKey(EXTRA_KEY)); - assertSame(map.merge(EXTRA_KEY, EXTRA_VALUE, (v, vv) -> { - assertNull(v); - assertSame(vv, EXTRA_VALUE); - return EXTRA_VALUE; - }), EXTRA_VALUE); - assertTrue(map.containsKey(EXTRA_KEY)); - assertSame(map.get(EXTRA_KEY), EXTRA_VALUE); + public void testMergeNullMerger(String description, Map map) { + assertThrows( () -> { map.merge(KEYS[1], VALUES[1], null);}, + NullPointerException.class, + "Should throw NPE"); } - @Test(expectedExceptions = {NullPointerException.class}) - public void testMergeNPEHashMap() { - Object value = new HashMap().merge(KEYS[1], VALUES[1], null); - } - - @Test(expectedExceptions = {NullPointerException.class}) - public void testMergeNPEHashtable() { - Object value = new Hashtable().merge(KEYS[1], VALUES[1], null); - } - - @Test(expectedExceptions = {NullPointerException.class}) - public void testMergeNPETreeMap() { - Object value = new TreeMap().merge(KEYS[1], VALUES[1], null); - } - - enum IntegerEnum { + public enum IntegerEnum { e0, e1, e2, e3, e4, e5, e6, e7, e8, e9, e10, e11, e12, e13, e14, e15, e16, e17, e18, e19, @@ -715,6 +707,89 @@ public class Defaults { return result; } + static class Merging { + public enum Value { + ABSENT, + NULL, + OLDVALUE, + NEWVALUE, + RESULT + } + + public enum Merger implements BiFunction { + UNUSED { + public String apply(String oldValue, String newValue) { + fail("should not be called"); + return null; + } + }, + NULL { + public String apply(String oldValue, String newValue) { + return null; + } + }, + RESULT { + public String apply(String oldValue, String newValue) { + return VALUES[3]; + } + }, + } + } + + @DataProvider(name = "MergeCases", parallel = true) + public Iterator mergeCasesProvider() { + Collection cases = new ArrayList<>(); + + cases.addAll(makeMergeTestCases()); + cases.addAll(makeMergeNullValueTestCases()); + + return cases.iterator(); + } + + static Collection makeMergeTestCases() { + Collection cases = new ArrayList<>(); + + for( Object[] mapParams : makeAllRWMaps() ) { + cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.ABSENT, Merging.Value.NEWVALUE, Merging.Merger.UNUSED, Merging.Value.NEWVALUE, Merging.Value.NEWVALUE }); + } + + for( Object[] mapParams : makeAllRWMaps() ) { + cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.OLDVALUE, Merging.Value.NEWVALUE, Merging.Merger.NULL, Merging.Value.ABSENT, Merging.Value.NULL }); + } + + for( Object[] mapParams : makeAllRWMaps() ) { + cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.OLDVALUE, Merging.Value.NEWVALUE, Merging.Merger.RESULT, Merging.Value.RESULT, Merging.Value.RESULT }); + } + + return cases; + } + + static Collection makeMergeNullValueTestCases() { + Collection cases = new ArrayList<>(); + + for( Object[] mapParams : makeAllRWMapsWithNulls() ) { + cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.OLDVALUE, Merging.Value.NULL, Merging.Merger.NULL, Merging.Value.ABSENT, Merging.Value.NULL }); + } + + for( Object[] mapParams : makeAllRWMapsWithNulls() ) { + cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.OLDVALUE, Merging.Value.NULL, Merging.Merger.RESULT, Merging.Value.RESULT, Merging.Value.RESULT }); + } + + for( Object[] mapParams : makeAllRWMapsWithNulls() ) { + cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.ABSENT, Merging.Value.NULL, Merging.Merger.UNUSED, Merging.Value.ABSENT, Merging.Value.NULL }); + } + + for( Object[] mapParams : makeAllRWMapsWithNulls() ) { + cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.NULL, Merging.Value.NULL, Merging.Merger.UNUSED, Merging.Value.ABSENT, Merging.Value.NULL }); + } + + for( Object[] mapParams : makeAllRWMapsWithNulls() ) { + cases.add(new Object[] { mapParams[0], mapParams[1], Merging.Value.NULL, Merging.Value.NEWVALUE, Merging.Merger.UNUSED, Merging.Value.NEWVALUE, Merging.Value.NEWVALUE }); + } + + return cases; + } + public interface Thrower { public void run() throws T;