[PATCH] mi: streamline CopyGetMasterEvent(), remove code duplication.

Peter Hutterer peter.hutterer at who-t.net
Tue Dec 9 15:18:12 PST 2008


On Tue, Dec 09, 2008 at 01:57:47AM +1100, Daniel Stone wrote:
> On Mon, Dec 08, 2008 at 06:13:36PM +1000, Peter Hutterer wrote:
> > +    int len = count * sizeof(xEvent);
> > +    xEvent *ev;
> > +
> > +    /* Assumption: GenericEvents always have count 1 */
> > +
> > +    if (GEV(original)->type == GenericEvent)
> > +        len += GEV(original)->length * 4;
> > +
> > +    ev = xalloc(len);
> > +    if (!ev)
> > +        FatalError("[mi] No memory left for master event.\n");
> > +    memcpy(ev, original, len);
> > +    *master = ev;
> > +
> > +    while(count--)
> 
> Would it be possible to do something like we do with GPE and friends
> where you pessimistically allocate based on the maximum possible number
> of events and only reallocate upwards when necessary? Having malloc() in
> the event delivery path is a bit of a loss.

How about this one?

>From 9be3abf13d291879e37652f7364c2712f6bcc592 Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer at redhat.com>
Date: Mon, 1 Dec 2008 21:20:48 +1000
Subject: [PATCH] mi: Clean up CopyGetMasterEvent, re-use the memory.

Alloc an EventList once and then re-use instead of allocing a new event each
time we need a master event.
There's a trick included: because all the event processing handlers only take
an xEvent, init a size 1 EventList and squash the events into this one.

Events that have count > 1 must be squished into an xEvent array anyway before
passing into the event handlers, so we don't lose anything here.

Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
 mi/mieq.c       |   60 +++++++++++++++++++++++++++---------------------------
 xkb/ddxDevBtn.c |   16 ++++++++++----
 2 files changed, 41 insertions(+), 35 deletions(-)

diff --git a/mi/mieq.c b/mi/mieq.c
index 971edf9..d1de5c6 100644
--- a/mi/mieq.c
+++ b/mi/mieq.c
@@ -80,6 +80,7 @@ typedef struct _EventQueue {
 } EventQueueRec, *EventQueuePtr;
 
 static EventQueueRec miEventQueue;
+static EventListPtr masterEvents; /* for use in mieqProcessInputEvents */
 
 Bool
 mieqInit(void)
@@ -98,6 +99,17 @@ mieqInit(void)
             FatalError("Could not allocate event queue.\n");
         miEventQueue.events[i].events = evlist;
     }
+
+    /* XXX: mE is just 1 event long, if we have Motion + Valuator they are
+     * squashed into the first event to make passing it into the event
+     * processing handlers easier. This should be fixed when the processing
+     * handlers switch to EventListPtr instead of xEvent */
+    masterEvents = InitEventList(1);
+    if (!masterEvents)
+        FatalError("Could not allocated MD event queue.\n");
+    SetMinimumEventSize(masterEvents, 1,
+                        7 /* 1 + MAX_VALUATOR_EVENTS */  * sizeof(xEvent));
+
     SetInputCheck(&miEventQueue.head, &miEventQueue.tail);
     return TRUE;
 }
@@ -275,28 +287,21 @@ ChangeDeviceID(DeviceIntPtr dev, xEvent* event)
  */
 void
 CopyGetMasterEvent(DeviceIntPtr mdev, xEvent* original,
-                   xEvent** master, int count)
+                   EventListPtr master, int count)
 {
-    if (count > 1) {
-        *master = xcalloc(count, sizeof(xEvent));
-        if (!*master)
-            FatalError("[mi] No memory left for master event.\n");
-        while(count--)
-        {
-            memcpy(&(*master)[count], &original[count], sizeof(xEvent));
-            ChangeDeviceID(mdev, &(*master)[count]);
-        }
-    } else
-    {
-        int len = sizeof(xEvent);
-        if (original->u.u.type == GenericEvent)
-            len += GEV(original)->length * 4;
-        *master = xalloc(len);
-        if (!*master)
-            FatalError("[mi] No memory left for master event.\n");
-        memcpy(*master, original, len);
-        ChangeDeviceID(mdev, *master);
-    }
+    int len = count * sizeof(xEvent);
+
+    /* Assumption: GenericEvents always have count 1 */
+
+    if (GEV(original)->type == GenericEvent)
+        len += GEV(original)->length * 4;
+
+    if (master->evlen < len)
+        SetMinimumEventSize(master, 1, len);
+
+    memcpy(master->event, original, len);
+    while (count--)
+        ChangeDeviceID(mdev, &master->event[count]);
 }
 
 /* Call this from ProcessInputEvents(). */
@@ -308,8 +313,7 @@ mieqProcessInputEvents(void)
     int x = 0, y = 0;
     int type, nevents, evlen, i;
     ScreenPtr screen;
-    xEvent *event,
-           *master_event = NULL;
+    xEvent *event;
     DeviceIntPtr dev = NULL,
                  master = NULL;
 
@@ -358,10 +362,7 @@ mieqProcessInputEvents(void)
         }
         else {
             if (master)
-                CopyGetMasterEvent(master, event,
-                                   &master_event, nevents);
-            else
-                master_event = NULL;
+                CopyGetMasterEvent(master, event, masterEvents, nevents);
 
             /* If someone's registered a custom event handler, let them
              * steal it. */
@@ -370,19 +371,18 @@ mieqProcessInputEvents(void)
                 handler(DequeueScreen(dev)->myNum, event, dev, nevents);
                 if (master)
                     handler(DequeueScreen(master)->myNum,
-                            master_event, master, nevents);
+                            masterEvents->event, master, nevents);
             } else
             {
                 /* process slave first, then master */
                 dev->public.processInputProc(event, dev, nevents);
 
                 if (master)
-                    master->public.processInputProc(master_event, master,
+                    master->public.processInputProc(masterEvents->event, master,
                                                     nevents);
             }
 
             xfree(event);
-            xfree(master_event);
         }
 
         /* Update the sprite now. Next event may be from different device. */
diff --git a/xkb/ddxDevBtn.c b/xkb/ddxDevBtn.c
index b68a28b..5a7ee7e 100644
--- a/xkb/ddxDevBtn.c
+++ b/xkb/ddxDevBtn.c
@@ -42,13 +42,14 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 extern	int	DeviceValuator;
 
+static EventListPtr masterEvents = NULL;
+
 void
 XkbDDXFakeDeviceButton(DeviceIntPtr dev,Bool press,int button)
 {
 int *			devVal;
 INT32 *			evVal;
-xEvent			events[2],
-			*m_events = NULL; /* master dev */
+xEvent			events[2];
 deviceKeyButtonPointer *btn;
 deviceValuator *	val;
 int			x,y;
@@ -107,18 +108,23 @@ DeviceIntPtr		master = NULL;
      * cases, unless dev is both a keyboard and a mouse.
      */
     if (!dev->isMaster && dev->u.master) {
+        if (!masterEvents)
+        {
+            masterEvents = InitEventList(1);
+                                                 /* 1 + MAX_VALUATOR_EVENTS */
+            SetMinimumEventSize(masterEvents, 1, 7 * sizeof(xEvent));
+        }
         master = dev->u.master;
         if (!IsPointerDevice(master))
             master = GetPairedDevice(dev->u.master);
 
-        CopyGetMasterEvent(master, &events, &m_events, count);
+        CopyGetMasterEvent(master, &events, masterEvents, count);
     }
 
     (*dev->public.processInputProc)((xEventPtr)btn, dev, count);
 
     if (master) {
-        (*master->public.processInputProc)(m_events, master, count);
-        xfree(m_events);
+        (*master->public.processInputProc)(masterEvents->event, master, count);
     }
     return;
 }
-- 
1.6.0.4






More information about the xorg mailing list