(10) (M) RFR: 8174231: Factor out and share PlatformEvent and Parker code for POSIX systems (original) (raw)

Robbin Ehn robbin.ehn at oracle.com
Fri May 19 08:36:29 UTC 2017

Hi David,

On 05/18/2017 08:25 AM, David Holmes wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8174231

webrevs: Build-related: http://cr.openjdk.java.net/~dholmes/8174231/webrev.top/ hotspot: http://cr.openjdk.java.net/~dholmes/8174231/webrev.hotspot/

I like this, with neg delta of 700 loc, nice!

It's hard to see if you broken anything, since you combined 4 separate implementation into 1. I guess you have tested this proper?

What stands out in os_posix.cpp is the static void to_abstime(timespec* abstime, jlong timeout, bool isAbsolute)

The ifdef scopes of SUPPORTS_CLOCK_MONOTONIC is large and calculations are repeated 3 times.

Please consider something like:

#ifdef SUPPORTS_CLOCK_MONOTONIC if (_use_clock_monotonic_condattr && !isAbsolute) { // Why aren't we using this when not isAbsolute is set? // I suggest removing that check from this if and use monotonic for that also. struct timespec now; int status = _clock_gettime(CLOCK_MONOTONIC, &now); assert_status(status == 0, status, "clock_gettime"); calc_time(abstime, timeout, isAbsolute, now.tv_sec, now.tv_nsec, NANOUNITS); } else { #else { #endif struct timeval now; int status = gettimeofday(&now, NULL); assert(status == 0, "gettimeofday"); calc_time(abstime, timeout, isAbsolute, now.tv_sec, now.tv_usec, MICROUNITS); } #endif

Thanks for fixing this!

/Robbin

More information about the hotspot-dev mailing list