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.lev...@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.lev...@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 >>> >
