(10) (M) RFR: 8174231: Factor out and share PlatformEvent and Parker code for POSIX systems (original) (raw)
Daniel D. Daugherty daniel.daugherty at oracle.com
Fri May 26 18:19:48 UTC 2017
- Previous message: (10) (M) RFR: 8174231: Factor out and share PlatformEvent and Parker code for POSIX systems
- Next message: (10) (M) RFR: 8174231: Factor out and share PlatformEvent and Parker code for POSIX systems
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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");
assert_status() 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 "nanos_per_sec" instead of "NANOSECS_PER_SEC"?
There's an extra blank line here.
}
Definitely looks and reads much cleaner.
Dan
- Previous message: (10) (M) RFR: 8174231: Factor out and share PlatformEvent and Parker code for POSIX systems
- Next message: (10) (M) RFR: 8174231: Factor out and share PlatformEvent and Parker code for POSIX systems
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]