diff --git a/ChangeLog b/ChangeLog index 141f140b..95d93aac 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2006-07-31 Soren Sandmann + + * stackstash.[ch]: Add a destroy notifier to StackStash + + * collector.c (collector_create_profile): Pass g_free as destroy + notifier. + + * collector.c (collector_reset): Pass NULL as destroy notifier + + * profile.c (profile_load): Pass g_free here. + + * profile.c (struct Profile): Remove unused "Node" typedef + + * collector.c (resolve_symbols): Free the array here. + + * TODO: various updates. + 2006-07-30 Soren Sandmann * signal-handler.c: Simplify this file a bit, and make it not rely diff --git a/TODO b/TODO index 85c77928..b0938129 100644 --- a/TODO +++ b/TODO @@ -96,6 +96,11 @@ Before 1.2: - plug all the leaks - don't leak the presentation strings/objects + - maybe add stack_stash_set_free_func() or something + - perhaps think a little bit more about the presentation + object idea. Having pango markup available might be nice + as well ... Plus the unique_dup() scheme is somewhat + confusing. - loading and saving probably leak right now - rethink loading and saving. Goals @@ -146,7 +151,22 @@ Before 1.2: input and output. As a first cut, perhaps just split out the Instruction code. + (done, somewhat). + - make the api saner; add format/content structs + Idea: all types have to be declared: + + SFormat *format = sformat_new() + SType *object_list = stype_new (format); + SType *object = stype_new (format); + ... + + stype_define_list (object_list, "objects", object); + stype_define_record (object, "object", + name, total, self, NULL); + stype_define_pointer (...); + + * See if the auto-expanding can be made more intelligent - "Everything" should be expanded exactly one level diff --git a/collector.c b/collector.c index 5bad4efd..a72c5be0 100644 --- a/collector.c +++ b/collector.c @@ -246,7 +246,7 @@ collector_reset (Collector *collector) process_flush_caches(); - collector->stash = stack_stash_new (); + collector->stash = stack_stash_new (NULL); collector->n_samples = 0; g_get_current_time (&collector->latest_reset); @@ -315,6 +315,8 @@ resolve_symbols (GList *trace, gint size, gpointer data) stack_stash_add_trace (info->resolved_stash, (gulong *)resolved_trace->pdata, resolved_trace->len, size); + + g_ptr_array_free (resolved_trace, TRUE); } Profile * @@ -323,10 +325,12 @@ collector_create_profile (Collector *collector) ResolveInfo info; Profile *profile; - info.resolved_stash = stack_stash_new (); + info.resolved_stash = stack_stash_new ((GDestroyNotify)g_free); info.unique_symbols = g_hash_table_new (g_direct_hash, g_direct_equal); stack_stash_foreach (collector->stash, resolve_symbols, &info); + + /* FIXME: we are leaking the value strings in info.unique_symbols */ g_hash_table_destroy (info.unique_symbols); diff --git a/profile.c b/profile.c index 1f935e5b..1a874918 100644 --- a/profile.c +++ b/profile.c @@ -27,8 +27,6 @@ #include "profile.h" #include "sfile.h" -typedef struct Node Node; - struct Profile { StackStash *stash; @@ -227,7 +225,7 @@ profile_load (const char *filename, GError **err) sformat_free (format); sfile_input_free (input); - profile->stash = stack_stash_new_from_root (root); + profile->stash = stack_stash_new_from_root (root, (GDestroyNotify)g_free); return profile; } @@ -518,10 +516,6 @@ profile_get_objects (Profile *profile) stack_stash_foreach_by_address ( profile->stash, build_object_list, &objects); - /* FIXME: everybody still assumes that they don't have to free the - * objects in the list, but these days they do, and so we are leaking. - */ - return objects; } diff --git a/signal-handler.c b/signal-handler.c index a80ae2e2..a241d253 100644 --- a/signal-handler.c +++ b/signal-handler.c @@ -85,6 +85,12 @@ signal_handler (int signo, { /* FIXME: I suppose we should handle short * and non-successful writes ... + * + * And also, there is a deadlock if so many signals arrive that + * write() blocks. Then we will be stuck right here, and the + * main loop will never run. Kinda hard to fix without dropping + * signals ... + * */ write (write_end, &signo, sizeof (int)); } @@ -190,7 +196,7 @@ signal_set_handler (int signo, return FALSE; } - + return TRUE; } diff --git a/stackstash.c b/stackstash.c index ba1a4cc3..21e2f6b6 100644 --- a/stackstash.c +++ b/stackstash.c @@ -21,9 +21,10 @@ struct StackStash { - StackNode *root; - GHashTable *nodes_by_data; - int ref_count; + int ref_count; + StackNode * root; + GHashTable * nodes_by_data; + GDestroyNotify destroy; }; static StackNode * @@ -40,23 +41,25 @@ stack_node_new (void) return node; } +/* "destroy", if non-NULL, is called once on every address */ static StackStash * -create_stack_stash (void) +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->ref_count = 1; + stash->destroy = destroy; return stash; } /* Stach */ StackStash * -stack_stash_new (void) +stack_stash_new (GDestroyNotify destroy) { - return create_stack_stash(); + return create_stack_stash (destroy); } void @@ -79,7 +82,7 @@ decorate_node (StackStash *stash, } node->toplevel = toplevel; - + node->next = g_hash_table_lookup ( stash->nodes_by_data, node->address); g_hash_table_insert ( @@ -195,10 +198,27 @@ stack_node_free (StackNode *node) g_free (node); } +static void +free_key (gpointer key, + gpointer value, + gpointer data) +{ + GDestroyNotify destroy = data; + + destroy (key); +} + static void stack_stash_free (StackStash *stash) { stack_node_free (stash->root); + + if (stash->destroy) + { + g_hash_table_foreach (stash->nodes_by_data, free_key, + stash->destroy); + } + g_hash_table_destroy (stash->nodes_by_data); g_free (stash); @@ -277,9 +297,10 @@ build_hash_table (StackNode *node, } StackStash * -stack_stash_new_from_root (StackNode *root) +stack_stash_new_from_root (StackNode *root, + GDestroyNotify destroy) { - StackStash *stash = create_stack_stash(); + StackStash *stash = create_stack_stash (destroy); stash->root = root; diff --git a/stackstash.h b/stackstash.h index f965d4e1..6ea5ee43 100644 --- a/stackstash.h +++ b/stackstash.h @@ -48,7 +48,9 @@ typedef void (* StackNodeFunc) (StackNode *node, gpointer data); /* Stach */ -StackStash *stack_stash_new (void); +StackStash *stack_stash_new (GDestroyNotify destroy); +StackStash *stack_stash_new_from_root (StackNode *root, + GDestroyNotify destroy); void stack_stash_add_trace (StackStash *stash, gulong *addrs, gint n_addrs, @@ -65,7 +67,6 @@ void stack_stash_foreach_by_address (StackStash *stash, StackNodeFunc func, gpointer data); StackNode *stack_stash_get_root (StackStash *stash); -StackStash *stack_stash_new_from_root (StackNode *root); StackStash *stack_stash_ref (StackStash *stash); void stack_stash_unref (StackStash *stash); diff --git a/sysprof.c b/sysprof.c index 6fe84afb..5c97f0d3 100644 --- a/sysprof.c +++ b/sysprof.c @@ -1558,6 +1558,14 @@ main (int argc, gtk_init (&argc, &argv); +#if 0 + /* FIXME: enable this when compiled against the relevant glib + * version. (The reason we want to enable it is that gslice + * caches too much memory and also confuses valgrind). + */ + g_slice_set_config (G_SLICE_CONFIG_ALWAYS_MALLOC, TRUE); +#endif + app = application_new (); if (!build_gui (app))