[9] Review request for 8140503: JavaFX "Pair" Hash Code Collisions

[Chien Yang]

 > The key in Pair could actually be null and in this case hashCode will 
throw NPE.

This might be a good enough reason to have this fix.

- Chien

On 11/3/15, 12:07 PM, Vadim Pakhnushev wrote:
> Hmm, yeah, the actual difference is in the prime number only (that is 
> changing the algorithm only doesn't improve anything), so the only 
> remaining reason to fix this is that Objects.hash guards against null 
> values (and I forgot to mention it in the review).
> The key in Pair could actually be null and in this case hashCode will 
> throw NPE.
>
> Vadim
>
> On 03.11.2015 23:01, Vadim Pakhnushev wrote:
>> Well, not exactly... Previously it was 13*hash(a) + hash(b) and now 
>> it's 31*(31 + hash(a)) + hash(b).
>> And apparently it improves the quality somehow. I did a test with 
>> 100^4 combinations and collision probability dropped by the factor of 
>> 3 from 0.065% to 0.022%.
>> Not really impressive, but still, and it uses well-defined utility 
>> method.
>> Yeah, I know it's not really a bug since you don't want to rely on 
>> the hashCode at all...
>>
>> Thanks,
>> Vadim
>>
>> On 03.11.2015 22:35, Jim Graham wrote:
>>> All this does is change the prime constant used to produce the hash 
>>> value.
>>>
>>> Objects.hash(a, b) uses 31*hash(a) + hash(b) instead of the 
>>> 13*hash(a) + hash(b) that the embedded implementation uses.
>>>
>>> I don't really think this is a bug.  The fact that Integer objects 
>>> make it easy to reverse engineer and compute collisions of any 
>>> reasonable hash combination computation don't mean that the 
>>> technique has a bug, it just means that the submitter can read the 
>>> code and think of a counter-example.
>>>
>>> If there are practical problems being caused for some particular and 
>>> popular use case by the use of this particular constant "13", then 
>>> we need to understand those issues and come up with a more 
>>> comprehensive solution than to simply hand off to another mechanism 
>>> which uses the same procedure with a different prime constant...
>>>
>>>             ...jim
>>>
>>> On 11/3/15 3:06 AM, Vadim Pakhnushev wrote:
>>>> Hi Chien,
>>>>
>>>> Could you please review the fix:
>>>> https://bugs.openjdk.java.net/browse/JDK-8140503
>>>> http://cr.openjdk.java.net/~vadim/8140503/webrev.00/
>>>>
>>>> Thanks,
>>>> Vadim
>>
>