On 2016-03-17 09:40:08 -0400, Robert Haas wrote:
> On Thu, Mar 17, 2016 at 9:01 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> > I'll look at 0005 next, but thought I would send these comments along first.
>
> 0005: This is obviously very much WIP, but I think the overall
> direction of it is good.
> 0006: Same.
>
> I think you should use PGINVALID_SOCKET rather than -1 in various
> places in various patches in this series, especially if you are going
> to try to merge the Windows code path.
Sure.
> I wonder if CreateEventSet should accept a MemoryContext argument. It
> seems like callers will usually want TopMemoryContext, and just being
> able to pass that might be more convenient than having to switch back
> and forth in the calling code.
Makes sense.
> I wonder if there's a way to refactor this code to avoid having so
> much cut-and-paste duplication.
I guess you mean WaitEventSetWait() and WaitEventAdjust*? I've tried,
and my attempt ended up look nearly unreadable, because of the number of
ifdefs. I've not found a good attempt. Which is sad, because adding back
select support is going to increase the duplication further :( - but
it's also further away from poll etc. (different type of timestamp,
entirely different way of returming events).
> When iterating over the returned events, maybe check whether events is
> 0 at the top of the loop and skip it forthwith if so.
You mean in WaitEventSetWait()? There's else if (rc == 0) { break; }
which is the timeout case. There should never be any other case of returning 0 elements?
> That's all I've got for now.
Thanks for looking.
Greetings,
Andres Freund