Created attachment 259578 [details] [review]
Avoid needless event copies when queueing from a backend to a stage

All backends follow the same pattern of queueing events first in
ClutterMainContext, then copying them to a ClutterStage queue and
immediately free them. Instead, we can just pass ownership of events
directly to ClutterStage thus avoiding the allocation and copy in
between.

Created attachment 260440 [details] [review]
Avoid needless event copies when queueing from a backend to a stage

--

I forgot an #include "clutter-stage-private.h" in a few files.

Review of attachment 260440 [details] [review]:

::: clutter/clutter-stage-private.h
@@ +62,3 @@
 gboolean            _clutter_stage_do_update             (ClutterStage          *stage);
 
+void     _clutter_take_event                              (ClutterEvent *event);

the name is not really nice. at most, it should be clutter_event_take(), and be declared in clutter-event-private.h.

to be fair, though, I don't see the point of this churn: creating a wrapper around clutter_stage_queue_event() just to take the stage out of the event? all places that have access to _clutter_stage_queue_event() also have access to the stage to queue on, or they can simply get the event's stage if they don't.

I'd like to move away from the global "push an event" API, and make it clear that the events are queued on a per-stage basis.

::: clutter/clutter-stage.c
@@ -951,3 +951,3 @@
   first_event = priv->event_queue->length == 0;
 
-  g_queue_push_tail (priv->event_queue, clutter_event_copy (event));
+  g_queue_push_tail (priv->event_queue, event);

I'd rather add a boolean 'copy_event' argument to clutter_stage_queue_event(), and conditionally copy the event.

Created attachment 260470 [details] [review]
Avoid needless event copies when queueing from a backend to a stage

--

Ok, this works as well for me, although I generally don't like boolean
arguments. But this is only internal API and there's precedent in
_clutter_event_push() .

I'd really like to have something like this at least in the evdev backend. As a compositor, we see all events coming in so reducing the amount of alloc/copy/free on this path should help to at least reduce memory fragmentation.

Review of attachment 260470 [details] [review]:

okay, looks good to me.

Attachment 260470 [details] pushed as e70a010 - Avoid needless event copies when queueing from a backend to a stage