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

[Stephen Colebourne]

On the LocalDateTime being passed in with nanoseconds, that was an
unconsidered use case. The whole offset system relies on second based
offsets, so it should really be validated/truncated to remove nanos.
That way the equals/compareTo could be simplified again. Seems like a
separate issue, but perhaps could be tackled here. You need an Oracle
sponsor to tell you ;-)

Stephen


On 29 April 2015 at 10:33, Peter Levart <peter.levart at gmail.com> 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
>>>
>