Re: Performance degradation in commit ac1d794

On 2016-01-14 12:07:23 -0500, Robert Haas wrote:
> On Thu, Jan 14, 2016 at 12:06 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > On 2016-01-14 11:31:03 -0500, Robert Haas wrote:
> >> On Thu, Jan 14, 2016 at 10:56 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> >> I think your idea of a data structure the encapsulates a set of events
> >> for which to wait is probably a good one. WaitLatch doesn't seem like
> >> a great name. Maybe WaitEventSet, and then we can have
> >> WaitLatch(&latch) and WaitEvents(&eventset).
> >
> > Hm, I'd like to have latch in the name. It seems far from improbably to
> > have another wait data structure. LatchEventSet maybe? The wait would be
> > implied by WaitLatch.
>
> I can live with that.

How about the following sketch of an API

typedef struct LatchEvent
{
uint32 events; /* interesting events */
uint32 revents; /* returned events */
int fd; /* fd associated with event */
} LatchEvent;

typedef struct LatchEventSet
{
int nevents;
LatchEvent events[FLEXIBLE_ARRAY_MEMBER];
} LatchEventSet;

/*
* Create a LatchEventSet with space for nevents different events to wait for.
*
* latch may be NULL.
*/
extern LatchEventSet *CreateLatchEventSet(int nevents, Latch *latch);

/* ---
* Add an event to the set. Possible events are:
* - WL_LATCH_SET: Wait for the latch to be set
* - WL_SOCKET_READABLE: Wait for socket to become readable
* can be combined in one event with WL_SOCKET_WRITEABLE
* - WL_SOCKET_WRITABLE: Wait for socket to become readable
* can be combined with WL_SOCKET_READABLE
* - WL_POSTMASTER_DEATH: Wait for postmaster to die
*/
extern void AddLatchEventToSet(LatchEventSet *set, uint32 events, int fd);

/*
* Wait for any events added to the set to happen, or until the timeout is
* reached.
*
* The return value is the union of all the events that were detected. This
* allows to avoid having to look into the associated events[i].revents
* fields.
*/
extern uint32 WaitLatchEventSet(LatchEventSet *set, long timeout);

I've two questions:
- Is there any benefit of being able to wait for more than one latch?
I'm inclined to not allow that for now, that'd make the patch bigger,
and I don't see a use-case right now.
- Given current users we don't need a large amount of events, so having
to iterate through the registered events doesn't seem bothersome. We
could however change the api to be something like

int WaitLatchEventSet(LatchEventSet *set, OccurredEvents *, int nevents, long timeout);

which would return the number of events that happened, and would
basically "fill" one of the (usually stack allocated) OccurredEvent
structures with what happened.

Comments?