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

[Daniel D. Daugherty]

On 5/26/20 12:59 AM, David Holmes wrote:
> bug: https://bugs.openjdk.java.net/browse/JDK-8242504
> webrev: http://cr.openjdk.java.net/~dholmes/8242504/webrev/

src/hotspot/os/posix/os_posix.hpp
     No comments.

src/hotspot/os/posix/os_posix.inline.hpp
     No comments.

src/hotspot/os/posix/os_posix.cpp
     old L1686: #ifdef NEEDS_LIBRT
     old L1687       // Close librt if there is no monotonic clock.
     old L1688       if (handle != RTLD_DEFAULT) {
     old L1689         dlclose(handle);
     old L1690       }
     old L1691 #endif
         I don't see any explanation in the bug or in the CR invite
         about why this code is deleted when this preceding code
         remains:

         L1658:   // For older linux we need librt, for other OS we can find
         L1659:   // this function in regular libc.
         L1660: #ifdef NEEDS_LIBRT
         L1661:   // We do dlopen's in this particular order due to bug 
in linux
         L1662:   // dynamic loader (see 6348968) leading to crash on exit.
         L1663:   handle = dlopen("librt.so.1", RTLD_LAZY);
         L1664:   if (handle == NULL) {
         L1665:     handle = dlopen("librt.so", RTLD_LAZY);
         L1666:   }
         L1667: #endif

src/hotspot/os/linux/os_linux.cpp
     old L1382:   return jlong(time.tv_sec) * 1000  + jlong(time.tv_usec 
/ 1000);
     new L1390:     return jlong(time.tv_sec) * MILLIUNITS  +
     new L1391:            jlong(time.tv_usec) / (MICROUNITS / MILLIUNITS);

     old L1390:   nanos = jlong(time.tv_usec) * 1000;
     new L1407:     nanos = jlong(time.tv_usec) * (NANOUNITS / MICROUNITS);
         Thanks for replacing the literal 1000s in the old code.

test/jdk/java/time/test/java/time/TestClock_System.java
     L207:                                + " millisecond precision: 
"+countBetterThanMillisPrecision+"/"+1000);
     L209:                                + " microsecond precision: 
"+countBetterThanMicrosPrecision+"/"+1000);
        nit - need spaces around some of the '+' operators.

test/micro/org/openjdk/bench/java/lang/SystemTime.java 
test/micro/org/openjdk/bench/java/lang/Systems.java
     No comments.

My only "concern" is the deletion of closing the librt handle.
If you have a good explanation for that, then I'm good with this patch.

Dan



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