Hi Mike,
Another thing to note: Some new methods in HashMap need to call
inflateTable(), since patch for 8011200 has already been pushed to tl...
Regards, Peter
On 04/14/2013 08:35 PM, Peter Levart wrote:
On 04/14/2013 07:54 PM, Peter Levart wrote:
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
I think the most optimal is actually this:
default boolean remove((Object key, Object value) {
Object curValue = get(key);
if (curValue == null && (value != null || !containsKey(key)) ||
curValue != value && !curValue.equals(value)) {
return false;
}
remove(key);
return true;
}
...and the same for replace(key, oldValue, newValue)...
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
