various small fixes

README

@@ -23,24 +23,26 @@ program "sysprof".
 	- "make World"
 
   (2) Install the X server making sure it can't see any ".a" files. If
-      You install on top of an existing installation, just do
+      you install on top of an existing installation, just do
 
 	  find /usr/X11R6/lib/"*.a" | sudo xargs rm
 
-      and install the newly compiled X server. If a ".so" X server
+      and install the newly compiled X server. 
-      finds .a files in its module path it will try to load those 
+
-      in preference to .so files and this causes symbol resolution
+      If a ".so" X server finds .a files in its module path it will
-      problems
+      try to load those in preference to .so files and this causes
+      symbol resolution problems
 
   (3) Run your new X server
 
   (4) Run sysprof as root. This is necessary because the X server binary 
       for security reasons is not readable by regular users. I could tell
-      you why this is so, but then I'd have to kill you.
+      you why, but then I'd have to kill you.
+
 
 Credits:
-	Kristian H<>gsberg for the first port to kernel 2.6
+      Kristian H<>gsberg for the first port to the 2.6 kernel.
-	Owen Taylor for the symbol lookup code in memprof
+      Owen Taylor for the symbol lookup code in memprof
 	
 
 S<EFBFBD>ren

TODO

@@ -1,3 +1,22 @@
+- consider caching [filename => bin_file]
+- Have kernel modulereport the file the symbol was found in
+	Should avoid a lot of potential brokenness with dlopen etc.
+- grep FIXME
+- hide internal stuff in ProfileDescendant
+
+- loading and saving
+
+- Non-GUI version that can save in a format the GUI could understand.
+  Could be used for profiling startup etc. Should be able to dump the
+  data to a network socket.
+
+DONE:
+
+- consider making ProfileObject more of an object.
+
+- make an "everything" object
+       maybe not necessary -- there is a libc_ctors_something()
+
 - make presentation strings nicer
 
   four different kinds of symbols:
@@ -10,19 +29,8 @@
   (a) is easy,  (b) should be <in ...>    (c) should just become "???" 
   (d) not sure
 
-- grep FIXME
-- make an "everything" object
-       maybe not necessary -- there is a libc_ctors_something()
-- consider making ProfileObject more of an object.
-- hide internal stuff in ProfileDescendant
-- consider caching [filename->bin_file]
-
-DONE:
-
 - processes with a cmdline of "" should get a [pid = %d] instead.
 
-- Kernel module should report the file the symbol was found in
-
 - make an "n samples" label
 Process stuff:
 

profile.c

@@ -1,10 +1,11 @@
 #include <glib.h>
+
 #include "binfile.h"
 #include "process.h"
 #include "stackstash.h"
 #include "profile.h"
 
-typedef struct RealProfile RealProfile;
+
 typedef struct Node Node;
 
 static void
@@ -30,7 +31,9 @@ struct Node
     Node *siblings;		/* siblings in the call tree */
     Node *children;		/* children in the call tree */
     Node *parent;		/* parent in call tree */
-    Node *next;			/* nodes that corresponding to same object */
+    Node *next;			/* nodes that correspond to same object are linked though
+				 * this pointer
+				 */
 
     guint total;
     guint self;
@@ -38,13 +41,13 @@ struct Node
     gboolean toplevel;
 };
 
-struct RealProfile
+struct Profile
 {
-    Profile profile;
+    gint		size;
-
+    GList *		objects;
-    Node *call_tree;
+    Node *		call_tree;
-    GHashTable *profile_objects;
+    GHashTable *	profile_objects;
-    GHashTable *nodes_by_object;
+    GHashTable *	nodes_by_object;
 };
 
 static ProfileObject *
@@ -57,6 +60,26 @@ profile_object_new (void)
     return obj;
 }
 
+gboolean
+profile_save (Profile		 *profile,
+	      const char	 *file_name,
+	      GError		**err)
+{
+    /* FIXME */
+    /* Actually the way to fix this is probably to save StackStashes instead
+     * of profiles
+     */
+    return FALSE;
+}
+
+Profile *
+profile_load (const char	 *filename,
+	      GError           **err)
+{
+    /* FIXME */
+    return NULL;
+}
+
 static void
 profile_object_free (ProfileObject *obj)
 {
@@ -65,13 +88,13 @@ profile_object_free (ProfileObject *obj)
 }
 
 static char *
-generate_key (RealProfile *profile, Process *process, gulong address)
+generate_key (Profile *profile, Process *process, gulong address)
 {
     if (address)
     {
 	const Symbol *symbol = process_lookup_symbol (process, address);
 
-	return g_strdup_printf ("%p%s", symbol->address, symbol->name);
+	return g_strdup_printf ("%p%s", (void *)symbol->address, symbol->name);
     }
     else
     {
@@ -80,7 +103,7 @@ generate_key (RealProfile *profile, Process *process, gulong address)
 }
 
 static char *
-generate_presentation_name (RealProfile *profile, Process *process, gulong address)
+generate_presentation_name (Profile *profile, Process *process, gulong address)
 {
     /* FIXME using 0 to indicate "process" is broken */
     if (address)
@@ -100,7 +123,7 @@ generate_presentation_name (RealProfile *profile, Process *process, gulong addre
 }
 
 static void
-ensure_profile_node (RealProfile *profile, Process *process, gulong address)
+ensure_profile_node (Profile *profile, Process *process, gulong address)
 {
     char *key = generate_key (profile, process, address);
 
@@ -120,7 +143,7 @@ ensure_profile_node (RealProfile *profile, Process *process, gulong address)
 }
 
 static ProfileObject *
-lookup_profile_object (RealProfile *profile, Process *process, gulong address)
+lookup_profile_object (Profile *profile, Process *process, gulong address)
 {
     ProfileObject *object;
     char *key = generate_key (profile, process, address);
@@ -133,7 +156,7 @@ lookup_profile_object (RealProfile *profile, Process *process, gulong address)
 static void
 generate_object_table (Process *process, GSList *trace, gint size, gpointer data)
 {
-    RealProfile *profile = data;
+    Profile *profile = data;
     GSList *list;
 
     ensure_profile_node (profile, process, 0);
@@ -144,7 +167,7 @@ generate_object_table (Process *process, GSList *trace, gint size, gpointer data
 	ensure_profile_node (profile, process, (gulong)list->data);
     }
 
-    profile->profile.profile_size += size;
+    profile->size += size;
 }
 
 static Node *
@@ -163,7 +186,7 @@ node_new ()
 }
 
 static Node *
-node_add_trace (RealProfile *profile, Node *node, Process *process,
+node_add_trace (Profile *profile, Node *node, Process *process,
 		GSList *trace, gint size,
 		GHashTable *seen_objects)
 {
@@ -228,7 +251,7 @@ dump_trace (GSList *trace)
 static void
 generate_call_tree (Process *process, GSList *trace, gint size, gpointer data)
 {
-    RealProfile *profile = data;
+    Profile *profile = data;
     Node *match = NULL;
     ProfileObject *proc = lookup_profile_object (profile, process, 0);
     GHashTable *seen_objects;
@@ -299,27 +322,27 @@ link_parents (Node *node, Node *parent)
 Profile *
 profile_new (StackStash *stash)
 {
-    RealProfile *real_profile = g_new (RealProfile, 1);
+    Profile *profile = g_new (Profile, 1);
 
-    real_profile->profile_objects =
+    profile->profile_objects =
 	g_hash_table_new_full (g_str_hash, g_str_equal,
 			       g_free, (GDestroyNotify)profile_object_free);
 
-    real_profile->call_tree = NULL;
+    profile->call_tree = NULL;
 
-    real_profile->nodes_by_object =
+    profile->nodes_by_object =
 	g_hash_table_new (direct_hash_no_null, g_direct_equal);
-    real_profile->profile.objects = NULL;
+    profile->objects = NULL;
 
-    real_profile->profile.profile_size = 0;
+    profile->size = 0;
 
-    stack_stash_foreach (stash, generate_object_table, real_profile);
+    stack_stash_foreach (stash, generate_object_table, profile);
-    stack_stash_foreach (stash, generate_call_tree, real_profile);
+    stack_stash_foreach (stash, generate_call_tree, profile);
-    link_parents (real_profile->call_tree, NULL);
+    link_parents (profile->call_tree, NULL);
     
-    g_hash_table_foreach (real_profile->nodes_by_object, build_object_list, real_profile);
+    g_hash_table_foreach (profile->nodes_by_object, build_object_list, profile);
 
-    return (Profile *)real_profile;
+    return profile;
 }
 
 static void
@@ -463,10 +486,9 @@ add_leaf_to_tree (ProfileDescendant **tree, Node *leaf, Node *top)
 }
 
 ProfileDescendant *
-profile_create_descendants (Profile *prf, ProfileObject *object)
+profile_create_descendants (Profile *profile, ProfileObject *object)
 {
     ProfileDescendant *tree = NULL;
-    RealProfile *profile = (RealProfile *)prf;
     Node *node;
 
     node = g_hash_table_lookup (profile->nodes_by_object, object);
@@ -503,10 +525,9 @@ profile_caller_new (void)
 }
 
 ProfileCaller *
-profile_list_callers (Profile       *prf,
+profile_list_callers (Profile       *profile,
 		      ProfileObject *callee)
 {
-    RealProfile *profile = (RealProfile *)prf;
     Node *callee_node;
     Node *node;
     GHashTable *callers_by_object;
@@ -596,15 +617,17 @@ node_free (Node *node)
 }
 
 void
-profile_free (Profile *prf)
+profile_free (Profile *profile)
 {
-    RealProfile *profile = (RealProfile *)prf;
+    g_list_free (profile->objects);
 
-    g_list_free (prf->objects);
     g_hash_table_destroy (profile->profile_objects);
+
     node_free (profile->call_tree);
+
     g_hash_table_destroy (profile->nodes_by_object);
-    g_free (prf);
+
+    g_free (profile);
 }
 
 void
@@ -628,3 +651,15 @@ profile_caller_free (ProfileCaller *caller)
     profile_caller_free (caller->next);
     g_free (caller);
 }
+
+GList *
+profile_get_objects (Profile *profile)
+{
+    return profile->objects;
+}
+
+gint
+profile_get_size (Profile *profile)
+{
+    return profile->size;
+}

profile.h

@@ -49,12 +49,6 @@ struct ProfileDescendant
     int			marked_total;
 };
 
-struct Profile
-{
-    gint		profile_size;
-    GList *		objects;
-};
-
 struct ProfileCaller
 {
     ProfileObject *	object;		/* can be NULL */
@@ -66,9 +60,16 @@ struct ProfileCaller
 
 Profile *          profile_new                (StackStash        *stash);
 void               profile_free               (Profile           *profile);
+gint		   profile_get_size	      (Profile           *profile);
+GList *		   profile_get_objects	      (Profile		 *profile);
 ProfileDescendant *profile_create_descendants (Profile           *prf,
 					       ProfileObject     *object);
 ProfileCaller *	   profile_list_callers       (Profile		 *profile,
 					       ProfileObject     *callee);
 void		   profile_caller_free	      (ProfileCaller     *caller);
 void               profile_descendant_free    (ProfileDescendant *descendant);
+gboolean	   profile_save		      (Profile		 *profile,
+					       const char	 *file_name,
+					       GError		**err);
+Profile *	   profile_load		      (const char	 *filename,
+					       GError           **err);

sysprof-module.c

@@ -219,10 +219,10 @@ static void
 generate_stack_trace(struct task_struct *task,
 		     SysprofStackTrace *trace)
 {
-#ifdef NOT_ON_4G4G	/* FIXME: What is the symbol really called? */
+#ifdef CONFIG_HIGHMEM
-#  define START_OF_STACK 0xBFFFFFFF
-#else
 #  define START_OF_STACK 0xFF000000
+#else
+#  define START_OF_STACK 0xBFFFFFFF
 #endif
 	struct pt_regs *regs = ((struct pt_regs *) (THREAD_SIZE + (unsigned long) task->thread_info)) - 1;
 	StackFrame frame;

sysprof.c

@@ -5,6 +5,7 @@
 #include <unistd.h>
 #include <glade/glade.h>
 #include <errno.h>
+#include <glib/gprintf.h>
 
 #include "binfile.h"
 #include "watch.h"
@@ -13,6 +14,7 @@
 #include "profile.h"
 #include "treeviewutils.h"
 
+
 /* FIXME */
 #define _(a) a
 
@@ -278,17 +280,18 @@ fill_main_list (Application *app)
 					 G_TYPE_DOUBLE,
 					 G_TYPE_POINTER);
 	
-	for (list = profile->objects; list != NULL; list = list->next)
+	for (list = profile_get_objects (profile); list != NULL; list = list->next)
 	{
 	    ProfileObject *object = list->data;
 	    GtkTreeIter iter;
+	    double profile_size = profile_get_size (profile);
 	    
 	    gtk_list_store_append (list_store, &iter);
 	    
 	    gtk_list_store_set (list_store, &iter,
 				OBJECT_NAME, object->name,
-				OBJECT_SELF, 100.0 * object->self/(double)profile->profile_size,
+				OBJECT_SELF, 100.0 * object->self / profile_size,
-				OBJECT_TOTAL, 100.0 * object->total/(double)profile->profile_size,
+				OBJECT_TOTAL, 100.0 * object->total / profile_size,
 				OBJECT_OBJECT, object,
 				-1);
 	}
@@ -343,10 +346,35 @@ on_profile_toggled (gpointer widget, gpointer data)
     update_sensitivity (app);
 }
 
+static void
+sorry (GtkWidget *parent_window,
+       const gchar *format,
+       ...)
+{
+	va_list args;
+	char *message;
+	GtkWidget *dialog;
+
+	va_start (args, format);
+	g_vasprintf (&message, format, args);
+	va_end (args);
+
+	dialog = gtk_message_dialog_new (parent_window ? GTK_WINDOW (parent_window) : NULL,
+					 GTK_DIALOG_DESTROY_WITH_PARENT,
+					 GTK_MESSAGE_WARNING,
+					 GTK_BUTTONS_OK, message);
+	free (message);
+
+	gtk_window_set_title (GTK_WINDOW (dialog), "System Profiler Warning");
+
+	gtk_dialog_run (GTK_DIALOG (dialog));
+	gtk_widget_destroy (dialog);
+}
+
 static void
 on_open_clicked (gpointer widget, gpointer data)
 {
-    
+    sorry (NULL, "Open is not implemented yet. (Fortunately, neither is saving),");
 }
 
 static void
@@ -367,7 +395,8 @@ on_save_as_clicked (gpointer widget, gpointer data)
 {
     Application *app = data;
 
-
+    sorry (NULL, "Saving profiles is not yet implemented.");
+    
     if (app)
 	;
     /* FIXME */
@@ -453,7 +482,7 @@ fill_descendants_tree (Application *app)
 	    app->descendants =
 		profile_create_descendants (app->profile, object);
 	    add_node (tree_store,
-		      app->profile->profile_size, NULL, app->descendants);
+		      profile_get_size (app->profile), NULL, app->descendants);
 	}
     }
 
@@ -483,6 +512,7 @@ add_callers (GtkListStore *list_store,
     {
 	gchar *name;
 	GtkTreeIter iter;
+	double profile_size = profile_get_size (profile);
 	
 	if (callers->object)
 	    name = callers->object->name;
@@ -493,8 +523,8 @@ add_callers (GtkListStore *list_store,
 	gtk_list_store_set (
 	    list_store, &iter,
 	    CALLERS_NAME, name,
-	    CALLERS_SELF, 100.0 * callers->self/(double)profile->profile_size,
+	    CALLERS_SELF, 100.0 * callers->self / profile_size,
-	    CALLERS_TOTAL, 100.0 * callers->total/(double)profile->profile_size,
+	    CALLERS_TOTAL, 100.0 * callers->total / profile_size,
 	    CALLERS_OBJECT, callers->object,
 	    -1);