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