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. Thank you for the improvements! Mike