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

[Peter Levart]

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