Hi Pieter,

On Mon, Apr 16, 2018 at 10:41:48PM +0200, PiBa-NL wrote:
> Hi Olivier,
> 
> Op 16-4-2018 om 17:09 schreef Olivier Houchard:
> > After some discussion with Willy, we came with a solution that may fix your
> > problem with kqueue.
> > Can you test the attached patch and let me know if it fixes it for you ?
> > 
> > Minor variation of the patch, that uses EV_RECEIPT if available, to avoid
> > scanning needlessly the kqueue.
> > 
> > Regards,
> > 
> > Olivier
> 
> Thanks the patch solves the issue i experienced at least for the testcase
> that i had. (And doesn't seem to cause obvious new issues that i could
> quickly spot..) Both with and without EV_RECEIPT on kev[0] it seems to work
> the same for my testcase..
> 
> Just a few thoughts though:
> Now only the first kev[0] gets the EV_RECEIPT flag, shouldn't it be added to
> all items in the array? Now sometimes 3 changes are send and only 2
> 'results' are reported back. If i read right the EV_RECEIPT should 'force' a
> result for each change send. Also is there a reason you put it inside a
> '#ifdef' ? It seems to me a hard requirement to not read any possible
> pending events when sending the list of updated filters at that moment.?. Or
> perhaps its possible to call kevent only once? Both sending changes, and
> receiving new events in 1 big go and without the RECEIPT flag?
> 

We only need one to prevent kevent() from trying to scanning the kqueue, so
only setting kev[0] should be enough. It's inside an #ifdef because
EV_RECEIPT was only implemented recently in OpenBSD, so all users may not have
it.
My hope is, if EV_RECEIPT is not set, we will indeed read even needlessly,
but we will read them again at the next kevent() call. If it does not, and
events are lost, then we'll have to add an extra entry that generates an error.
We considered using only one kevent() call for both adding events, and reading
them, but it makes it difficult to respect maxpollevents, because we'd need
enough room for all errors and reading the new events. 

> There are now more 'changes' send than required that need to be disregarded
> with a 'error flag' by kqueue
> Doesn't that (slightly) affect performance? Or would checking a bitmask
> beforehand not be cheaper than what kevent itself needs to do to ignore an
> item and 'error report' some of the changes.?. I've not tried to measure
> this, but technically i think there will be a few more cpu operations needed
> overall this way.?.

It should be fine. What kevent() does is :
foreach (change) {
        ret = process_change();
        if (ret != 0 || flags & EV_RECEIPT)
                nerrors++;
}
if (nerrors > 0)
        return nerrors;
return scan_kqueue();

So it should virtually have no impact on performances.

Regards,

Olivier



Reply via email to