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

Robbin Ehn robbin.ehn at oracle.com
Sat May 27 12:06:28 UTC 2017

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 toabstime 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 "nowpartsec" is - nanos or micros. And the calcabstime always has a deadline in millis. So I simplified and did a little renaming, and tracked maxsecs in debugonly 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 "nowpartsec" (may be nanos or micros depending // on which clock is being used). static void calcreltime(timespec* abstime, jlong timeout, jlong nowsec, jlong nowpartsec, jlong unit) { timet maxsecs = nowsec + MAXSECS; jlong seconds = timeout / NANOUNITS; timeout %= NANOUNITS; // remaining nanos if (seconds >= MAXSECS) { // More seconds than we can add, so pin to maxsecs. abstime->tvsec = maxsecs; abstime->tvnsec = 0; } else { abstime->tvsec = nowsec + seconds; long nanos = (nowpartsec * (NANOUNITS / unit)) + timeout; if (nanos >= NANOUNITS) { // overflow abstime->tvsec += 1; nanos -= NANOUNITS; } abstime->tvnsec = 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 unpackabstime(timespec* abstime, jlong deadline, jlong nowsec) { timet maxsecs = nowsec + MAXSECS; jlong seconds = deadline / MILLIUNITS; jlong millis = deadline % MILLIUNITS; if (seconds >= maxsecs) { // Absolute seconds exceeds allowed max, so pin to maxsecs. abstime->tvsec = maxsecs; abstime->tvnsec = 0; } else { abstime->tvsec = seconds; abstime->tvnsec = millis * (NANOUNITS / MILLIUNITS); } } static void toabstime(timespec* abstime, jlong timeout, bool isAbsolute) { There's an extra blank line here. DEBUGONLY(int maxsecs = MAXSECS;) if (timeout < 0) {_ _timeout = 0;_ _}_ _#ifdef SUPPORTSCLOCKMONOTONIC_ _if (useclockmonotoniccondattr && !isAbsolute) {_ _struct timespec now;_ _int status = clockgettime(CLOCKMONOTONIC, &now);_ _assertstatus(status == 0, status, "clockgettime");_ _calcreltime(abstime, timeout, now.tvsec, now.tvnsec, NANOUNITS);_ _DEBUGONLY(maxsecs += now.tvsec;)_ _} else {_ _#else_ _{ // Match the block scope._ _#endif // SUPPORTSCLOCKMONOTONIC_ _// Time-of-day clock is all we can reliably use._ _struct timeval now;_ _int status = gettimeofday(&now, NULL);_ _assert(status == 0, "gettimeofday");_ _assertstatus() is used above, but assert() is used here. Why?_ _if (isAbsolute) {_ _unpackabstime(abstime, timeout, now.tvsec);_ _}_ _else {_ _Inconsistent "else-branch" formatting._ _I believe HotSpot style is "} else {"_ _calcreltime(abstime, timeout, now.tvsec, now.tvusec, MICROUNITS);_ _}_ _DEBUGONLY(maxsecs += now.tvsec;)_ _}_ _assert(abstime->tvsec >= 0, "tvsec < 0");_ _assert(abstime->tvsec <= maxsecs, "tvsec > maxsecs"); assert(abstime->tvnsec >= 0, "tvnsec < 0");_ _assert(abstime->tvnsec < NANOSECSPERSEC, "tvnsec >= nanospersec"); Why does the assert mesg have "nanospersec" instead of "NANOSECSPERSEC"? There's an extra blank line here. } Definitely looks and reads much cleaner. Dan

More information about the hotspot-dev mailing list