change to add only one _XAsyncHandler per display, speeding things up a

2002-10-06  Havoc Pennington  <hp@pobox.com>

	* src/async-getprop.c: change to add only one _XAsyncHandler per
	display, speeding things up a bit.

ChangeLog

@@ -1,3 +1,8 @@
+2002-10-06  Havoc Pennington  <hp@pobox.com>
+
+	* src/async-getprop.c: change to add only one _XAsyncHandler per
+	display, speeding things up a bit.
+
 2002-10-06  Havoc Pennington  <hp@pobox.com>
 
 	* src/async-getprop.c: Add wacky hack suggested by Keith Packard

src/async-getprop.c

@@ -43,14 +43,22 @@
 #define NULL ((void*)0)
 #endif
 
+typedef struct _ListNode ListNode;
+typedef struct _AgPerDisplayData AgPerDisplayData;
+
+struct _ListNode
+{
+  ListNode *next;
+};
+
 struct _AgGetPropertyTask
 {
-  Display *display;
+  ListNode node;
+  
+  AgPerDisplayData *dd;
   Window window;
   Atom property;
 
-  _XAsyncHandler async;
-
   unsigned long request_seq;
   int error;
 
@@ -62,21 +70,29 @@ struct _AgGetPropertyTask
   unsigned char *data;
 
   Bool have_reply;
-
-  AgGetPropertyTask *next;
 };
 
-static AgGetPropertyTask *pending_tasks = NULL;
+struct _AgPerDisplayData
-static AgGetPropertyTask *pending_tasks_tail = NULL;
+{
-static AgGetPropertyTask *completed_tasks = NULL;
+  ListNode node;
-static AgGetPropertyTask *completed_tasks_tail = NULL;
+  _XAsyncHandler async;
-static int n_tasks_pending = 0;
+  
-static int n_tasks_completed = 0;
+  Display *display;
+  ListNode *pending_tasks;
+  ListNode *pending_tasks_tail;
+  ListNode *completed_tasks;
+  ListNode *completed_tasks_tail;
+  int n_tasks_pending;
+  int n_tasks_completed;
+};
+
+static ListNode *display_datas = NULL;
+static ListNode *display_datas_tail = NULL;
 
 static void
-append_to_list (AgGetPropertyTask **head,
+append_to_list (ListNode **head,
-                AgGetPropertyTask **tail,
+                ListNode **tail,
-                AgGetPropertyTask  *task)
+                ListNode  *task)
 {
   task->next = NULL;
   
@@ -94,12 +110,12 @@ append_to_list (AgGetPropertyTask **head,
 }
 
 static void
-remove_from_list (AgGetPropertyTask **head,
+remove_from_list (ListNode **head,
-                  AgGetPropertyTask **tail,
+                  ListNode **tail,
-                  AgGetPropertyTask  *task)
+                  ListNode  *task)
 {
-  AgGetPropertyTask *prev;
+  ListNode *prev;
-  AgGetPropertyTask *node;
+  ListNode *node;
 
   prev = NULL;
   node = *head;
@@ -129,18 +145,60 @@ remove_from_list (AgGetPropertyTask **head,
 }
 
 static void
-move_to_completed (AgGetPropertyTask *task)
+move_to_completed (AgPerDisplayData  *dd,
+                   AgGetPropertyTask *task)
 {
-  remove_from_list (&pending_tasks,
+  remove_from_list (&dd->pending_tasks,
-                    &pending_tasks_tail,
+                    &dd->pending_tasks_tail,
-                    task);
+                    &task->node);
   
-  append_to_list (&completed_tasks,
+  append_to_list (&dd->completed_tasks,
-                  &completed_tasks_tail,
+                  &dd->completed_tasks_tail,
-                  task);
+                  &task->node);
 
-  --n_tasks_pending;
+  dd->n_tasks_pending -= 1;
-  ++n_tasks_completed;
+  dd->n_tasks_completed += 1;
+}
+
+static AgGetPropertyTask*
+find_pending_by_request_sequence (AgPerDisplayData *dd,
+                                  unsigned long     request_seq)
+{
+  ListNode *node;
+
+  /* if the sequence is after our last pending task, we
+   * aren't going to find a match
+   */
+  {
+    AgGetPropertyTask *task = (AgGetPropertyTask*) dd->pending_tasks_tail;
+    if (task != NULL)
+      {
+        if (task->request_seq < request_seq)
+          return NULL;
+        else if (task->request_seq == request_seq)
+          return task; /* why not check this */
+      }
+  }
+
+  /* Generally we should get replies in the order we sent
+   * requests, so we should usually be using the task
+   * at the head of the list, if we use any task at all.
+   * I'm not sure this is 100% guaranteed, if it is,
+   * it would be a big speedup.
+   */
+  
+  node = dd->pending_tasks;
+  while (node != NULL)
+    {
+      AgGetPropertyTask *task = (AgGetPropertyTask*) node;
+      
+      if (task->request_seq == request_seq)
+        return task;
+      
+      node = node->next;
+    }
+  
+  return NULL;
 }
 
 static Bool
@@ -153,20 +211,25 @@ async_get_property_handler (Display *dpy,
   xGetPropertyReply  replbuf;
   xGetPropertyReply *reply;
   AgGetPropertyTask *task;
+  AgPerDisplayData *dd;
   int bytes_read;
 
-  task = (AgGetPropertyTask*) data;
+  dd = (AgPerDisplayData*) data;
   
 #if 0
-  printf ("%s: waiting for %ld seeing %ld buflen %d\n", __FUNCTION__,
+  printf ("%s: seeing request seq %ld buflen %d\n", __FUNCTION__,
-          task->request_seq, dpy->last_request_read, len);
+          dpy->last_request_read, len);
 #endif
   
-  if (dpy->last_request_read != task->request_seq)
+  task = find_pending_by_request_sequence (dd, dpy->last_request_read);
+
+  if (task == NULL)
     return False;
 
+  assert (dpy->last_request_read == task->request_seq);
+
   task->have_reply = True;
-  move_to_completed (task);
+  move_to_completed (dd, task);
   
   /* read bytes so far */
   bytes_read = SIZEOF (xReply);
@@ -366,6 +429,57 @@ async_get_property_handler (Display *dpy,
   return True;
 }
 
+static AgPerDisplayData*
+get_display_data (Display *display,
+                  Bool     create)
+{
+  ListNode *node;
+  AgPerDisplayData *dd;
+  
+  node = display_datas;
+  while (node != NULL)
+    {
+      dd = (AgPerDisplayData*) node;
+      
+      if (dd->display == display)
+        return dd;
+      
+      node = node->next;
+    }
+
+  if (!create)
+    return NULL;
+  
+  dd = Xcalloc (1, sizeof (AgPerDisplayData));
+  if (dd == NULL)
+    return NULL;
+
+  dd->display = display;
+  dd->async.next = display->async_handlers;
+  dd->async.handler = async_get_property_handler;
+  dd->async.data = (XPointer) dd;
+  dd->display->async_handlers = &dd->async;
+
+  append_to_list (&display_datas,
+                  &display_datas_tail,
+                  &dd->node);
+  
+  return dd;
+}
+
+static void
+maybe_free_display_data (AgPerDisplayData *dd)
+{
+  if (dd->pending_tasks == NULL &&
+      dd->completed_tasks == NULL)
+    {
+      DeqAsyncHandler (dd->display, &dd->async);
+      remove_from_list (&display_datas, &display_datas_tail,
+                        &dd->node);
+      XFree (dd);
+    }
+}
+
 AgGetPropertyTask*
 ag_task_create (Display *dpy,
                 Window   window,
@@ -378,9 +492,18 @@ ag_task_create (Display *dpy,
   AgGetPropertyTask *task;
   xGetPropertyReq *req;
   xError error;
+  AgPerDisplayData *dd;  
   
   /* Fire up our request */
   LockDisplay (dpy);
+
+  dd = get_display_data (dpy, True);
+  if (dd == NULL)
+    {
+      UnlockDisplay (dpy);
+      return NULL;
+    }
+  
   GetReq (GetProperty, req);
   req->window = window;
   req->property = property;
@@ -399,19 +522,15 @@ ag_task_create (Display *dpy,
       return NULL;
     }
 
-  task->display = dpy;
+  task->dd = dd;
   task->window = window;
   task->property = property;
   task->request_seq = dpy->request;
-  task->async.next = dpy->async_handlers;
-  task->async.handler = async_get_property_handler;
-  task->async.data = (XPointer) task;
-  dpy->async_handlers = &task->async;
 
-  append_to_list (&pending_tasks,
+  append_to_list (&dd->pending_tasks,
-                  &pending_tasks_tail,
+                  &dd->pending_tasks_tail,
-                  task);
+                  &task->node);
-  ++n_tasks_pending;
+  dd->n_tasks_pending += 1;
   
   UnlockDisplay (dpy);
 
@@ -420,6 +539,17 @@ ag_task_create (Display *dpy,
   return task;
 }
 
+static void
+free_task (AgGetPropertyTask *task)
+{
+  remove_from_list (&task->dd->completed_tasks,
+                    &task->dd->completed_tasks_tail,
+                    &task->node);
+  task->dd->n_tasks_completed -= 1;
+  maybe_free_display_data (task->dd);
+  XFree (task);
+}
+
 Status
 ag_task_get_reply_and_free (AgGetPropertyTask  *task,
                             Atom               *actual_type,
@@ -432,24 +562,21 @@ ag_task_get_reply_and_free (AgGetPropertyTask  *task,
 
   *prop = NULL;
 
-  dpy = task->display; /* Xlib macros require a variable named "dpy" */
+  dpy = task->dd->display; /* Xlib macros require a variable named "dpy" */
   
   if (task->error != Success)
     {
       Status s = task->error;
-      DeqAsyncHandler (task->display, &task->async);
+
-      remove_from_list (&completed_tasks, &completed_tasks_tail, task);
+      free_task (task);
-      --n_tasks_completed;
+      
-      XFree (task);
       return s;
     }
 
   if (!task->have_reply)
     {
-      DeqAsyncHandler (task->display, &task->async);
+      free_task (task);
-      remove_from_list (&completed_tasks, &completed_tasks_tail, task);
+
-      --n_tasks_completed;
-      XFree (task);
       return BadAlloc; /* not Success */
     }
 
@@ -462,10 +589,7 @@ ag_task_get_reply_and_free (AgGetPropertyTask  *task,
 
   SyncHandle ();
 
-  DeqAsyncHandler (dpy, &task->async);
+  free_task (task);
-  remove_from_list (&completed_tasks, &completed_tasks_tail, task);
-  --n_tasks_completed;
-  XFree (task);
   
   return Success;
 }
@@ -491,26 +615,24 @@ ag_task_get_window (AgGetPropertyTask *task)
 Display*
 ag_task_get_display (AgGetPropertyTask *task)
 {
-  return task->display;
+  return task->dd->display;
 }
 
 AgGetPropertyTask*
 ag_get_next_completed_task (Display *display)
 {
-  AgGetPropertyTask *node;
+  AgPerDisplayData *dd;
+
+  dd = get_display_data (display, False);
+
+  if (dd == NULL)
+    return NULL;
   
 #ifdef DEBUG_SPEW
-  printf ("%d pending %d completed\n", n_tasks_pending, n_tasks_completed);
+  printf ("%d pending %d completed\n",
+          dd->n_tasks_pending,
+          dd->n_tasks_completed);
 #endif
 
-  node = completed_tasks;
+  return (AgGetPropertyTask*) dd->completed_tasks;
-  while (node != NULL)
-    {
-      if (node->display == display)
-        return node;
-      
-      node = node->next;
-    }
-
-  return NULL;
 }

src/testasyncgetprop.c

@@ -374,12 +374,10 @@ run_speed_comparison (Display *xdisplay,
   int n_left;
   
   /* We just use atom values 0 to n_props, many are probably BadAtom,
-   * that's fine. The larger this number the worse the async case
+   * that's fine.
-   * looks; my guess is it's because of display->async_handlers
-   * becoming a very long list. Need to modify async-getprop.c
-   * to install only a single async handler per display.
    */
-  n_props = 200;
+  n_props = 4000;
+  printf ("Timing with %d property requests\n", n_props);
   
   gettimeofday (&start, NULL);