Hashtable.Entry.hashCode() no longer conforms to Map.Entry.hashCode()

[Neil Richards]

Hi Mike,
Thanks for the swift appraisal.

Good suggestion to expand the test to cover other Map implementations -
I'd toyed with the idea, but hadn't spotted Collisions.java as a
template to follow.

I've posted an new webrev [2] with the updated (and moved) testcase,
which also makes use of Objects.hashCode() . 


Might there be a performance impact of using Objects.hashCode() in the
fix itself, rather than having the logic inline?

On the surface, it looks like it would be an extra method call, but
maybe the JIT would iron that out?

(I fret about this as (unnecessary) impacts in Hashtable performance
tend to widely felt).

I notice that the other Map.Entry implementations (that I've looked at
so far) have this logic inlined, which is one reason why I went that
route.


Thanks for raising 8000956 for the Java 7 backport.
I guess this means you think the fix should get back there fairly
swiftly.

Presumably 8000956 is linked to 8000955, so I would take the same
change, with the same commit comment - crucially using the same,
original bug id, 8000955 - back onto jdk7u-dev ?

(This is what I've done for other items I've helped in taking back to
jdk7u-dev. Using the same bug id makes it easier to track changes that
have been applied across different streams).

I'm guessing I also need to raise this on the jdk7u-dev mailing list and
get the nod (from Sean Coffey) to put it back there ?


Prior to this, I think I need a blessing for this change from a jdk8
"Reviewer" to push the change to jdk8/tl.


Regards,
Neil

[2] http://cr.openjdk.java.net/~ngmr/8000955/webrev.02

On Mon, 2012-10-15 at 18:48 -0700, Mike Duigou wrote:
> Good find Neil. 
> 
> I have opened JDK-8000955 for this issue along with JDK-8000956 for the Java 7 backport.
> 
> The code could be simplified by using Objects.hashCode()
> 
> ie.
> 
> return Objects.hashCode(key) ^ Objects.hashCode(value);
> 
> Any objection to extending the test similar to Collisions.java to test other Map classes?
> 
> Mike
> 
> On Oct 15 2012, at 18:29 , Neil Richards wrote:
> 
> > Hi,
> > I notice that the behaviour of java.util.Hashtable.Entry.hashCode() no
> > longer conforms to the defined behaviour (in the Java API Javadoc [1])
> > for java.util.Map.Entry.hashCode() implementations.
> > 
> > The code in Hashtable.Entry.hashCode() assumes that the value in
> > Hashtable.Entry.hash will always be the same as that for
> > Hashtable.Entry.getKey().hashCode() .
> > 
> > However, since Java bug 7126277 (Alternative String hashing
> > implementation), the use of Hashtable.hashSeed, a randomizing factor,
> > has been introduced into the calculation of Hashtable.hash().
> > 
> > It is the result from Hashtable.hash() which ends up stored in the
> > Hashtable.Entry.hash field.
> > 
> > So the assumption made in Hashtable.Entry.hashCode() is no longer valid,
> > and the code needs to be corrected, so that it once more complies with
> > the Java API defined behaviour.
> > 
> > 
> > I have created a webrev [2] with my suggested fix for this problem,
> > together with a simple testcase to demonstrate it.
> > 
> > (I've also checked the other Map implementations modified by 7126277,
> > and verified that the others still conform to the Java API in this
> > regard).
> > 
> > Please review the problem and suggested fix, and let me know your
> > thoughts.
> > 
> > I am not aware of an existing Java bug for this issue. 
> > Provided you agree with my analysis, could one be raised to allow the
> > fix to be committed?
> > 
> > Thanks,
> > Neil
> > 
> > [1] http://docs.oracle.com/javase/7/docs/api/java/util/Map.Entry.html#hashCode%28%29
> > [2] http://cr.openjdk.java.net/~ngmr/hashtable-entry-hashcode-fix/webrev.00
> > 
> > -- 
> > Unless stated above:
> > IBM email: neil_richards at uk.ibm.com
> > IBM United Kingdom Limited - Registered in England and Wales with number 741598.
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
> > 
> 


-- 
Unless stated above:
IBM email: neil_richards at uk.ibm.com
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU