RFR : 8016446 : (m) Add override forEach/replaceAll to HashMap, Hashtable, IdentityHashMap, WeakHashMap, TreeMap

[Mike Duigou]

I certainly have been curious as to how well our defaults work for the non-OpenJDK collections implementations but unfortunately haven't had time to pursue testing them yet. We plan to run a broader selection of software with JDK8 as we progress to release and testing with Guava, Apache Collections and Goldman Sachs collections are certainly part of that. In the meantime we are definitely interested in hearing any reports of issues or performance concerns.

Mike

On Jun 19 2013, at 00:08 , Peter Levart wrote:

> On 06/19/2013 08:44 AM, Peter Levart wrote:
>> On 06/19/2013 01:13 AM, Mike Duigou wrote:
>>> On Jun 18 2013, at 05:19 , Doug Lea wrote:
>>> 
>>>> On 06/17/13 19:30, Mike Duigou wrote:
>>>> 
>>>>> I had to add the improved default for ConcurrentMap which was present in the lambda repo in order to have correct behaviour. Since getOrDefault is already in ConcurrentMap I will include this but we have to be careful when we do a jsr 166 syncup to make sure that the defaults don't get lost.
>>>>> 
>>>> Now synched up on my side.
>>>> 
>>>> -Doug
>>>> 
>>> 
>>> 
>>> Per a suggestion from Remi I updated the ConcurrentMap.replaceAll default to use forEach. This trades off the entrySet iterator overhead for creation of a capturing BiConsumer lambda.
>>> 
>>> http://hg.openjdk.java.net/jdk8/tl/jdk/raw-diff/1f7cbe4829fe/src/share/classes/java/util/concurrent/ConcurrentMap.java 
>>> 
>>> Mike
>> 
>> Hi Mike, Remi,
>> 
>> Since forEach implementation can be taken from default Map.forEach in some implementations of ConcurrentMap, and that implementation is based on entrySet Iterator, isn't it dangerous for this to trigger ConcurrentModificationException in some implementation of ConcurrentMap? I see nothing in the spec. of ConcurrentMap that suggests it's entrySet iterators are never fail-fast. They can be prepared for modifications from other threads (synchronization), but may not tolerate re-entrant calls.
>> 
>> For example some implementation of (Concurrent)Map could be structurally modified as a result of Map.replace(key, old, new) - imagine a ConcurrentWeakHashMap that expunges stale entries on each call - and forEach iteration may not be prepared to handle such situations.
>> 
>> Regards, Peter
>> 
> 
> Hi Mike,
> 
> I'd also like to suggest something. You have made lots of tests that cover the functionality of new default methods in Map and other collections interfaces which prove the correctness of behaviour when used with implementations of those interfaces in JDK. Perhaps It would be wise to extend those tests to include some JDK-external implementations that are known to pass basic tests, taken for example from Guava or Apache Commons or even JDK7. The interactions of default methods with existing methods in various implementations of collection interfaces could reveal "bugs" that may be on the part of default methods. I'm sure the maintainers of these external implementations will do the same, but may be doing that only after JDK8 release.
> 
> Regards, Peter
>