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

[Daniel D. Daugherty]

On 5/26/17 1:27 AM, David Holmes wrote:
> Robbin, Dan,
>
> Below is a modified version of the refactored to_abstime code that 
> Robbin suggested.
>
> Robbin: there were a couple of issues with your version. For relative 
> time the timeout is always in nanoseconds - the "unit" only tells you 
> what form the "now_part_sec" is - nanos or micros. And the 
> calc_abs_time always has a deadline in millis. So I simplified and did 
> a little renaming, and tracked max_secs in debug_only instead of 
> returning it.
>
> Please let me know what you think.

Looks OK to me. Nit comments below...


>
> Thanks,
> David
> -----
>
>
> // Calculate a new absolute time that is "timeout" nanoseconds from 
> "now".
> // "unit" indicates the unit of "now_part_sec" (may be nanos or micros 
> depending
> // on which clock is being used).
> static void calc_rel_time(timespec* abstime, jlong timeout, jlong 
> now_sec,
>                           jlong now_part_sec, jlong unit) {
>   time_t max_secs = now_sec + MAX_SECS;
>
>   jlong seconds = timeout / NANOUNITS;
>   timeout %= NANOUNITS; // remaining nanos
>
>   if (seconds >= MAX_SECS) {
>     // More seconds than we can add, so pin to max_secs.
>     abstime->tv_sec = max_secs;
>     abstime->tv_nsec = 0;
>   } else {
>     abstime->tv_sec = now_sec  + seconds;
>     long nanos = (now_part_sec * (NANOUNITS / unit)) + timeout;
>     if (nanos >= NANOUNITS) { // overflow
>       abstime->tv_sec += 1;
>       nanos -= NANOUNITS;
>     }
>     abstime->tv_nsec = nanos;
>   }
> }
>
> // Unpack the given deadline in milliseconds since the epoch, into the 
> given timespec.
> // The current time in seconds is also passed in to enforce an upper 
> bound as discussed above.
> static void unpack_abs_time(timespec* abstime, jlong deadline, jlong 
> now_sec) {
>   time_t max_secs = now_sec + MAX_SECS;
>
>   jlong seconds = deadline / MILLIUNITS;
>   jlong millis = deadline % MILLIUNITS;
>
>   if (seconds >= max_secs) {
>     // Absolute seconds exceeds allowed max, so pin to max_secs.
>     abstime->tv_sec = max_secs;
>     abstime->tv_nsec = 0;
>   } else {
>     abstime->tv_sec = seconds;
>     abstime->tv_nsec = millis * (NANOUNITS / MILLIUNITS);
>   }
> }
>
>
> static void to_abstime(timespec* abstime, jlong timeout, bool 
> isAbsolute) {

There's an extra blank line here.

>
>   DEBUG_ONLY(int max_secs = MAX_SECS;)
>
>   if (timeout < 0) {
>     timeout = 0;
>   }
>
> #ifdef SUPPORTS_CLOCK_MONOTONIC
>
>   if (_use_clock_monotonic_condattr && !isAbsolute) {
>     struct timespec now;
>     int status = _clock_gettime(CLOCK_MONOTONIC, &now);
>     assert_status(status == 0, status, "clock_gettime");
>     calc_rel_time(abstime, timeout, now.tv_sec, now.tv_nsec, NANOUNITS);
>     DEBUG_ONLY(max_secs += now.tv_sec;)
>   } else {
>
> #else
>
>   { // Match the block scope.
>
> #endif // SUPPORTS_CLOCK_MONOTONIC
>
>     // Time-of-day clock is all we can reliably use.
>     struct timeval now;
>     int status = gettimeofday(&now, NULL);
>     assert(status == 0, "gettimeofday");

assert_status() is used above, but assert() is used here. Why?


>     if (isAbsolute) {
>       unpack_abs_time(abstime, timeout, now.tv_sec);
>     }
>     else {

Inconsistent "else-branch" formatting.
I believe HotSpot style is "} else {"


> calc_rel_time(abstime, timeout, now.tv_sec, now.tv_usec, MICROUNITS);
>     }
>     DEBUG_ONLY(max_secs += now.tv_sec;)
>   }
>
>   assert(abstime->tv_sec >= 0, "tv_sec < 0");
>   assert(abstime->tv_sec <= max_secs, "tv_sec > max_secs");
>   assert(abstime->tv_nsec >= 0, "tv_nsec < 0");
>   assert(abstime->tv_nsec < NANOSECS_PER_SEC, "tv_nsec >= 
> nanos_per_sec");

Why does the assert mesg have "nanos_per_sec" instead of "NANOSECS_PER_SEC"?

There's an extra blank line here.

>
> }

Definitely looks and reads much cleaner.

Dan