Review Request CR#7118743 : Alternative Hashing for String with Hash-based Maps (original) (raw)

Mike Duigou mike.duigou at oracle.com
Thu May 31 00:59:29 UTC 2012

On May 30 2012, at 17:59 , Rémi Forax wrote:

On 05/30/2012 06:23 PM, Mike Duigou wrote:

On May 30 2012, at 07:38 , Rémi Forax wrote:

On 05/30/2012 01:05 AM, Mike Duigou wrote: Another round of updates for Java 7 [1] and Java 8 [2] implementations. This revision incorporates Remi's suggestions and some feedback from Doug Lea regarding applying the per-instance seed to the result of String.hash32()

[1] althashing "7" webrev : http://cr.openjdk.java.net/~mduigou/althashing7/10/webrev/ [2] althashing "8" webrev : http://cr.openjdk.java.net/~mduigou/althashing8/10/webrev/ Barring any emergencies this will be integrated to both Java 7 and Java 8 on Wednesday May 30th, 2012. Thanks to all who provided feedback! Mike Hi Mike, I've still trouble to understand why you want to expose something which is not used. We know that the implementation of String.hashCode() is broken but we can't change it because the implementation is part of the spec. We can't do an instanceof check on an interface in the implementations of Map because the VM implementations (not only Hotspot by the way) are not able to transform it to a quick class check. String.hash32 is not a polymorphic method but just a method used to fix the fact that we can't change the implementation of String.hashCode() so there is no need for a interface like Hashable32. as a proof, as Ulf said, String even doesn't implement Hashable32. It's a mistake that String doesn't implement Hashable32 in the java 8 patch. That line somehow got lost with multiple revisions. (most likely due to constant merge conflicts with the String offset/count patch) I will restore it. The current proposal for Java 8: - A new interface Hashable32 is introduced. - Hashable32 provides a method hash32() - String implements Hashable32 and hash32() method - HashMap et al recognize String and invoke hash32() rather than hashCode() - ** End of current implementation ** - When the mainline javac supports extensions methods the implementation will be extended. - Add extension method to Hashable32.hash32() that calls hashCode() - Object implements Hashable32 [controversial idea] - HashMap et al unconditionally call hash32(). Other JDK code may also switch to call hash32() instead of hashCode() this item is controversial too because HashMap specifies that it use hashCode() not hash32. Anyway, Hashable32 is not needed for this patch and can be introduced later without any problem. So I prefer to postpone the introduction of Hashable32 until we will be able to see if it's the a good idea or not

Fair enough. I am eager to get this patch in so I will remove it if that is the only remaining objection.

Mike

More information about the core-libs-dev mailing list