[PoC] WaitLatchOrSocketMulti (Re: Performance degradation in commit ac1d794)

Hello, I don't see how ac1d794 will be dealt, but I tried an
example implement of multi-socket version of WaitLatchOrSocket
using callbacks on top of the current master where ac1d794 has
not been removed yet.

At Thu, 14 Jan 2016 13:46:44 -0500, Robert Haas <robertmhaas@gmail.com> wrote in
<CA+TgmoYBa8TJRGS07JCSLKpqGkrRd5hLpirvwp36s=83ChmQDA@mail.gmail.com>
> On Thu, Jan 14, 2016 at 12:14 PM, Andres Freund <andres@anarazel.de> wrote:
> > On 2016-01-14 12:07:23 -0500, Robert Haas wrote:
> >> > Do we want to provide a backward compatible API for all this? I'm fine
> >> > either way.
> >>
> >> How would that work?
> >
> > I'm thinking of something like;
> >
> > int WaitOnLatchSet(LatchEventSet *set, int wakeEvents, long timeout);
> >
> > int
> > WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock,long timeout)
> > {
> >         LatchEventSet set;
> >
> >         LatchEventSetInit(&set, latch);
> >
> >         if (sock != PGINVALID_SOCKET)
> >            LatchEventSetAddSock(&set, sock);
> >
> >         return WaitOnLatchSet(set, wakeEvents, timeout);
> > }
> >
> > I think we'll need to continue having wakeEvents and timeout parameters
> > for WaitOnLatchSet, we quite frequently want to wait socket
> > readability/writability, not wait on the socket, or have/not have
> > timeouts.
> 
> Well, if we ever wanted to support multiple FDs, we'd need the
> readability/writeability thing to be per-fd, not per-set.
> 
> Overall, if this is what you have in mind for backward compatibility,
> I rate it M for Meh.  Let's just break compatibility and people will
> have to update their code.  That shouldn't be hard, and if we don't
> make people do it when we make the change, then we'll be stuck with
> the backward-compatibility interface for a decade.  I doubt it's worth
> it.

The API is similar to what Robert suggested but different because
it would too complicate a bit for the most cases. So this example
implement has an intermediate style of the current API and the
Robert's suggestion, and using callbacks as I proposed.

int WaitLatchOrSocketMulti(pgwaitobject *wobjs, int nobjs, long timeout);

This is implemented only for poll, not for select.

A sample usage is seen in secure_read().

>     pgwaitobject objs[3];
...
>     InitWaitLatch(objs[0], MyLatch);
>     InitWaitPostmasterDeath(objs[1]);
>     InitWaitSocket(objs[2], port->sock, waitfor);
> 
>     w = WaitLatchOrSocketMulti(objs, 3, 0);
> //    w = WaitLatchOrSocket(MyLatch,
> //                WL_LATCH_SET | WL_POSTMASTER_DEATH | waitfor,
> //                port->sock, 0);


The core of the function looks as the following. It runs
callbacks for every fired events.

>    rc = poll(pfds, nfds, (int) cur_timeout);
...
>    if (rc < 0)
...
>    else
>    {
>      for (i = 0 ; i < nfds ; i++)
>      {
>        wobjs[i].retEvents = 0;
>        if (pfds[i].revents && wobjs[i].cb)
>          result |= wobjs[i].cb(&wobjs[i], pfds[i].revents);
>
>        if (result & WL_IMMEDIATELY_BREAK)
>          break;
>      }
>    }

In the above part, poll()'s event is passed the callbacks so
callbacks may have a different inplement for select().

Having a callback for sockets. The initializer could be as the
following.

> InitWaitSocketCB(wait_obj, sock, event, your_callback);


If we want to have the waiting-object array independently from
specific functions to achieve asynchronous handling of socket
events. It could be realised by providing a set of wrapper
functions as exactly what Robert said as above.

Does this make sense?
Does anyone have any opinion? or thoughts?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center