On Mon, 2011-10-24 at 10:34 +0200, Mark Wielaard wrote: > On Mon, 2011-10-24 at 10:24 +0200, Mark Wielaard wrote: > > On Mon, 2011-10-24 at 10:11 +0300, Pekka Enberg wrote: > > > @@ -345,7 +345,10 @@ public class HashMap<K, V> extends AbstractMap<K, V> > > > > > > while (e != null) > > > { > > > - if ((key.hashCode() == e.key.hashCode()) && equals(key, e.key)) > > > + int hash1 = key == null ? 0 : key.hashCode(); > > > + int hash2 = e.key == null ? 0 : e.key.hashCode(); > > > + > > > + if ((hash1 == hash2) && equals(key, e.key)) > > > { > > > e.access(); // Must call this for bookkeeping in > > > LinkedHashMap. > > > V r = e.value; > > > > Are you sure that is right? > > What about a key which isn't null but has a hashCode() of zero? > > Doh. That wouldn't be a problem, because that would still trigger the > equals(key, e.key) to be called... OK, I think this is the right fix.
Actually, one suggestion for a micro-optimization. The compiler might detect that hash1 only needs to be calculated once, but we can make that explicit by pushing the int hash1 = ...; line just before the while loop. Thanks, Mark