On Wed, Mar 16, 2016 at 10:04 PM, Andres Freund <and...@anarazel.de> wrote: > Questions: > * I'm kinda inclined to merge the win32 and unix latch > implementations. There's already a fair bit in common, and this is > just going to increase that amount.
Don't care either way. > * Right now the caller has to allocate the WaitEvents he's waiting for > locally (likely on the stack), but we also could allocate them as part > of the WaitEventSet. Not sure if that'd be a benefit. I'm not seeing this. What do you mean? 0001: Looking at this again, I'm no longer sure this is a bug. Doesn't your patch just check the same conditions in the opposite order? 0002: I think I reviewed this before. Boring. Just commit it already. 0003: Mostly boring. But the change to win32_latch.c seems to remove an unrelated check. 0004: + * drain it everytime WaitLatchOrSocket() is used. Should the + * pipe-buffer fill up in some scenarios - widly unlikely - we're every time wildly Why is it wildly (or widly) unlikely? The rejiggering this does between what is on which element of pfds[] appears to be unrelated to the ostensible purpose of the patch. + * Check again wether latch is set, the arrival of a signal/self-byte whether. Also not clearly related to the patch's main purpose. /* at least one event occurred, so check masks */ + if (FD_ISSET(selfpipe_readfd, &input_mask)) + { + /* There's data in the self-pipe, clear it. */ + drainSelfPipe(); + } The comment just preceding this added hunk now seems to be out of place, and maybe inaccurate as well. I think the new code could have a bit more detailed comment. My understanding is something like /* Since we didn't clear the self-pipe before attempting to wait, select() may have returned immediately even though there has been no recent change to the state of the latch. To prevent busy-looping, we must clear the pipe before attempting to wait again. */ I'll look at 0005 next, but thought I would send these comments along first. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers