RFR: 8004518 & 8010122 : Default methods on Map (original) (raw)

Peter Levart peter.levart at gmail.com
Sun Apr 14 10:54:49 PDT 2013

Hi Mike,

Just a nit: The order of boolean sub-expressions in Map.replace(key, oldValue, newValue):

740 if (!containsKey(key) || !Objects.equals(get(key), oldValue))

...would be more optimal if reversed (like in Map.remove(key, value)).

Regards, Peter

On 04/13/2013 12:02 AM, Mike Duigou wrote:

I have updated the webrev with these changes and a few more.

merge() required an update to it's specification to correctly account for null values. http://cr.openjdk.java.net/~mduigou/JDK-8010122/5/webrev/ This version is currently undergoing final pre-integration testing. Unless additional problems are found or the testing fails this version will be integrated. Mike On Apr 12 2013, at 12:53 , Mike Duigou wrote:

Thanks for the corrections. I have incorporated all of these into the integration version of the patch.

On Apr 12 2013, at 12:50 , Akhil Arora wrote:

Hi Mike,

a few small things - UnmodifiableMap.forEach is missing Objects.requireNonNull(action); Added. EmptyMap.replaceAll(BiFunction) should just return instead of throwing UnsupportedOpEx particularly since EmptyList.replaceAll also returns silently after checking if function is null to fulfil the NPE contract I agree. Hashtable is missing a synchronized override of forEach added. CheckedMap.typeCheck(BiFunction) is missing from the patch (won't compile without it) Apparently I forgot a qrefresh before generating the webrev. I had this in my local copy as it's necessary. On 04/11/2013 01:03 PM, Mike Duigou wrote: Another revision incorporating primarily documentation feedback.

http://cr.openjdk.java.net/~mduigou/JDK-8010122/2/webrev/ I've also included the java.util.Collections overrides for the default methods. All of these are performance enhancements--the semantics were already correct because the defaults use only public methods. This is likely, modulo formatting of the source examples, the version that will be pushed to TL on Friday unless somebody squawks. Moving the current compute, computeIfPresent, computeIfAbsent and merge defaults to ConcurrentMap and replacing the current Map defaults with versions that throw ConcurrentModificationException will likely be resolved in a future issue if the EG determines it to be important. Mike

On Apr 10 2013, at 22:42 , Mike Duigou wrote: I've posted an updated webrev with the review comments I have received. http://cr.openjdk.java.net/~mduigou/JDK-8010122/1/webrev/ One important point to consider: - The current implementations of compute, computeIfPresent, computeIfAbsent, merge are implemented so that they can work correctly for ConcurrentMap instances. For non-concurrent implementations it might be better to fail and throw ConcurrentModification exception whenever concurrent modification is detected. For regular Map implementations the retry behaviour will only serve to mask errors. Thoughts? Mike On Apr 8 2013, at 11:07 , Mike Duigou wrote: Hello all; This is a combined review for the new default methods on the java.util.Map interface being added for the JSR-335 lambda libraries. The reviews are being combined because they share a common unit test. http://cr.openjdk.java.net/~mduigou/JDK-8010122/0/webrev/ 8004518: Add in-place operations to Map forEach() replaceAll() 8010122: Add atomic operations to Map getOrDefault() putIfAbsent() * remove(K, V) replace(K, V) replace(K, V, V) compute() * merge() * computeIfAbsent() * computeIfPresent() * The * operations treat null values as being absent. (ie. the same as there being no mapping for the specified key). The default implementations provided in Map are overridden in HashMap for performance purposes, in Hashtable for atomicity and performance purposes and in Collections for atomicity. Mike

More information about the lambda-dev mailing list