Sun Nov 6 Soeren Sandmann <sandmann@redhat.com>

Sun Nov  6  Soeren Sandmann  <sandmann@redhat.com>

        * TODO: updates

        * collector.c (on_read): Only call back when a new sample actually
        arrived.

        * collector.c (collector_stop): close the filedescriptor

        * sysprof.c (on_start_toggled): Change sense of test.

        * sysprof.c (ensure_profile): Stop the collector.

        * sysprof.c (on_reset_clicked): Stop the collector when state
        becomes INITIAL.

ChangeLog

@@ -1,3 +1,19 @@
+Sun Nov  6  Soeren Sandmann  <sandmann@redhat.com>
+
+	* TODO: updates
+
+	* collector.c (on_read): Only call back when a new sample actually
+	arrived.
+
+	* collector.c (collector_stop): close the filedescriptor
+
+	* sysprof.c (on_start_toggled): Change sense of test.
+
+	* sysprof.c (ensure_profile): Stop the collector.
+
+	* sysprof.c (on_reset_clicked): Stop the collector when state
+	becomes INITIAL.
+
 Sun Nov  6 18:31:23 2005  Soeren Sandmann  <sandmann@redhat.com>
 
 	* stackstash.c (stack_stash_foreach): 

TODO

@@ -13,8 +13,8 @@ Before 1.0.1:
 
      Someone already did create a package - should be googlable.
 
-* Press start without kernel module loaded, then load kernel module and 
-  press start again. Segmentation fault.
+* See if we can reproduce this: Press start without kernel module loaded,
+  then load kernel module and press start again. Segmentation fault. 
 
 Before 1.2:
 
@@ -24,8 +24,6 @@ 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
   programmatically.
 
@@ -120,6 +118,8 @@ Before 1.2:
 	- 
 	
 * See if the auto-expanding can be made more intelligent
+	- "Everything" should be expanded exactly one level
+	- all nodes should be expanded at least one level
 
 * Send entire stack to user space, then do stackwalking there. That would
   allow us to do more complex algorithms, like dwarf, in userspace. Though
@@ -210,7 +210,6 @@ http://www.linuxbase.org/spec/booksets/LSB-Embedded/LSB-Embedded/ehframe.html
 
 - support more than one reader of the samples properly
 	- Don't generate them if noone cares
-	- When not profiling, sysprof shouldn't care
 
 - Add ability to show more than one function at a time. Algorithm:
 	Find all relevant nodes;
@@ -237,13 +236,7 @@ http://www.linuxbase.org/spec/booksets/LSB-Embedded/LSB-Embedded/ehframe.html
 
 - Have kernel module report the file the address was found in
 	Should avoid a lot of potential broken/raciness with dlopen etc.
-- Make things faster
-	- Can I get it to profile itself?
-	- speedprof seems to report that lots of time is spent in 
-	  stack_stash_foreach() and also in generate_key()
-- add an 'everything' object. It is really needed for a lot of things
-	- should be easy to do with stackstash reorganization.
-
+	Probably better to send a list of maps with each trace.
 - Figure out how Google's pprof script works. Then add real call graph 
   drawing. (google's script is really simple; uses dot from graphviz).
 
@@ -433,6 +426,18 @@ Later:
 
 DONE:
 
+- When not profiling, sysprof shouldn't keep the file open.
+
+- Make things faster
+	- Can I get it to profile itself?
+	- speedprof seems to report that lots of time is spent in 
+	  stack_stash_foreach() and also in generate_key()
+- add an 'everything' object. It is really needed for a lot of things
+	- should be easy to do with stackstash reorganization.
+
+
+* Handle time being set back in the RESET_DEAD_PERIOD code.
+
 - total should probably be cached so that compute_total() doesn't
   take 80% of the time to generate a profile.
 

collector.c

@@ -138,10 +138,10 @@ on_read (gpointer data)
 	add_trace_to_stash (&trace, collector->stash);
 	
 	collector->n_samples++;
-    }
     
-    if (collector->callback)
-	collector->callback (collector->data);
+	if (collector->callback)
+	    collector->callback (collector->data);
+    }
 }
 
 static gboolean
@@ -205,7 +205,13 @@ collector_start (Collector *collector,
 void
 collector_stop (Collector *collector)
 {
-    fd_set_read_callback (collector->fd, NULL);
+    if (collector->fd >= 0)
+    {
+	fd_remove_watch (collector->fd);
+	
+	close (collector->fd);
+	collector->fd = -1;
+    }
 }
 
 void

sysprof.c

@@ -342,9 +342,15 @@ on_start_toggled (GtkWidget *widget, gpointer data)
 	return;
     }
 
-    /* FIXME: get the real error message */
-    if (!collector_start (app->collector, NULL))
+    if (collector_start (app->collector, NULL))
     {
+	delete_data (app);
+
+	app->state = PROFILING;
+    }
+    else
+    {
+	/* FIXME: get the real error message */
 	sorry (app->main_window,
 	       "Can't open /proc/sysprof-trace. You need to insert\n"
 	       "the sysprof kernel module. Run\n"
@@ -353,12 +359,6 @@ on_start_toggled (GtkWidget *widget, gpointer data)
 	       "\n"
 	       "as root.");
     }
-    else
-    {
-	delete_data (app);
-
-	app->state = PROFILING;
-    }
 
     update_sensitivity (app);
 }
@@ -639,6 +639,8 @@ ensure_profile (Application *app)
     
     app->profile = collector_create_profile (app->collector);
 
+    collector_stop (app->collector);
+    
     fill_lists (app);
     
     app->state = DISPLAYING;
@@ -689,7 +691,10 @@ on_reset_clicked (gpointer widget, gpointer data)
     delete_data (app);
     
     if (app->state == DISPLAYING)
+    {
 	app->state = INITIAL;
+	collector_stop (app->collector);
+    }
     
     update_sensitivity (app);
 
@@ -818,6 +823,8 @@ set_loaded_profile (Application *app,
     
     update_sensitivity (app);
     
+    collector_stop (app->collector);
+    
     set_busy (app->main_window, FALSE);
 }