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

[David Holmes]

Hi Daniel,

Looking at the hotspot part ...

On 10/01/2015 2:56 AM, Daniel Fuchs 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)

Everything seems consistent with javaTimeMillis code. Only nit on 
Windows is that windows_time_stamp isn't a very descriptive name (no 
idea what it returns) - perhaps windows_time_as_micros and adjust the 
calculations accordingly?

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

IIUC JVM_GetNanoTimeAdjustment(jlong offset_secs) returns the difference 
in nanoseconds between current UTC time (from Epoch) and the UTC time 
since the Epoch that offset_secs represents - is that right? I don't 
find the name particularly intuitive and I guess you renamed it 
somewhere along the way as the mapfiles are no longer in alphabetical 
order (but would be if this was JVM_GetTimeXXXX). So please fix mapfiles.

> java (jdk) side:
>
>   - adds a native sun.misc.VM.getNanoTimeAdjustment method
>     (which is bound to JVM_GetNanoTimeAdjustment)
>     (see VM.java and VM.c)

Just curious: why the indirection through sun.misc.VM?

Cheers,
David

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