> I compiled dbmail and glib with profiling and gprof shows almost all
> time being spent in IA__g_list_last.  (Note the CPU time reported by
> time was 277.17s -- the rest is probably in glibc and mysql libs..)
> 
>  time   seconds   seconds    calls   s/call   s/call  name    
>  99.35    272.99   272.99   344704     0.00     0.00  IA__g_list_last
>   0.20    273.54     0.55 18702183     0.00     0.00  ucmp
>   0.10    273.82     0.28   344681     0.00     0.00  g_tree_node_insert
>   0.06    273.99     0.17   860545     0.00     0.00  g_tree_node_lookup
>   0.05    274.12     0.13   344771     0.00     0.00  _g_list_alloc
>   0.03    274.20     0.08       14     0.01     0.01  db_getmailbox
> (full profile log attached)

OK -- O(n^2) behavior is bad.  The traverse_tree_{keys, values, merge}
functions all ended up appending message pointers to the end of a linked
list.  When you do this for 80,000 messages, you end up following about
3.2 billion pointers.. Yuck.

Attached patch created copies of these functions (traverse_tree_keys_rev
etc) which prepends list items and then uses a g_list_reverse to sort
out the mess.  It seems to work for me but I've given it minimal
testing.

There are better ways of doing this, but for me this reduces the above
mailbox query from 272cpusec to 5.1cpusec, for a tidy 50x speedup.

Matt
diff -ur dbmail-2.1.5/dbmail-mailbox.c dbmail-2.1.5-new/dbmail-mailbox.c
--- dbmail-2.1.5/dbmail-mailbox.c       2006-02-12 10:08:02.000000000 -0600
+++ dbmail-2.1.5-new/dbmail-mailbox.c   2006-03-06 21:34:48.000000000 -0600
@@ -999,8 +999,9 @@
                id = g_new0(u64_t,1);
                *id = db_get_result_u64(i,0);
                if (g_tree_lookup(self->ids,id))
-                       self->sorted = g_list_append(self->sorted,id);
+                       self->sorted = g_list_prepend(self->sorted,id);
        }
+       g_list_reverse(self->sorted);
        g_string_free(q,TRUE);
        db_free_result();
        
diff -ur dbmail-2.1.5/list.c dbmail-2.1.5-new/list.c
--- dbmail-2.1.5/list.c 2005-10-03 05:19:40.000000000 -0500
+++ dbmail-2.1.5-new/list.c     2006-03-03 11:22:17.000000000 -0600
@@ -36,6 +36,9 @@
  */
 void dm_list_free(struct element **start)
 {
+       if (!start || !(*start)) 
+               return;
+
        if (!(*start))
                return;
 
diff -ur dbmail-2.1.5/misc.c dbmail-2.1.5-new/misc.c
--- dbmail-2.1.5/misc.c 2006-02-28 14:59:51.000000000 -0600
+++ dbmail-2.1.5-new/misc.c     2006-03-06 21:47:01.000000000 -0600
@@ -1279,22 +1279,39 @@
        *(GList **)l = g_list_append(*(GList **)l, key);
        return FALSE;
 }
+
+static gboolean traverse_tree_keys_rev(gpointer key, gpointer value UNUSED, 
GList **l)
+{
+       *(GList **)l = g_list_prepend(*(GList **)l, key);
+       return FALSE;
+}
+
 static gboolean traverse_tree_values(gpointer key UNUSED, gpointer value, 
GList **l)
 {
        *(GList **)l = g_list_append(*(GList **)l, value);
        return FALSE;
 }
 
+static gboolean traverse_tree_values_rev(gpointer key UNUSED, gpointer value, 
GList **l)
+{
+       *(GList **)l = g_list_prepend(*(GList **)l, value);
+       return FALSE;
+}
+
+
+
 GList * g_tree_keys(GTree *tree)
 {
        GList *l = NULL;
-       g_tree_foreach(tree, (GTraverseFunc)traverse_tree_keys, &l);
+       g_tree_foreach(tree, (GTraverseFunc)traverse_tree_keys_rev, &l);
+       g_list_reverse(l);
        return g_list_first(l);
 }
 GList * g_tree_values(GTree *tree)
 {
        GList *l = NULL;
-       g_tree_foreach(tree, (GTraverseFunc)traverse_tree_values, &l);
+       g_tree_foreach(tree, (GTraverseFunc)traverse_tree_values_rev, &l);
+       g_list_reverse(l);
        return g_list_first(l);
 }
 
@@ -1347,6 +1364,28 @@
        return FALSE;
 }
 
+static gboolean traverse_tree_merger_rev(gpointer key, gpointer value UNUSED, 
tree_merger_t **merger)
+{
+       tree_merger_t *t = *(tree_merger_t **)merger;
+       GTree *tree = t->tree;
+       int condition = t->condition;
+
+       switch(condition) {
+               case IST_SUBSEARCH_NOT:
+               break;
+               
+               default:
+               case IST_SUBSEARCH_OR:
+               case IST_SUBSEARCH_AND:
+                       if (! g_tree_lookup(tree,key)) 
+                               (*(tree_merger_t **)merger)->list = 
g_list_prepend((*(tree_merger_t **)merger)->list,key);
+               break;
+       }
+
+       return FALSE;
+}
+
+
 
 int g_tree_merge(GTree *a, GTree *b, int condition)
 {
@@ -1370,7 +1409,8 @@
                        /* delete from A all keys not in B */
                        merger->tree = b;
                        merger->condition = IST_SUBSEARCH_AND;
-                       g_tree_foreach(a,(GTraverseFunc)traverse_tree_merger, 
&merger);
+                       
g_tree_foreach(a,(GTraverseFunc)traverse_tree_merger_rev, &merger);
+                       g_list_reverse(merger->list);
                        
                        keys = g_list_first(merger->list);
                        if (! g_list_length(keys))
@@ -1393,7 +1433,8 @@
 
                        merger->tree = a;
                        merger->condition = IST_SUBSEARCH_OR;
-                       g_tree_foreach(b,(GTraverseFunc)traverse_tree_merger, 
&merger);
+                       
g_tree_foreach(b,(GTraverseFunc)traverse_tree_merger_rev, &merger);
+                       g_list_reverse(merger->list);
                        keys = g_list_first(merger->list);
                
                        /* add to A all keys in B */

Reply via email to