RFR 8068730: Increase the precision of the implementation of java.time.Clock.systemUTC()

[Stephen Colebourne]

Well that is a nice surprise ;-) And I think the implementation via an
adjustment is very sensible.

In java.time.Instant, a readObject() has been added. However, I don't
believe this will ever be called because Instant has a writeReplace()
method and so is not deserialized. (There may be some security related
"evil serialization stream" reason why readObject() should exist, I
can't say).

In java.util.Formatter, there should be no reason to retain use of the
MILLI_OF_SECOND, as any TemporalAccessor that can return
NANO_OF_SECOND should also return MILLI_OF_SECOND, however the spec is
not quite tight enough to guarantee that sadly. As such, I think the
catch will have to be retained.

TestFormatter has some commented out System.out statements that should
probably be removed.


I suspect that this change will break some user code, but it certainly
doesn't break the spec. As such, I think the change should go in.

I do believe that this change means that a new method should be added
to Clock however:

    public static Clock tickMillis(ZoneId zone) {
        return new TickClock(system(zone), NANOS_PER_MILLI);
    }

While this can be achieved without a new method, the API would feel
slightly strange without it now better-than-milli clocks exist. I
recommend raising a separate RFE to track this.

Stephen



On 9 January 2015 at 16:56, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> Hi,
>
> Please find below a webrev for:
>
> 8068730: Increase the precision of the implementation
>          of java.time.Clock.systemUTC()
> https://bugs.openjdk.java.net/browse/JDK-8068730
>
> The java.time.Clock.system() method (and variants thereof) are
> specified to "obtain a clock that returns the current instant
> using best available system clock". However the current
> implementation of the clock returned is based on
> System.currentTimeMillis() whereas the underlying native clock
> used by System.currentTimeMillis() has often a greater precision
> than milliseconds (for instance, on Linux, System.currentTimeMillis()
> is based on gettimeofday, which offers microseconds precision).
>
> This patch enhances the implementation of the
> java.time.Clock.SystemClock, so that it offer at least the
> same precision than the underlying clock available on the system.
>
> There is no change in the public API.
>
> http://cr.openjdk.java.net/~dfuchs/webrev_8068730/webrev.00/
>
> Some more details on the patch:
>
> native (hotspot) side:
>
>  - adds a new method to the os class:
>    os::javaTimeSystemUTC(jlong &seconds, jlong &nanos)
>    which allows to get the time in the form of a number
>    of seconds and number of nanoseconds
>    (see os.hpp and various os_<os>.cpp files)
>
>  - adds a JVM_GetNanoTimeAdjustment method, which takes
>    an offset (in seconds) as parameter and returns a
>    nano time adjustment compared to the offset.
>    Calls os::javaTimeSystemUTC to compute the adjustment
>    (see jvm.cpp)
>
> java (jdk) side:
>
>  - adds a native sun.misc.VM.getNanoTimeAdjustment method
>    (which is bound to JVM_GetNanoTimeAdjustment)
>    (see VM.java and VM.c)
>
>  - modifies java.time.Clock.SystemClock to call the new
>    sun.misc.VM.getNanoTimeAdjustment instead of
>    System.currentTimeMillis.
>
>  - fixes java.util.Formatter - which wasn't expecting
>    greater than millisecond precision.
>
> testing:
>
>  - A new test is added to test sun.misc.VM.getNanoTimeAdjustment
>    In particular it checks the edge cases.
>  - New tests are added to TestClock_System.java to check for
>    the increased precision.
>    There is also a test for the edge cases there.
>  - Some java.time tests where tripped by the increased precision,
>    and had to be modified to take that into account
>
> Note: comparing the new os::javaTimeSystemUTC with os::javaTimeMillis
>       can help in reviewing the changes.
>
> best regards,
>
> -- daniel