Re: Introduce timeout capability for ConditionVariableSleep

Hi Thomas,

Thanks for reviewing!

On Wed, Mar 13, 2019 at 12:40:57PM +1300, Thomas Munro wrote:
> Can we just refer to the other function's documentation for this?  I
> don't want two copies of this blurb (and this copy-paste already
> failed to include "Timed" in the example function name).

Hah - point well taken. Fixed.

> One difference compared to pthread_cond_timedwait() is that pthread
> uses an absolute time and here you use a relative time (as we do in
> WaitEventSet API).  The first question is which makes a better API,
> and the second is what the semantics of a relative timeout should be:
> start again every time we get a spurious wake-up, or track time
> already waited?  Let's see...
> 
> -        (void) WaitEventSetWait(cv_wait_event_set, -1, &event, 1,
> +        ret = WaitEventSetWait(cv_wait_event_set, timeout, &event, 1,
>                                  wait_event_info);
> 
> Here you're restarting the timeout clock every time through the loop
> without adjustment, and I think that's not a good choice: spurious
> wake-ups cause bonus waiting.

Agree. In my testing WaitEventSetWait did the work for calculating the 
right timeout remaining. It's a bummer that we have to repeat the same 
pattern at the ConditionVariableTimedSleep() but I guess anyone who 
loops in such cases will have to adjust their values accordingly.

BTW, I am curious why Andres in 98a64d0bd71 didn't just create an 
artificial event with WL_TIMEOUT and return that from 
WaitEventSetWait(). Would have made it cleaner than checking checking 
return values for -1.

Updated v2 patch attached.

-- 
Shawn Debnath
Amazon Web Services (AWS)