[PATCH 4/8] client: Add wl_event_queue for multi-thread dispatching
Kristian Høgsberg
hoegsberg at gmail.com
Wed Oct 10 12:50:35 PDT 2012
On Wed, Oct 10, 2012 at 03:43:26PM +0300, Ander Conselvan de Oliveira wrote:
> On 10/10/2012 05:38 AM, Kristian Høgsberg wrote:
> >This introduces wl_event_queue, which is what will make multi-threaded
> >wayland clients possible and useful. The driving use case is that of a
> >GL rendering thread that renders and calls eglSwapBuffer independently of
> >a "main thread" that owns the wl_display and handles input events and
> >everything else. In general, the EGL and GL APIs have a threading model
> >that requires the wayland client library to be usable from several threads.
> >Finally, the current callback model gets into trouble even in a single
> >threaded scenario: if we have to block in eglSwapBuffers, we may end up
> >doing unrelated callbacks from within EGL.
> >
> >The wl_event_queue mechanism lets the application (or middleware such as
> >EGL or toolkits) assign a proxy to an event queue. Only events from objects
> >associated with the queue will be put in the queue, and conversely,
> >events from objects associated with the queue will not be queue up anywhere
> >else. The wl_display struct has a built-in event queue, which is considered
> >the main and default event queue. New proxies are associated with the
> >same queue as the object that created them (either the object that a
> >request with a new-id argument was sent to or the object that sent an
> >event with a new-id argument). A proxy can be moved to a different event
> >queue by calling wl_proxy_set_queue().
> >
> >A subsystem, such as EGL, will then create its own event queue and associate
> >the objects it expects to receive events from with that queue. If EGL
> >needs to block and wait for a certain event, it can keep dispatching event
> >from its queue until that events comes in. This wont call out to unrelated
> >code with an EGL lock held. Similarly, we don't risk the main thread
> >handling an event from an EGL object and then calling into EGL from a
> >different thread without the lock held.
> >---
> > src/wayland-client.c | 148 ++++++++++++++++++++++++++++++++++++++------------
> > src/wayland-client.h | 8 +++
> > 2 files changed, 122 insertions(+), 34 deletions(-)
> >
> >diff --git a/src/wayland-client.c b/src/wayland-client.c
> >index 6a499e0..694fd39 100644
> >--- a/src/wayland-client.c
> >+++ b/src/wayland-client.c
>
> [...]
>
> >@@ -552,45 +612,50 @@ queue_event(struct wl_display *display, int len, struct wl_list *list)
> > if (wl_debug)
> > wl_closure_print(closure, &proxy->object, false);
> >
> >- if (closure == NULL || create_proxies(display, closure) < 0) {
> >+ if (closure == NULL || create_proxies(proxy, closure) < 0) {
> > fprintf(stderr, "Error demarshalling event\n");
> > abort();
> > }
> >
> >- wl_list_insert(list->prev, &closure->link);
> >+ if (wl_list_empty(&proxy->queue->event_list))
> >+ pthread_cond_signal(&proxy->queue->cond);
> >+ wl_list_insert(proxy->queue->event_list.prev, &closure->link);
>
> I find a bit confusing that event is added to the queue after
> signaling the condition although I see this works because the other
> thread will need to get the display lock before proceeding and this
> is held here. Maybe a comment would help.
Yeah, nothing happens until we release the mutex so there's no race
here. Testing whether the list was empty is easier to do before we
start adding elements to it, which is why it's done that way. It's a
key feature of how condition variables work, so I don't think it's too
subtle.
> [...]
>
> >@@ -599,27 +664,35 @@ wl_display_dispatch(struct wl_display *display)
> > /* FIXME: Handle flush errors, EAGAIN... */
> > wl_connection_flush(display->connection);
> >
> >- /* FIXME: Shouldn't always read here... */
> >- len = wl_connection_read(display->connection);
> >- if (len == -1)
> >- return -1;
> >-
> >- wl_list_init(&list);
> >- while (len >= 8) {
> >- size = queue_event(display, len, &list);
> >- if (size == 0)
> >- break;
> >- len -= size;
> >+ if (wl_list_empty(&queue->event_list) &&
> >+ pthread_equal(display->display_thread, pthread_self())) {
> >+ len = wl_connection_read(display->connection);
> >+ if (len == -1)
> >+ return -1;
>
> This returns with the display lock held.
Oops, fixed. Thanks.
> Cheers,
> Ander
>
> >+ while (len >= 8) {
> >+ size = queue_event(display, len);
> >+ if (size == 0)
> >+ break;
> >+ len -= size;
> >+ }
> >+ } else if (wl_list_empty(&queue->event_list)) {
> >+ pthread_cond_wait(&queue->cond, &display->mutex);
> > }
> >
> >+ while (!wl_list_empty(&queue->event_list))
> >+ dispatch_event(display, queue);
> >+
> > pthread_mutex_unlock(&display->mutex);
> >
> >- while (!wl_list_empty(&list)) {
> >- closure = container_of(list.next, struct wl_closure, link);
> >- dispatch_event(display, closure);
> >- }
> >+ return 0;
> >+}
> >+
> >+WL_EXPORT int
> >+wl_display_dispatch(struct wl_display *display)
> >+{
> >+ display->display_thread = pthread_self();
> >
> >- return len;
> >+ return wl_display_dispatch_queue(display, &display->queue);
> > }
> >
> > WL_EXPORT int
> >@@ -685,6 +758,13 @@ wl_proxy_get_id(struct wl_proxy *proxy)
> > return proxy->object.id;
> > }
> >
> >+
> >+WL_EXPORT void
> >+wl_proxy_set_queue(struct wl_proxy *proxy, struct wl_event_queue *queue)
> >+{
> >+ proxy->queue = queue;
> >+}
> >+
> > WL_EXPORT void
> > wl_log_set_handler_client(wl_log_func_t handler)
> > {
> >diff --git a/src/wayland-client.h b/src/wayland-client.h
> >index 5fcb86d..fbbee09 100644
> >--- a/src/wayland-client.h
> >+++ b/src/wayland-client.h
> >@@ -32,6 +32,9 @@ extern "C" {
> >
> > struct wl_proxy;
> > struct wl_display;
> >+struct wl_event_queue;
> >+
> >+void wl_event_queue_destroy(struct wl_event_queue *queue);
> >
> > void wl_proxy_marshal(struct wl_proxy *p, uint32_t opcode, ...);
> > struct wl_proxy *wl_proxy_create(struct wl_proxy *factory,
> >@@ -46,6 +49,7 @@ int wl_proxy_add_listener(struct wl_proxy *proxy,
> > void wl_proxy_set_user_data(struct wl_proxy *proxy, void *user_data);
> > void *wl_proxy_get_user_data(struct wl_proxy *proxy);
> > uint32_t wl_proxy_get_id(struct wl_proxy *proxy);
> >+void wl_proxy_set_queue(struct wl_proxy *proxy, struct wl_event_queue *queue);
> >
> > void *wl_display_bind(struct wl_display *display,
> > uint32_t name, const struct wl_interface *interface);
> >@@ -74,8 +78,12 @@ struct wl_display *wl_display_connect_to_fd(int fd);
> > void wl_display_disconnect(struct wl_display *display);
> > int wl_display_get_fd(struct wl_display *display);
> > int wl_display_dispatch(struct wl_display *display);
> >+int wl_display_dispatch_queue(struct wl_display *display,
> >+ struct wl_event_queue *queue);
> >+
> > int wl_display_flush(struct wl_display *display);
> > void wl_display_roundtrip(struct wl_display *display);
> >+struct wl_event_queue *wl_display_create_queue(struct wl_display *display);
> >
> > struct wl_global_listener;
> > typedef void (*wl_display_global_func_t)(struct wl_display *display,
> >
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list