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
