At Tue, 22 Nov 2016 17:48:07 +1300, Thomas Munro <thomas.mu...@enterprisedb.com> wrote in <CAEepm=2VNfOq3spjSRGgM8WB+-PhfPXFB_adjizUUef9=cv...@mail.gmail.com> > On Tue, Nov 22, 2016 at 3:05 PM, Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.jp> wrote: > > Hello, > > > > At Mon, 21 Nov 2016 15:57:47 -0500, Robert Haas <robertmh...@gmail.com> > > wrote in > > <ca+tgmobfjwcfeiq8j+fvh5cdxhdvjffmemnlq8mqfesg2+4...@mail.gmail.com> > >> On Thu, Aug 11, 2016 at 5:47 PM, Robert Haas <robertmh...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers