From d82fe2e474afa2ccb6a9814f50eee9782d143b24 Mon Sep 17 00:00:00 2001 From: Soren Sandmann Date: Fri, 16 Nov 2007 07:47:22 +0000 Subject: [PATCH] Updates 2007-11-16 Soren Sandmann * TODO: Updates * process.c (process_locate_map): Move map to front * profile.c (profile_load): Ignore the toplevel field in the file since we can compute it ourselves. * stackstash.c (stack_stash_decorate): New function * stackstash.c (stack_stash_add_trace): Decorate the tree lazily instead of on each sample. svn path=/trunk/; revision=387 --- ChangeLog | 14 +++++ TODO | 5 ++ process.c | 16 +++++- profile.c | 4 +- stackstash.c | 158 ++++++++++++++++++++++++++++----------------------- 5 files changed, 120 insertions(+), 77 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7ba4201f..0ba1c02c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2007-11-16 Soren Sandmann + + * TODO: Updates + + * process.c (process_locate_map): Move map to front + + * profile.c (profile_load): Ignore the toplevel field in the file + since we can compute it ourselves. + + * stackstash.c (stack_stash_decorate): New function + + * stackstash.c (stack_stash_add_trace): Decorate the tree lazily + instead of on each sample. + 2007-10-22 Soren Sandmann * process.c (look_for_vmlinux): Use an array instead of a diff --git a/TODO b/TODO index 8ac70a47..b93de00c 100644 --- a/TODO +++ b/TODO @@ -23,6 +23,11 @@ Before 1.0.4: Before 1.2: +* If we profile something that is not very CPU bound, sysprof itself + seems to get a disproportionate amount of the samples. Should look into this. + +* Is the move-to-front in process_locate_map() really worth it? + * Apparently, if you upgrade the kernel, then don't re-run configure, the kernel Makefile will delete all of /lib/modules//kernel if you run make install in the module directory. Need to find out what diff --git a/process.c b/process.c index 5d60cffb..e04a3648 100644 --- a/process.c +++ b/process.c @@ -127,13 +127,13 @@ read_maps (int pid, int *n_maps) g_array_append_val (result, map); } } - + g_free (name); fclose (in); if (n_maps) *n_maps = result->len; - + return (Map *)g_array_free (result, FALSE); } @@ -241,6 +241,18 @@ process_locate_map (Process *process, gulong addr) if ((addr >= map->start) && (addr < map->end)) { + if (i > 4) + { + /* FIXME: Is this move-to-front really worth it? */ + Map tmp = *map; + + memmove (process->maps + 1, process->maps, i * sizeof (Map)); + + *(process->maps) = tmp; + + map = process->maps; + } + return map; } } diff --git a/profile.c b/profile.c index 145f3215..0e3fbfb8 100644 --- a/profile.c +++ b/profile.c @@ -191,7 +191,6 @@ profile_load (const char *filename, GError **err) for (i = 0; i < n; ++i) { StackNode *node = stack_node_new (profile->stash); - gboolean toplevel; gint32 size; gint32 total; @@ -203,10 +202,9 @@ profile_load (const char *filename, GError **err) sfile_get_pointer (input, "parent", (gpointer *)&node->parent); sfile_get_integer (input, "total", &total); sfile_get_integer (input, "self", (gint32 *)&size); - sfile_get_integer (input, "toplevel", &toplevel); + sfile_get_integer (input, "toplevel", NULL); node->total = total; - node->toplevel = toplevel; node->size = size; sfile_end_get (input, "node", node); diff --git a/stackstash.c b/stackstash.c index 8490270d..1b2a92fa 100644 --- a/stackstash.c +++ b/stackstash.c @@ -30,6 +30,80 @@ struct StackStash GPtrArray * blocks; }; +static void +decorate_node (StackNode *node, + StackStash *stash) +{ + StackNode *n; + + if (!node) + return; + + decorate_node (node->siblings, stash); + decorate_node (node->children, stash); + + node->next = g_hash_table_lookup (stash->nodes_by_data, node->address); + g_hash_table_insert (stash->nodes_by_data, node->address, node); + + /* FIXME: This could be done more efficiently + * by keeping track of the ancestors we have seen. + */ + node->toplevel = TRUE; + for (n = node->parent; n != NULL; n = n->parent) + { + if (n->address == node->address) + { + node->toplevel = FALSE; + break; + } + } +} + +static void +stack_stash_decorate (StackStash *stash) +{ + if (stash->nodes_by_data) + return; + + stash->nodes_by_data = g_hash_table_new (g_direct_hash, g_direct_equal); + + decorate_node (stash->root, stash); +} + +static void +free_key (gpointer key, + gpointer value, + gpointer data) +{ + GDestroyNotify destroy = data; + + destroy (key); +} + +static void +stack_stash_undecorate (StackStash *stash) +{ + if (stash->nodes_by_data) + { + if (stash->destroy) + { + g_hash_table_foreach ( + stash->nodes_by_data, free_key, stash->destroy); + } + + g_hash_table_destroy (stash->nodes_by_data); + } +} + +static GHashTable * +get_nodes_by_data (StackStash *stash) +{ + if (!stash->nodes_by_data) + stack_stash_decorate (stash); + + return stash->nodes_by_data; +} + StackNode * stack_node_new (StackStash *stash) { @@ -73,7 +147,7 @@ create_stack_stash (GDestroyNotify destroy) StackStash *stash = g_new (StackStash, 1); stash->root = NULL; - stash->nodes_by_data = g_hash_table_new (g_direct_hash, g_direct_equal); + stash->nodes_by_data = NULL; stash->ref_count = 1; stash->destroy = destroy; @@ -91,28 +165,12 @@ stack_stash_new (GDestroyNotify destroy) } -static void -free_key (gpointer key, - gpointer value, - gpointer data) -{ - GDestroyNotify destroy = data; - - destroy (key); -} - static void stack_stash_free (StackStash *stash) { int i; - if (stash->destroy) - { - g_hash_table_foreach (stash->nodes_by_data, free_key, - stash->destroy); - } - - g_hash_table_destroy (stash->nodes_by_data); + stack_stash_undecorate (stash); for (i = 0; i < stash->blocks->len; ++i) g_free (stash->blocks->pdata[i]); @@ -122,33 +180,6 @@ stack_stash_free (StackStash *stash) g_free (stash); } -static void -decorate_node (StackStash *stash, - StackNode *node) -{ - StackNode *n; - gboolean toplevel = TRUE; - - /* FIXME: we will probably want to do this lazily, - * and more efficiently (only walk the tree once). - */ - for (n = node->parent; n != NULL; n = n->parent) - { - if (n->address == node->address) - { - toplevel = FALSE; - break; - } - } - - node->toplevel = toplevel; - - node->next = g_hash_table_lookup ( - stash->nodes_by_data, node->address); - g_hash_table_insert ( - stash->nodes_by_data, node->address, node); -} - void stack_stash_add_trace (StackStash *stash, gulong *addrs, @@ -161,6 +192,9 @@ stack_stash_add_trace (StackStash *stash, if (!n_addrs) return; + + if (stash->nodes_by_data) + stack_stash_undecorate (stash); for (i = n_addrs - 1; i >= 0; --i) { @@ -168,7 +202,7 @@ stack_stash_add_trace (StackStash *stash, StackNode *prev; prev = NULL; - for (match = *location; match != NULL; prev = match, match = match->siblings) + for (match = *location; match; prev = match, match = match->siblings) { if (match->address == (gpointer)addrs[i]) { @@ -191,8 +225,6 @@ stack_stash_add_trace (StackStash *stash, match->siblings = *location; match->parent = parent; *location = match; - - decorate_node (stash, match); } match->total += size; @@ -235,9 +267,9 @@ do_callback (StackNode *node, } void -stack_stash_foreach (StackStash *stash, - StackFunction stack_func, - gpointer data) +stack_stash_foreach (StackStash *stash, + StackFunction stack_func, + gpointer data) { do_callback (stash->root, NULL, stack_func, data); } @@ -280,7 +312,7 @@ stack_stash_find_node (StackStash *stash, { g_return_val_if_fail (stash != NULL, NULL); - return g_hash_table_lookup (stash->nodes_by_data, data); + return g_hash_table_lookup (get_nodes_by_data (stash), data); } typedef struct @@ -306,38 +338,20 @@ stack_stash_foreach_by_address (StackStash *stash, info.func = func; info.data = data; - g_hash_table_foreach (stash->nodes_by_data, do_foreach, &info); + g_hash_table_foreach (get_nodes_by_data (stash), do_foreach, &info); } StackNode * -stack_stash_get_root (StackStash *stash) +stack_stash_get_root (StackStash *stash) { return stash->root; } -static void -build_hash_table (StackNode *node, - StackStash *stash) -{ - if (!node) - return; - - build_hash_table (node->siblings, stash); - build_hash_table (node->children, stash); - - node->next = g_hash_table_lookup ( - stash->nodes_by_data, node->address); - g_hash_table_insert ( - stash->nodes_by_data, node->address, node); -} - void stack_stash_set_root (StackStash *stash, StackNode *root) { g_return_if_fail (stash->root == NULL); - g_return_if_fail (g_hash_table_size (stash->nodes_by_data) == 0); stash->root = root; - build_hash_table (stash->root, stash); }