Add "total" field.

Mon Oct 31 23:41:33 2005  Soeren Sandmann  <sandmann@redhat.com>

        * stackstash.h (struct StackNode): Add "total" field.

        * stackstash.c (stack_stash_add_trace): Keep track of the
        aggregate size.

        * profile.c (profile_get_size): Sum the totals of the siblings
        instead of all the children.

        * profile.c (build_object_list): Correctly compute obj->self

        * profile.c (profile_load): Don't ignore the node->total field.

        * profile.c (serialize_call_tree): Save node->total instead of the
        computed total

        * profile.c (compute_total): Use n->total instead of computing it
        from scratch.

        * profile.c: Remove unused sum_children() function.

        * TODO: Updates

        * process.c (process_get_from_pid): Plug leak.

diff --git a/ChangeLog b/ChangeLog
index 6a8611c..414ca87 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,29 @@
+Mon Oct 31 23:41:33 2005  Soeren Sandmann  <sandmann@redhat.com>
+
+       * stackstash.h (struct StackNode): Add "total" field.
+
+       * stackstash.c (stack_stash_add_trace): Keep track of the
+       aggregate size.
+
+       * profile.c (profile_get_size): Sum the totals of the siblings
+       instead of all the children.
+
+       * profile.c (build_object_list): Correctly compute obj->self
+
+       * profile.c (profile_load): Don't ignore the node->total field.
+
+       * profile.c (serialize_call_tree): Save node->total instead of the
+       computed total
+
+       * profile.c (compute_total): Use n->total instead of computing it
+       from scratch.
+
+       * profile.c: Remove unused sum_children() function.
+
+       * TODO: Updates
+
+       * process.c (process_get_from_pid): Plug leak.
+
 Mon Oct 31 21:36:37 2005  Søren Sandmann  <sandmann@redhat.com>
 
        * sysprof.c (fill_main_list): free the profile objects.

diff --git a/TODO b/TODO
index 3b8b644..89ad414 100644 (file)
--- a/TODO
+++ b/TODO
@@ -15,6 +15,8 @@ Before 1.0.1:
 
 Before 1.2:
 
+* Don't build the GUI if gtk+ is not installed
+
 * Handle time being set back in the RESET_DEAD_PERIOD code.
 
 * Find out if the first sort order of a GtkTreeView column can be changed
@@ -31,8 +33,6 @@ Before 1.2:
   (Reported by Kjartan Maraas).
 
 - Fix bugs/performance issues:
-       - total should probably be cached so that compute_total() doesn't
-         take 80% of the time to generate a profile.
        - decorate_node should be done lazily
        - Find out why we sometimes get completely ridicoulous stacktraces,
          where main seems to be called from within Xlib etc. This happens
@@ -47,6 +47,9 @@ Before 1.2:
                process b spends 1% in foo() called from baz()
          we get reports of baz() using > 80% of the time.
          Or something.
+       - add_trace_to_tree() might be a little slow when dealing with deeply
+         recursive profiles. Hypothesis: seen_nodes can grow large, and the
+         algorithm is O(n^2) in the length of the trace.
 
 - make the things we put in a stackstash real
   objects so that
@@ -422,6 +425,9 @@ Later:
 
 DONE:
 
+- total should probably be cached so that compute_total() doesn't
+  take 80% of the time to generate a profile.
+
 - Fixing the oops in kernels < 2.6.11
 
        - Probably just require 2.6.11 (necessary for timer interrupt

diff --git a/collector.c b/collector.c
index 75fa816..8e89282 100644 (file)
--- a/collector.c
+++ b/collector.c
@@ -100,6 +100,7 @@ on_read (gpointer data)
     /* After a reset we ignore samples for a short period so that
      * a reset will actually cause 'samples' to become 0
      */
+    /* FIXME: handle time getting set back */
     if (time_diff (&now, &collector->latest_reset) < RESET_DEAD_PERIOD)
        return;
     

diff --git a/process.c b/process.c
index 657ecce..391c292 100644 (file)
--- a/process.c
+++ b/process.c
@@ -334,11 +334,13 @@ process_get_from_pid (int pid)
     
     cmdline = get_name (pid);
 
+    idle_free (cmdline);
+    
     p = g_hash_table_lookup (processes_by_cmdline, cmdline);
-    if (p)
-       return p;
-    else
-       return create_process (idle_free (cmdline), pid);
+    if (!p)
+       p = create_process (cmdline, pid);
+
+    return p;
 }
 
 const Symbol *

diff --git a/profile.c b/profile.c
index 82b3eae..366a60b 100644 (file)
--- a/profile.c
+++ b/profile.c
@@ -78,32 +78,15 @@ create_format (void)
 }
 
 static int
-sum_children (StackNode *node)
-{
-    int total;
-    StackNode *child;
-    
-    /* FIXME: this is pretty inefficient. Instead perhaps
-     * maintain or compute it in the stackstash
-     */
-    total = node->size;
-    
-    for (child = node->children; child != NULL; child = child->siblings)
-       total += sum_children (child);
-    
-    return total;
-}
-
-static int
 compute_total (StackNode *node)
 {
     StackNode *n;
     int total = 0;
-    
+
     for (n = node; n != NULL; n = n->next)
     {
        if (n->toplevel)
-           total += sum_children (n);
+           total += n->total;
     }
     
     return total;
@@ -121,7 +104,7 @@ serialize_call_tree (StackNode *node,
     sfile_add_pointer (output, "siblings", node->siblings);
     sfile_add_pointer (output, "children", node->children);
     sfile_add_pointer (output, "parent", node->parent);
-    sfile_add_integer (output, "total", compute_total (node));
+    sfile_add_integer (output, "total", node->total);
     sfile_add_integer (output, "self", node->size);
     sfile_add_integer (output, "toplevel", node->toplevel);
     sfile_end_add (output, "node", node);
@@ -249,7 +232,7 @@ profile_load (const char *filename, GError **err)
        sfile_get_pointer (input, "siblings", (gpointer *)&node->siblings);
        sfile_get_pointer (input, "children", (gpointer *)&node->children);
        sfile_get_pointer (input, "parent", (gpointer *)&node->parent);
-       sfile_get_integer (input, "total", NULL);
+       sfile_get_integer (input, "total", &node->total);
        sfile_get_integer (input, "self", (gint32 *)&node->size);
        sfile_get_integer (input, "toplevel", &node->toplevel);
        
@@ -566,16 +549,16 @@ build_object_list (StackNode *node, gpointer data)
 {
     GList **objects = data;
     ProfileObject *obj;
+    StackNode *n;
     
     obj = g_new (ProfileObject, 1);
     obj->name = node->address;
     
     obj->total = compute_total (node);
-    
-    /* FIXME: this is incorrect. We need to sum all the node linked
-     * through node->next
-     */
-    obj->self = node->size;
+
+    obj->self = 0;
+    for (n = node; n != NULL; n = n->siblings)
+       obj->self += n->size;
     
     *objects = g_list_prepend (*objects, obj);
 }
@@ -597,5 +580,11 @@ profile_get_objects (Profile *profile)
 gint
 profile_get_size (Profile *profile)
 {
-    return compute_total (stack_stash_get_root (profile->stash));
+    StackNode *n;
+    gint size = 0;
+    
+    for (n = stack_stash_get_root (profile->stash); n != NULL; n = n->siblings)
+       size += compute_total (n);
+
+    return size;
 }

diff --git a/stackstash.c b/stackstash.c
index b717bfc..7975760 100644 (file)
--- a/stackstash.c
+++ b/stackstash.c
@@ -35,6 +35,7 @@ stack_node_new (void)
     node->parent = NULL;
     node->size = 0;
     node->next = NULL;
+    node->total = 0;
     return node;
 }
 
@@ -46,7 +47,7 @@ stack_stash_new       (void)
 
     stash->root = NULL;
     stash->nodes_by_data = g_hash_table_new (g_direct_hash, g_direct_equal);
-    
+
     return stash;
 }
 
@@ -116,6 +117,8 @@ stack_stash_add_trace (StackStash *stash,
            decorate_node (stash, match);
        }
 
+       match->total += size;
+
        location = &(match->children);
        parent = match;
     }

diff --git a/stackstash.h b/stackstash.h
index 1372be7..a75b339 100644 (file)
--- a/stackstash.h
+++ b/stackstash.h
@@ -29,6 +29,7 @@ typedef struct StackNode StackNode;
 struct StackNode
 {
     gpointer   address;
+    int                total;
     int                size;
     
     StackNode *        parent;