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

[Remi Forax]

On 06/13/2013 01:14 AM, Mike Duigou wrote:
> On Jun 12 2013, at 15:49 , Remi Forax wrote:
>
>> On 06/12/2013 11:23 PM, Mike Duigou wrote:
>>> Hello all;
>>>
>>> This patch adds optimized implementations of Map.forEach and Map.replaceAll to HashMap, Hashtable, IdentityHashMap, WeakHashMap, TreeMap
>>>
>>> http://cr.openjdk.java.net/~mduigou/JDK-8016446/0/webrev/
>>>
>>> The implementations traverse the map more efficiently than the default iterator implementation and sometimes avoid creation of transient Map.Entry instances. The fast-fail behaviour of the default implementation is retained.
>>>
>>> Mike
>> Hi Mike,
>> funnily I was writing HashMap.forEach/LinkedHashMap.forEach at the same time.
>> (you need also to override forEach in LinkedHashMap because the one you inherits from HashMap doesn't use the linked list of entries).
> I don't think we need to offer a guarantee of ordering for the forEach but using the linked list is probably more efficient.
>
>> My code is slightly different from yours because I've moved the cases where the item is a red/black tree out of the fast path
>> (the tree is created either if you are unlucky, if hashCode is badly written or if someone forge keys to have collisions)
> Yes, this is probably worthwhile
>
>> and does the modCount check for each element because a call to the consumer can change the underlying map
>> so you can not do a modCount check only at the end.
> Yes, mine wasn't fast fail enough. I am always torn between satisfying those who will moan at the per-element cost and those who will moan about the lack of fast-fail behaviour.

I've not tested (yet) but I'me pretty sure that Hotspot will move the 
modCount test out of the loop if it can see through the lambda that the 
map is not modified. So it's a win-win situation here :)

>
> Thank you for the improvements!
>
> Mike

cheers,
Rémi