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

[David Holmes]

Hi Robbin,

Thanks for looking at this.

On 19/05/2017 6:36 PM, Robbin Ehn wrote:
> Hi David,
> 
> On 05/18/2017 08:25 AM, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8174231
>>
>> webrevs:
>>
>> Build-related: http://cr.openjdk.java.net/~dholmes/8174231/webrev.top/
>>        hotspot: 
>> http://cr.openjdk.java.net/~dholmes/8174231/webrev.hotspot/
>>
> 
> I like this, with neg delta of 700 loc, nice!
> 
> It's hard to see if you broken anything, since you combined 4 separate 
> implementation into 1.

Well not really. As I said this is basically a cleaned up version of the 
Linux code. The BSD and AIX versions were already based on earlier 
versions of the Linux code, minus the proper handling of CLOCK_MONOTONIC 
and absolute timeouts.

> I guess you have tested this proper?

Only JPRT so far. I should have mentioned that I'm not expecting this to 
be reviewed in pushed within a couple of days, so some refinements and 
continual testing will occur.

> What stands out in os_posix.cpp is the
> static void to_abstime(timespec* abstime, jlong timeout, bool isAbsolute)
> 
> The ifdef scopes of SUPPORTS_CLOCK_MONOTONIC is large and calculations 
> are repeated 3 times.

They have to be as there are three cases:

1. Relative wait using CLOCK_MONOTONIC
2. Relative wait using gettimeofday()
3. Absolute wait using gettimeofday()

> Please consider something like:
> 
> #ifdef SUPPORTS_CLOCK_MONOTONIC
>    if (_use_clock_monotonic_condattr  && !isAbsolute) { // Why aren't we 
> using this when not isAbsolute is set?
>                                 // I suggest removing that check from 
> this if and use monotonic for that also.

Absolute waits have to be based on wall-clock time and follow any 
adjustments made to wall clock time. In contrast relative waits should 
never be affected by wall-clock time adjustments hence the use of 
CLOCK_MONOTONIC when available.

In Java the relative timed-waits are:
- Thread.sleep(ms)
- Object.wait(ms)/wait(ms,ns)
- LockSupport.parkNanos(ns) (and all the j.u.c blocking methods built on 
top of it)

While the only absolute timed-wait we have is the LockSupport.parkUntil 
method(s).

Hope that clarifies things.

Thanks,
David
-----

>       struct timespec now;
>       int status = _clock_gettime(CLOCK_MONOTONIC, &now);
>       assert_status(status == 0, status, "clock_gettime");
>       calc_time(abstime, timeout, isAbsolute, now.tv_sec, now.tv_nsec, 
> NANOUNITS);
>    } else {
> #else
>    {
> #endif
>       struct timeval now;
>       int status = gettimeofday(&now, NULL);
>       assert(status == 0, "gettimeofday");
>       calc_time(abstime, timeout, isAbsolute, now.tv_sec, now.tv_usec, 
> MICROUNITS);
>    }
> #endif
> 
> Thanks for fixing this!
> 
> /Robbin