Hi Mark,
On 27/05/2020 2:15 am, Mark Kralj-Taylor wrote:
David,
Thanks for taking this enhancement, and making it work on the older
glibc (pre 2.17) Linux platforms currently supported by openjdk.
I like that it is a small change to split the JVM startup check on
availability of Posix clock_gettime/getres() APIs and then if
additionally CLOCK_MONOTONIC is supported.
Glad to hear it. I had originally also envisaged something more complex
so the simplicity here is appealing. Hopefully for 16 (or else 17) we
can get rid of all the legacy support.
On Daniel's comment on the spec. Yes the nice part of this is that the
JavaDoc on Clock.systemUTC() is worded to allow higher clock resolution
than system.currentTimeMillis().
Please let me know if I can help any more on this.
I think we are set. Your very detailed write-up and the changes to the
test and benchmark were a great assist. You've set a very high bar on
your first OpenJDK contribution. :)
Now I just need someone from hotspot-runtime to sign-off on the VM side.
Cheers,
David
Mark
On Tue, 26 May 2020 at 16:35, Daniel Fuchs <daniel.fu...@oracle.com
<mailto:daniel.fu...@oracle.com>> wrote:
Hi David,
This is not a review for the posix code.
Your webrev looks good to me and corresponds to what I expected
to see. I understand that not all operating systems / platforms
are expected to have the nano second precision, so your test
probably can't go much beyond what is currently being tested.
Last time I upgraded the system clock to micro second precision [1]
I had to write a CSR and release notes. That was the first time
the clock went beyond millisecond precision however - and my
expectation is that your proposed change should no longer
require a CSR as potential nanosecond precision should
now be covered by the spec.
I had to modify a few other places as well (see [1]) - where precision
greater than 1ms was not expected, but these modifications
should cover your current changes too, so I don't think anything
more is required. Some --test-repeat might be in order
to verify things are stable but I don't expect any issue there :-)
LGTM.
best regards,
-- daniel
[1] https://bugs.openjdk.java.net/browse/JDK-8068730
On 26/05/2020 05:59, David Holmes wrote:
> bug: https://bugs.openjdk.java.net/browse/JDK-8242504
> webrev: http://cr.openjdk.java.net/~dholmes/8242504/webrev/
>
> This work was contributed by Mark Kralj-Taylor:
>
>
https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-April/038975.html
>
>
> On the hotspot side we change the Linux implementations of
> javaTimeMillis() and javaTimeSystemUTC() so that they use
> clock_gettime(CLOCK_REALTIME) instead of gettimeofday(). In
keeping with
> our conditional use of this API I added a new guard
>
> os::Posix::supports_clock_gettime()
>
> and refined the existing supports_monotonic_clock() so that we
can still
> use CLOCK_REALTIME if CLOCK_MONOTONIC does not exist. In the future
> (hopefully very near future) we will simply assume these APIs
always exist.
>
> On the core-libs side the existing test:
>
> test/jdk/java/time/test/java/time/TestClock_System.java
>
> is adjusted to track the precision in more detail.
>
> Finally Mark has added instantNowAsEpochNanos() to the benchmark
> previously known as
>
> test/micro/org/openjdk/bench/java/lang/Systems.java
>
> which I've renamed to SystemTime.java as Mark suggested. I agree
having
> these three functions measured together makes sense.
>
> Testing:
> - test/jdk/java/time/test/java/time/TestClock_System.java
> - test/micro/org/openjdk/bench/java/lang/SystemTime.java
> - Tiers 1-3
>
> Thanks,
> David
