Review Request CR#7118743 : Alternative Hashing for String with Hash-based Maps

[Rémi Forax]

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

>
> Mike

Rémi