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

Reply via email to