RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

Joe Wang huizhe.wang at oracle.com
Fri Aug 7 17:16:19 UTC 2020

Naoto, Roger,

Sorry have to come back with another iteration. It looks like the 
addition of getMillisecond() over the original implementation does 
change things.

In particular, getMillisecond() returns FIELD_UNDEFINED when 
fractionalSecond == null (or not specified). But the normalization 
process will set it to zero after a mathematics adjustment. Now this 
will fail when comparing with a datetime that does not need to be 
normalized and thereby still returns FIELD_UNDEFINED (FIELD_UNDEFINED = 
Integer.MIN_VALUE). This situation is demonstrated in tests 3 at line 56 
and 4 at line 58. 

The culprit is XMLGregorianCalendarImpl::normalizeToTimezone. In the 
original implementation, XMLGregorianCalendarImpl was calling the 
private normalizeToTimezone while XMLGregorianCalendar the public 
normalize(). The later will do the right thing to set the Millisecond 
back to UNDEFINED.

In webrev_03, XMLGregorianCalendarImpl now calls super.hashCode() since 
the above was the only difference between the hashCode() impls of 
XMLGregorianCalendarImpl and XMLGregorianCalendar.



On 8/6/20 1:51 PM, Joe Wang wrote:
> Thanks Naoto, Roger!  I'll prepare push based on version 01.
> Best,
> Joe
> On 8/6/20 1:29 PM, Roger Riggs wrote:
>> +1
>> On 8/6/20 4:18 PM, naoto.sato at oracle.com wrote:
>>> Hi Joe, thank you for looking it into further.
>>> Yes, I agree the chances of collision is very rare, as 
>>> sub-millisecond difference in two objects. So fine with the version 01.
>>> Naoto
>>> On 8/6/20 12:18 PM, Joe Wang wrote:
>>>> Thanks Naoto, Roger for the review!
>>>> For Naoto's concern about the equals method using EonAndYear and 
>>>> FractionalSecond, I did cut corners using the all int getXXX 
>>>> method. The rational was: it fulfills the equals-hashcode contract 
>>>> just fine, is consistent with the existing implementation, and 
>>>> concise. This API was introduced since 1.5, but I have yet to see 
>>>> any usage of EonAndYear, and very rarely FractionalSecond. The 
>>>> chances collisions occur due to these differences are very low.
>>>> But I understand your concern. To be precise or exact as equals(), 
>>>> we would need to call getFractionalSecond and EonAndYear. Here's 
>>>> what that would look like:
>>>> http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev_02/
>>>> To me, I like version 1 for the reasons above:
>>>> http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev_01/
>>>> What would you think?
>>>> Regards,
>>>> Joe
>>>> On 8/5/20 6:13 PM, naoto.sato at oracle.com wrote:
>>>>> Hi Joe,
>>>>> For the consistency with equals(), just wondering if the 
>>>>> sub-second element should be obtained with getFractionalSecond() 
>>>>> instead of getMillisecond(), as the equals() subsequently calls it 
>>>>> in internalCompare() method. Also should it call getEonAndYear() 
>>>>> appropriately for the year?
>>>>> Naoto
>>>>> On 8/5/20 4:37 PM, Joe Wang wrote:
>>>>>> Hello,
>>>>>> Please review a fix for the hashCode generation.
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8246816
>>>>>> webrev: http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev/
>>>>>> Thanks,
>>>>>> Joe

More information about the core-libs-dev mailing list