RFR: JDK-8074003 java.time.zone.ZoneRules.getOffset(java.time.Instant) can be optimized

[Roger Riggs]

Hi Peter,

There should be two changesets; so pretend the truncation has been 
performed for this change.
It maybe useful to backport the performance improvement to jdk 8 but the 
spec change
will have to be in 9 (or wait for a maintenance release).

The simplification of toInstant can happen with the changeset for 8079063.

Thanks, Roger


On 4/29/2015 11:26 AM, Peter Levart wrote:
>
>
> On 04/29/2015 03:31 PM, Roger Riggs wrote:
>> Hi Peter,
>>
>> Point taken about the constructor and that should have a spec 
>> clarification to ignore the nanoseconds.
>> The issue is tracked with:
>> JDK-8079063 <https://bugs.openjdk.java.net/browse/JDK-8079063> 
>> ZoneOffsetTransition constructor should ignore nanoseconds
>>
>> With that, the compareTo method can be simpler.  The rest looks fine.
>>
>> Roger
>
> Hi Roger,
>
> Should I prepare a patch for both issues in one changeset as the 
> correct compareTo/equals depends on the truncation or should I just 
> pretend that truncation has been performed and make this change 
> accordingly or should I 1st do the JDK-8079063 and then this one on top?
>
> Also, getInstant() can be much simpler if the truncation is performed: 
> return Instant.of(epochSecond);
>
> Regards, Peter
>
>>
>>
>> On 4/29/2015 5:33 AM, Peter Levart wrote:
>>>
>>> On 04/27/2015 06:51 PM, Stephen Colebourne wrote:
>>>> One additional change is needed. The compareTo() method can rely on
>>>> the new epochSecond field as well.
>>>> Otherwise good!
>>>> Stephen
>>>
>>> Hi Stephen,
>>>
>>> LocalDateTime (transition) has nanosecond precision. It may be that 
>>> transitions loaded from file in ZoneRules only have second 
>>> precisions, but ZoneOffsetTransition is a public class with public 
>>> factory method that takes a LocalDateTime transition parameter, so I 
>>> think compareTo() can't rely on epochSecond alone. But epochSecond 
>>> can be used as optimization in compareTo() as well as equals():
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/ZoneOffsetTransition.epochSecond/webrev.03/ 
>>>
>>>
>>> An alternative to keeping epochSecond field in ZoneOffsetTransition 
>>> would be to keep a reference to Instant instead. Instant contains an 
>>> epochSecond field (as well as nanos) and could be used for both 
>>> toEpochSecond() and getInstant() methods.
>>>
>>> What do you think?
>>>
>>> It also occurred to me that serialization format of 
>>> ZoneOffsetTransition is not adequate currently as it looses 
>>> nanosecond precision.
>>>
>>> Regards, Peter
>>>
>>>>
>>>> On 27 April 2015 at 17:24, Peter Levart <peter.levart at gmail.com> 
>>>> wrote:
>>>>> Hi again,
>>>>>
>>>>> Here's another optimization to be reviewed that has been discussed 
>>>>> a while
>>>>> ago (just rebased from webrev.01) and approved by Stephen:
>>>>>
>>>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/ZoneOffsetTransition.epochSecond/webrev.02/ 
>>>>>
>>>>>
>>>>>
>>>>> The discussion about it is intermingled with the 
>>>>> ZoneId.systemDefault()
>>>>> discussion and starts about here:
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-February/031873.html 
>>>>>
>>>>>
>>>>>
>>>>> The rationale for the optimization is speeding-up the conversion 
>>>>> from epoch
>>>>> time to LocalDateTime. This conversion uses 
>>>>> ZoneRules.getOffset(Instant)
>>>>> where there is a loop over ZoneOffsetTransition[] array that 
>>>>> searches for
>>>>> 1st transition that has its toEpochSecond value less than the 
>>>>> Instant's
>>>>> epochSecond. This calls ZoneOffsetTransition.toEpochSecond 
>>>>> repeatedly,
>>>>> converting ZoneOffsetTransition.transition which is a 
>>>>> LocalDateTime to
>>>>> epochSecond. This repeated conversion is unnecessary, as
>>>>> ZoneOffsetTransition[] array is part of ZoneRules which is cached.
>>>>> Optimizing the ZoneOffsetTransition implementation (keeping both
>>>>> LocalDateTime variant and eposhSecond variant of transition time 
>>>>> as the
>>>>> object's state) speeds up this conversion.
>>>>>
>>>>>
>>>>> Regards, Peter
>>>>>
>>>
>>
>