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

[Roger Riggs]

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


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
>>>
>