Re: condition variables

At Tue, 22 Nov 2016 17:48:07 +1300, Thomas Munro <thomas.munro@enterprisedb.com> wrote in
<CAEepm=2VNfOq3spjSRGgM8WB+-PhfPXFB_adjizUUef9=cVDWQ@mail.gmail.com>
> On Tue, Nov 22, 2016 at 3:05 PM, Kyotaro HORIGUCHI
> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> > Hello,
> >
> > At Mon, 21 Nov 2016 15:57:47 -0500, Robert Haas <robertmhaas@gmail.com> wrote in
<CA+TgmobFjwcFEiq8j+fvH5CdXHdVJffmemNLq8MqFesg2+4Gwg@mail.gmail.com>
> >> On Thu, Aug 11, 2016 at 5:47 PM, Robert Haas <robertmhaas@gmail.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