[PATCH] event-loop: add wl_event_loop_has_idle() helper

[David Herrmann]

Hi Kristian

On Mon, Sep 10, 2012 at 6:39 PM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
> On Sun, Sep 09, 2012 at 04:02:45PM +0200, David Herrmann wrote:
>> As event-loop uses epoll() as base object we can stick one event-loop into
>> another by retrieving the epoll-fd and watching for events on it. However,
>> the idle objects aren't based on file-descriptors so if one event-callback
>> adds a new idle-source, the possible parent event-loop isn't notified of
>> this and will probably go to sleep instead of calling
>> wl_event_loop_dispatch() again in the next dispatch-round.
>>
>> This wl_event_loop_has_idle() helper can be used to avoid this starvation.
>> The parent event loop simply calls this before going to sleep. If it is
>> true, it sets the timeout to 0, calls its own events and then calls
>> wl_event_loop_dispatch() to dispatch the wayland-event-loop idlers.
>
> Can we just expose dispatch_idle_sources as
> wl_event_loop_dispatch_idles() and the you can call that before going
> to sleep?

No, that doesn't really help. I can just call
wl_event_loop_dispatch(loop, 0); This guarantees that epoll_wait()
doesn't sleep and thus, if no FD is readable, is equivalent to
wl_event_loop_dispatch_idles().

However, if an idle source adds itself a new idle source, this doesn't
work as we need to re-call dispatch() to call this new idle-source.
But I just noticed, this doesn't even work with current event-loop
design (as epoll_wait() might stall in between), so no reason to make
it work with other event loops.

But then again it would be probably cleaner to expose
wl_event_loop_dispatch_idles() (like you said) as it is definitely a
prettier name than wl_event_loop_dispatch(loop, 0); and has no
side-effects. I will prepare a patch and send it to the ML.

Thanks
David

> Kristian
>
>> We could also add an "eventfd(2)" file descriptor internally to the
>> event-loop and write to it, when an idle source is active, and read from
>> it when no idle-source is active, anymore. This guarantees, that this
>> eventfd is readable as long as an idle-source is registered. So the parent
>> event-loop is notified that the epoll-fd is readable.
>> However, on worst case this adds one syscall per idle-source registration
>> and one per idle-source execution which can be quite heavy if used
>> agressively. So the wl_event_loop_has_idle() approach is the less
>> aggressive solution to this.
>>
>> This, of course, is only used when stacking an wl_display/event-loop
>> object into your own event loop. Weston doesn't need this but other
>> compositors might want to stick to their own event-loop implementation
>> instead of using "event-loop" for their whole application.
>>
>> I used _Bool as I don't know whether stdbool.h is actually compatible with
>> all the X-headers and legacy stuff where wayland may be used.
>>
>> Signed-off-by: David Herrmann <dh.herrmann at googlemail.com>
>> ---
>>  src/event-loop.c     | 6 ++++++
>>  src/wayland-server.h | 1 +
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/src/event-loop.c b/src/event-loop.c
>> index a839daf..d0603aa 100644
>> --- a/src/event-loop.c
>> +++ b/src/event-loop.c
>> @@ -424,3 +424,9 @@ wl_event_loop_get_fd(struct wl_event_loop *loop)
>>  {
>>       return loop->epoll_fd;
>>  }
>> +
>> +WL_EXPORT _Bool
>> +wl_event_loop_has_idle(struct wl_event_loop *loop)
>> +{
>> +     return !wl_list_empty(&loop->idle_list);
>> +}
>> diff --git a/src/wayland-server.h b/src/wayland-server.h
>> index cd79801..83b6635 100644
>> --- a/src/wayland-server.h
>> +++ b/src/wayland-server.h
>> @@ -71,6 +71,7 @@ struct wl_event_source *wl_event_loop_add_idle(struct wl_event_loop *loop,
>>                                              wl_event_loop_idle_func_t func,
>>                                              void *data);
>>  int wl_event_loop_get_fd(struct wl_event_loop *loop);
>> +_Bool wl_event_loop_has_idle(struct wl_event_loop *loop);
>>
>>  struct wl_client;
>>  struct wl_display;
>> --
>> 1.7.12
>>