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

David Holmes david.holmes at oracle.com
Mon May 29 04:19:35 UTC 2017

Dan, Robbin, Thomas,

Okay here is the final ready to push version:

http://cr.openjdk.java.net/~dholmes/8174231/webrev.hotspot.v2/

this fixes all Dan's nits and refactors the time calculation code as suggested by Robbin.

Thomas: if you are around and able, it would be good to get a final sanity check on AIX. Thanks.

Testing:

Thanks again for the reviews.

David

On 29/05/2017 10:29 AM, David Holmes wrote:

On 27/05/2017 4:19 AM, 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 Dan - more below.

// 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. Fixed. 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?_ _Historical. assertstatus was introduced for the pthread* and other_ _posix funcs that return the error value rather than returning -1 and_ _setting errno. gettimeofday is not one of those so still has the old_ _assert. However, as someone pointed out a while ago you can use_ _assertstatus with these and pass errno as the "status". So I did that._ _if (isAbsolute) {_ _unpackabstime(abstime, timeout, now.tvsec);_ _}_ _else {_ _Inconsistent "else-branch" formatting._ _I believe HotSpot style is "} else {"_ _Fixed._ _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"? No reason. Actually that should now refer to NANOUNITS. Hmmm I can not recall why we have NANOUNITS and NANAOSECSPERSEC ... possibly an oversight. There's an extra blank line here. Fixed. Will send out complete updated webrev soon. Thanks, David } Definitely looks and reads much cleaner. Dan

More information about the hotspot-dev mailing list