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

[Alexander Kouznetsov]

Moreover, the following two sentences:

"However, this is an incorrect way to compute a hash code of two values."
"This can lead to hard-to-find bugs anywhere that instances of Pair are 
used in a data structure like a HashSet or HashTable."

seem to indicate misunderstanding of what hashcode is and how it is to 
be used.

Best regards,
Alexander Kouznetsov
(408) 276-0387

On 3 ноя 2015 13:42, Alexander Kouznetsov wrote:
> After the fix, you should expect another incident report of
>
> Objects.hash(1, 0) == Objects.hash(0, 31)
>
> always true :-)
>
> I'd rather file another bug on key == null causing NPE and closing 
> this one as incomplete or not an issue.
>
> Best regards,
> Alexander Kouznetsov
> (408) 276-0387
>
> On 3 ноя 2015 12:07, 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
>>>
>>
>