On 12/01/2015 10:00 PM, Daniel Fuchs wrote:
On 12/01/15 03:20, David Holmes wrote:
Hi Daniel,
Looking at the hotspot part ...
On 10/01/2015 2:56 AM, Daniel Fuchs wrote:
Hi David,
[...]
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?
I'm not a big fan of the current name either.
I would gladly rename it. I did think of
windows_to_java_time_micros, but it actually returns tenth of
micros - so it would be lying...
Is there a good name for 'tenth of micros'?
hundreds of nanos? :)
Otherwise - I assume I could rename it into:
windows_to_java_time_max_precision
Would that be a better name?
Or would windows_time_as_tenth_of_micros be better?
The windows_ticks name seems adequate.
- 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?
Yes. Exactly.
I don't find the name particularly intuitive
Well - I borrowed the name from the java time API.
Instant has a factory method that takes a number of seconds
and a 'nanoAdjustement':
http://docs.oracle.com/javase/8/docs/api/java/time/Instant.html#ofEpochSecond-long-long-
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.
You have an eagle's eye ;-)
Yes - in my early prototype it was named JVM_GetTimeStamp - which was
not a great name.
Mapfiles fixed. Thanks for spotting that!
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?
In my early prototype I didn't want to add new .c files,
and so I looked for a home for this new method.
When I asked around whether adding it to sun.misc.VM would
be appropriate nobody seemed too shocked.
If you think it's better I could make it a private static
method in java.time.Clock - but then I'd have to change
the new unit test for that (GetNanoTimeAdjustment.java) to
use reflection (and would probably also need to add an
"initialize" native method to java.time.Clock as well).
I was just curious, I'm not aware of any general policy on the core-libs
side regarding placement of native methods.
Here is the new webrev with Stephen & your feedback included
http://cr.openjdk.java.net/~dfuchs/webrev_8068730/webrev.01/
(windows_time_stamp not renamed yet)
I checked the updated webrev.02 and everything looks good on the hotspot
side.
What are your plans for pushing this?
David
-----
Thanks a lot David!
-- daniel
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
