RFR(S) 8242504: Enhance the system clock to nanosecond precision

[David Holmes]

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.fuchs at oracle.com 
> <mailto:daniel.fuchs at 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
>