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

[Roger Riggs]

Hi Stephen, Peter,

I think we should clarify the constructor to indicate that nanoseconds 
are truncated/ignored.
That should be done as a separate request since it is a spec change and 
needs additional review.

Roger


On 4/29/2015 5:43 AM, Stephen Colebourne wrote:
> 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
>>>>