Hi Naoto,
That sounds like a test issue. I would not expect a cloned Calendar or
TimeZone to have a different hashCode.
None of the fields involved in the hashCode/equals should be different.
(Or I'm missing something about them).
Thanks, Roger
On 9/4/18 5:19 PM, Naoto Sato wrote:
Hi Roger,
I tentatively tried to return a shared zone within a cloned Calendar.
One test failed: java/util/Calendar/CalendarRegression.Test4136399,
where it makes sure that the cloned Calendar object hash code should
be different for the better distribution. Although the comment does
not reflect the current implementation (getTimeZone() returning cloned
zone), the intention here seems to have the better hash distribution
for cloned objects.
Naoto
On 9/4/18 1:41 PM, Roger Riggs wrote:
Hi Naoto,
That access via reflection is going to go away sometime; so I'm not
too concerned about
maintaining compatibility of the internal implementation.
I think I'd rather see the memory savings, however small.
Let see if anyone else has a recommendation.
Roger
On 9/4/18 4:12 PM, Naoto Sato wrote:
Hi Roger,
Yes, I considered that too, but did not change the behavior and just
to maintain the field consistent. I agree that it would not be
observable via the public Calendar API, but some apps (like how the
submitter found it) may be doing something using reflection.
Naoto
On 9/4/18 12:31 PM, Roger Riggs wrote:
Hi Naoto,
The spec for clone doesn't say whether the clone should share or
not share the TimeZone.
Did you consider that if sharedZone was true , *not* to clone the
TimeZone?
It would still get cloned when requested from getTimeZone().
This does seem somewhat safer not to change the cloning behavior
but I don't think the behavior would be observable.
The current code and test is fine, except for reducing the
potential for sharing the TimeZone.
Thanks, Roger
On 9/4/2018 2:14 PM, Naoto Sato wrote:
Hello,
Please review the fix to the following issue:
https://bugs.openjdk.java.net/browse/JDK-8210142
The proposed fix is located at:
http://cr.openjdk.java.net/~naoto/8210142/webrev.00/
The fix is a simple one line change, which is to make the
sharedZone field consistent with the cloned TimeZone instance in
Calendar.clone().
Naoto
