Re: condition variables

At Tue, 22 Nov 2016 17:48:07 +1300, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> wrote in <CAEepm=2VNfOq3spjSRGgM8WB+-PhfPXFB_adjizUUef9=cVDWQ(at)mail(dot)gmail(dot)com>
> On Tue, Nov 22, 2016 at 3:05 PM, Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> > Hello,
> >
> > At Mon, 21 Nov 2016 15:57:47 -0500, Robert Haas <robertmhaas(at)gmail(dot)com> wrote in <CA+TgmobFjwcFEiq8j+fvH5CdXHdVJffmemNLq8MqFesg2+4Gwg(at)mail(dot)gmail(dot)com>
> >> On Thu, Aug 11, 2016 at 5:47 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >> > So, in my
> >> > implementation, a condition variable wait loop looks like this:
> >> >
> >> > for (;;)
> >> > {
> >> > ConditionVariablePrepareToSleep(cv);
> >> > if (condition for which we are waiting is satisfied)
> >> > break;
> >> > ConditionVariableSleep();
> >> > }
> >> > ConditionVariableCancelSleep();
> >>
> >> I have what I think is a better idea. Let's get rid of
> >> ConditionVariablePrepareToSleep(cv) and instead tell users of this
> >> facility to write the loop this way:
> >>
> >> for (;;)
> >> {
> >> if (condition for which we are waiting is satisfied)
> >> break;
> >> ConditionVariableSleep(cv);
> >> }
> >> ConditionVariableCancelSleep();
> >
> > It seems rather a common way to wait on a condition variable, in
> > shorter,
> >
> > | while (condition for which we are waiting is *not* satisfied)
> > | ConditionVariableSleep(cv);
> > | ConditionVariableCancelSleep();
>
> Ok, let's show it that way.
>
> >> ConditionVariableSleep(cv) will check whether the current process is
> >> already on the condition variable's waitlist. If so, it will sleep;
> >> if not, it will add the process and return without sleeping.
> >>
> >> It may seem odd that ConditionVariableSleep(cv) doesn't necessary
> >> sleep, but this design has a significant advantage: we avoid
> >> manipulating the wait-list altogether in the case where the condition
> >> is already satisfied when we enter the loop. That's more like what we
> >
> > The condition check is done far faster than maintaining the
> > wait-list for most cases, I believe.
> >
> >> already do in lwlock.c: we try to grab the lock first; if we can't, we
> >> add ourselves to the wait-list and retry; if we then get the lock
> >> after all we have to recheck whether we can get the lock and remove
> >> ourselves from the wait-list if so. Of course, there is some cost: if
> >> we do have to wait, we'll end up checking the condition twice before
> >> actually going to sleep. However, it's probably smart to bet that
> >> actually needing to sleep is fairly infrequent, just as in lwlock.c.
> >>
> >> Thoughts?
> >
> > FWIW, I agree to the assumption.
>
> Here's a version that works that way, though it allows you to call
> ConditionVariablePrepareToSleep *optionally* before you enter your
> loop, in case you expect to have to wait and would rather avoid the
> extra loop. Maybe there isn't much point in exposing that though,
> since your condition test should be fast and waiting is the slow path,
> but we don't really really know what your condition test is. I
> thought about that because my use case (barrier.c) does in fact expect
> to hit the wait case more often than not. If that seems pointless
> then perhaps ConditionVariablePrepareToSleep should become static and
> implicit. This version does attempt to suppress spurious returns, a
> bit, using proclist_contains. No more cvSleeping.

Nice!

> It's possible that future users will want a version with a timeout, or
> multiplexed with IO, in which case there would be some interesting
> questions about how this should interact with WaitEventSet. It also
> seems like someone might eventually want to handle postmaster death.
> Perhaps there shoul eventually be a way to tell WaitEventSet that
> you're waiting for a CV so these things can be multiplexed without
> exposing the fact that it's done internally with latches.

Interesting. IMHO, returning on POSTMASTER_DEATH doesn't seem to
harm ordinary use and might be useful right now. CVSleepTimeout()
would be made sooner or later if someone needs. Multiplexed IO is
apparently a matter of WaitEventSet. Waiting CV by WaitEventSet
would be a matter of future.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center