[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(at)gmail(dot)com> wrote in <CA+TgmoYBa8TJRGS07JCSLKpqGkrRd5hLpirvwp36s=83ChmQDA(at)mail(dot)gmail(dot)com>
> On Thu, Jan 14, 2016 at 12:14 PM, Andres Freund <andres(at)anarazel(dot)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