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

[David Holmes]

Correction ...

On 19/05/2017 8:53 PM, David Holmes wrote:
> On 19/05/2017 7:25 PM, Robbin Ehn wrote:
>> On 05/19/2017 11:07 AM, David Holmes wrote:
>>>
>>> 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.
>>
>> Yes thanks!
>>
>> But you can still re-factoring to something similar to what I 
>> suggested and two of the calculation should be the same just ns vs us, 
>> correct?
> 
> There are three different forms of the calculation. The two relative 
> time versions use a different time function and so a different time 
> structure (timeval vs timespec) and a different calculation.
> 
>> Leaving the if statement with the "!isAbsolute" check, in my head 
>> calc_time is something like:
>>
>> void calc_time(...) {
>>    if (isAbsolute) {
>>      calc_abs_time(...);
>>    } else {
> #ifdef SUPPORTS_CLOCK_MONOTONIC
>>      calc_rel_time_from_clock_monotonic(...);
> #else
>  >      calc_rel_time_from_gettimeofday(...);
> #endif
>>    }
>> }

It's more complicated than that because we may have build time 
SUPPORTS_CLOCK_MONOTONIC but we still need the runtime check as well.

to_abstime is the old linux unpackTime with the addition of the build 
time conditionals.

David

> David
> -----
> 
>> I do not see a problem with this, only better readability?
>>
>> /Robbin
>>
>>>
>>> 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