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
