Make this a two-pass algorithm, one pass to add the trace, and one to do

2007-10-22  Soren Sandmann <sandmann@daimi.au.dk>

	* profile.c (add_trace_to_tree): Make this a two-pass
	algorithm, one pass to add the trace, and one to do the
	accounting.


svn path=/trunk/; revision=384

ChangeLog

@@ -1,3 +1,9 @@
+2007-10-22  Soren Sandmann <sandmann@daimi.au.dk>
+
+	* profile.c (add_trace_to_tree): Make this a two-pass
+	algorithm, one pass to add the trace, and one to do the
+	accounting.
+
 2007-10-22  Soren Sandmann <sandmann@daimi.au.dk>
 
 	* sfile.c: Fix some spelling errors

TODO

@@ -156,18 +156,9 @@ Before 1.2:
 	- etc.
   done: HEAD will not load files with the wrong inode now.
 
-* In profile.c, change "non_recursive" to "cumulative", and
-  "marked_non_recursive" to a boolean "charged". This is tricky code,
-  so be careful. Possibly make it a two-pass operation:
-     	- first add the new trace
-	- then walk from the leaf, charging nodes
-  That would allow us to get rid of the marked field altogether. In fact,
-  maybe the descendants tree could become a stackstash. We'll just have
-  to make stack_stash_add_trace() return the leaf.
-      Hmm, not quite - we still need the "go-back-on-recursion" behavior.
-  That could be added of course, but that gets complex.
-
-  DONE: the name is now "cumulative"
+* Consider whether ProfileDescendant can be done with a StackStash We
+  need the "go-back-on-recursion" behavior.  That could be added of
+  course ... the functions are otherwise very similar.
 
 * Add spew infrastructure to make remote debugging easier.
 
@@ -752,6 +743,17 @@ Later:
 
 -=-=-=-=-=-=-=-=-=-=-=-=-=-=- ALREADY DONE: -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
 
+* In profile.c, change "non_recursive" to "cumulative", and
+  "marked_non_recursive" to a boolean "charged". This is tricky code,
+  so be careful. Possibly make it a two-pass operation:
+     	- first add the new trace
+	- then walk from the leaf, charging nodes
+  That would allow us to get rid of the marked field altogether. In fact,
+  maybe the descendants tree could become a stackstash. We'll just have
+  to make stack_stash_add_trace() return the leaf.
+
+  DONE: the name is now "cumulative"
+
 
 * vdso
 	- assume its the same across processes, just look at

profile.c

@@ -237,21 +237,16 @@ profile_new (StackStash *stash)
 static void
 add_trace_to_tree (GList *trace, gint size, gpointer data)
 {
-    static GPtrArray *nodes_to_unmark;
     GList *list;
     ProfileDescendant *parent = NULL;
-    int i, len;
-    ProfileDescendant **tree = data;    
+    ProfileDescendant **tree = data;
     
-    if (!nodes_to_unmark)
-	nodes_to_unmark = g_ptr_array_new ();
-
     for (list = g_list_last (trace); list != NULL; list = list->prev)
     {
 	gpointer address = list->data;
-	ProfileDescendant *match = NULL;
 	ProfileDescendant *prev = NULL;
-
+	ProfileDescendant *match = NULL;
+	
 	for (match = *tree; match != NULL; prev = match, match = match->siblings)
 	{
 	    if (match->name == address)
@@ -269,36 +264,11 @@ add_trace_to_tree (GList *trace, gint size, gpointer data)
 	
 	if (!match)
 	{
-	    ProfileDescendant *seen_tree_node;
-	    ProfileDescendant *n;
-	    
 	    /* Have we seen this object further up the tree? */
-	    seen_tree_node = NULL;
-	    for (n = parent; n != NULL; n = n->parent)
+	    for (match = parent; match != NULL; match = match->parent)
 	    {
-		if (n->name == address)
-		{
-		    seen_tree_node = n;
+		if (match->name == address)
 		    break;
-		}
-	    }
-	    
-	    if (seen_tree_node)
-	    {
-		ProfileDescendant *node;
-		
-		g_assert (parent);
-
-		for (node = parent; node != seen_tree_node->parent; node = node->parent)
-		{
-		    node->cumulative -= size;
-		    --node->marked_non_recursive;
-	
-		    g_assert (node->marked_non_recursive == 0 ||
-			      node->marked_non_recursive == 1);
-		}
-		
-		match = seen_tree_node;
 	    }
 	}
 	
@@ -310,41 +280,21 @@ add_trace_to_tree (GList *trace, gint size, gpointer data)
 	    match->cumulative = 0;
 	    match->self = 0;
 	    match->children = NULL;
-	    match->marked_non_recursive = 0;
 	    match->parent = parent;
 	    match->siblings = *tree;
 	    *tree = match;
 	}
 	
-	g_assert (match->marked_non_recursive == 0 ||
-		  match->marked_non_recursive == 1);
-	
-	if (!match->marked_non_recursive)
-	{
-	    g_ptr_array_add (nodes_to_unmark, match);
-	    match->cumulative += size;
-	    ++match->marked_non_recursive;
-	}
-	
-	if (!list->prev)
-	    match->self += size;
-	
 	tree = &(match->children);
 	parent = match;
     }
-    
-    len = nodes_to_unmark->len;
-    for (i = 0; i < len; ++i)
-    {
-	ProfileDescendant *tree_node = nodes_to_unmark->pdata[i];
 
-	g_assert (tree_node->marked_non_recursive == 0 ||
-		  tree_node->marked_non_recursive == 1);
-	
-	tree_node->marked_non_recursive = 0;
+    parent->self += size;
+    while (parent)
+    {
+	parent->cumulative += size;
+	parent = parent->parent;
     }
-    
-    g_ptr_array_set_size (nodes_to_unmark, 0);
 }
 
 ProfileDescendant *

profile.h

@@ -47,8 +47,6 @@ struct ProfileDescendant
     ProfileDescendant * parent;
     ProfileDescendant * siblings;
     ProfileDescendant * children;
-
-    int			marked_non_recursive;
 };
 
 struct ProfileCaller