From c778a1b1ecd8daf2dffe3dbe09284128a1e9d20a Mon Sep 17 00:00:00 2001 From: Soeren Sandmann Date: Tue, 1 Nov 2005 04:37:16 +0000 Subject: [PATCH] Add "total" field. Mon Oct 31 23:41:33 2005 Soeren Sandmann * 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. --- ChangeLog | 26 ++++++++++++++++++++++++++ TODO | 10 ++++++++-- collector.c | 1 + process.c | 10 ++++++---- profile.c | 43 ++++++++++++++++--------------------------- stackstash.c | 5 ++++- stackstash.h | 1 + 7 files changed, 62 insertions(+), 34 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6a8611c..414ca87 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,29 @@ +Mon Oct 31 23:41:33 2005 Soeren Sandmann + + * 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 * sysprof.c (fill_main_list): free the profile objects. diff --git a/TODO b/TODO index 3b8b644..89ad414 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- a/stackstash.h +++ b/stackstash.h @@ -29,6 +29,7 @@ typedef struct StackNode StackNode; struct StackNode { gpointer address; + int total; int size; StackNode * parent; -- 2.7.4