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