[PATCH] event-loop: Only read one epoll event per loop dispatch.

[Andreas Ericsson]

Your MUA seems to spit out 6km long lines, which makes it very
hard to read when you also post code so I can't auto-reformat
on my end. I'll give it a go though.

On 03/20/2012 06:39 PM, Bill Spitzak wrote:
> I may not be understanding the problem, or maybe my assumption that
> the destruction of a "source" is not under Wayland's control.
> 

I take it that sentence was about to end in "is wrong".

The sources we're talking about here are input, signal and timer
sources. More specifically, wayland's representation of them as
wl_event_source objects. The handling of those are very much under
wayland's control.

> My opinion is that "deferred deletion" is a bad idea. There may
> be other objects being destroyed that destroy the source, and they
> rely on the source actually disappearing. What happens then is you
> have to defer the deletion of these objects as well. Eventually
> *everything* has to have a deferred deletion ability. Look at Qt if
> you don't believe me.
> 

I believe you. In this case though, "destruction of a source" means
just free()'ing it in one of wayland's many internal API's, and we
can't do that until there are no more pointers to it, but we have
to mark it as "pending destruction" so we don't try to use it after
it has been deinitialized. Think of it as a poor-mans refcount.

> I seem to be having a hard time describing this. Here is what I would do:
> 
> source* table[MAXENTRIES]; // I'll ignore overflow of this for now
> int nextentry;
> 
> add(source* s) {
> int n = nextentry++;
> s->entry = n;
> table[n] = s;
> ep.data.int = n;
> epoll_ctl(...ep);
> }
> 
> remove(source* s) {
> table[s->entry] = 0;
> epoll_ctl(...);
> }
> 
> wl_event_loop_dispatch() {
> epoll(...);
> for (i=0; i < count; i++) {
> source = table[ep[i].data.int];
> if (source) source->interface->dispatch(source, &ep[i]);
> }
> }


Allocate the table large enough to hold the max number of usable file
descriptors and use the file descriptor as an index to the array to
stop wasting one int per entry and you've got exactly the code I ended
up with in my own I/O-broker thingie.

It's slightly more efficient cpu-wise but eats a larger chunk of constant
memory. Not having to free() and malloc() so much could gain that memory
back in the long run from less fragmentation, but I doubt it will matter
much for wayland, for a couple of reasons:

* Wayland event-loops will be pretty sparsely populated most of the time
(compared to full-mesh networked jobspawners that pretty much saturate
the table with shortlived entries more or less constantly), so allocating
but not using a sizeable chunk of memory would be a large-ish waste.
* The exception case handled here won't happen so often that the extra
cycles spent will mean that much.
* Since wayland will likely have relatively few sources, it might even
be more efficient to not keep the continous table of sources, since
that one will almost certainly be prioritized by the LRU sitting cache.

Either way; There was a problew. Now it's most likely fixed. If that
fix leads to another problem, be it performance or bugs, we'll fix
that when we need to.

-- 
Andreas Ericsson                   andreas.ericsson at op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.