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

[Robbin Ehn]

Thanks David, looks good!

/Robbin

On 2017-05-26 20:19, Daniel D. Daugherty wrote:
> 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
>