Re: [RFC] Re: Profiling gdb?

[Daniel Jacobowitz <drow at mvista dot com>]

On Mon, Jan 06, 2003 at 09:14:41AM -0500, Andrew Cagney wrote:
> >
> >Perhaps some other time.  I do _use_ monstartup, however.
> >
> >This patch should address all comments from the last three times this
> >came up.  I chose to use monstartup/_mcleanup instead of using
> >moncontrol; when someone wants to use this on a system without those
> >functions, _then_ we can decide how to handle it.  That's just my
> >opinion though.  What I've done should work for at least GNU/Linux and
> >FreeBSD, and that's enough to be useful.  It only comes in if you
> >configure it on, anyway.
> >
> >It works like a charm; absolutely beautifully.  Thoughts, all?
> 
> (Unfortunatly, I've also leant a few things about how profiling works 
> ... :-().

Learning is good :)  Let's take advantage of it!

> I think the commands should always be present - conditional on the 
> corresponding function being present.  Enabling / disabling the profiler 
> is orthogonal to compiling with -pg.  I'm not sure about the unpublished 

OK, I see how this works.  I'll make the configuration flag just add
-pg, and update the manual.

> _moncleanup() call.  Are you sure of monstartup()'s function signature 
> (ari doesn't like the extern declarations but there isn't a header file 
> :-().

_mcleanup() is no more or less published than monstartup(), as far as I
can tell; I'll submit a glibc patch to document these when I get around
to it.  I know both FreeBSD and GNU/Linux have these, because that's
how Emacs controls profiling.

Hmm, make that less documented, since FreeBSD has a man page for
monstartup.  They're both provided, however.

I am indeed sure of monstartup()'s function signature - well, at least
for GNU/Linux; I don't have access to a FreeBSD system but according to
the source for libc in FreeBSD 5.0 it'll work there too.  Although I
think it may leak memory in multiple runs, the way I use it.  It's
declared in a non-system-specific file as taking longs (unsigned, oops,
will fix).

> I also suspect that more explict commands such as:
> 
> 	moncontrol on/off
> 	monstartup [ <start> <end> ]
> 
> would be better.  The person using this feature will need to know how it 
> is implemented anyway.

No, they won't, IMO.  More explicit commands might be better, but I'm
going to leave that be for now, OK?  I don't see the benefit of being
able to do this - it would let you get combined profiling data for
multiple events into the same gmon.out, big deal, just profile the
stuff in the middle too.

> (Did Jason Molenda, have something to do with the original patch?)

No, but he had something to do with the revised version, I'll add him
to the ChangeLog.

Here's an updated patch, addressing your concerns and Eli's.  Like it
better?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-01-06  Daniel Jacobowitz  <drow@mvista.com>

	Original patch by Tom Tromey <tromey@cygnus.com> and
	Jason Molenda <jmolenda@apple.com>.
	* Makefile.in (PROFILE_CFLAGS): Substitute from configure.
	(INTERNAL_LDFLAGS): Don't include PROFILE_CFLAGS.
	* NEWS: Mention profiling.
	* configure.in (--enable-gdbtk): Fix typo.
	(--enable-profiling): New.  Set PROFILE_CFLAGS.
	* maint.c (maintenance_set_profile_cmd): Remove NOTYET.
	Fill in function.
	(profiling_state): New variable.
	(mcleanup_wrapper): New function.
	(_initialize_maint): Remove NOTYET, fix call to
	add_setshow_boolean_cmd for "maint set profile".
	* configure: Regenerated.

2003-01-06  Daniel Jacobowitz  <drow@mvista.com>

	* gdb.texinfo (Maintenance Commands): Add "maint set profile"
	and "maint show profile".

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.309
diff -u -p -r1.309 Makefile.in
--- Makefile.in	5 Jan 2003 01:39:54 -0000	1.309
+++ Makefile.in	6 Jan 2003 15:04:01 -0000
@@ -318,7 +318,8 @@ GDB_CFLAGS = -I. -I$(srcdir) -I$(srcdir)
 # M{H,T}_CFLAGS, if defined, have host- and target-dependent CFLAGS
 # from the config directory.
 GLOBAL_CFLAGS = $(MT_CFLAGS) $(MH_CFLAGS)
-#PROFILE_CFLAGS = -pg
+
+PROFILE_CFLAGS = @PROFILE_CFLAGS@
 
 # CFLAGS is specifically reserved for setting from the command line
 # when running make.  I.E.  "make CFLAGS=-Wmissing-prototypes".
@@ -345,7 +346,8 @@ LDFLAGS = @LDFLAGS@
 # Profiling options need to go here to work.
 # I think it's perfectly reasonable for a user to set -pg in CFLAGS
 # and have it work; that's why CFLAGS is here.
-INTERNAL_LDFLAGS = $(CFLAGS) $(GLOBAL_CFLAGS) $(PROFILE_CFLAGS) $(MH_LDFLAGS) $(LDFLAGS) $(CONFIG_LDFLAGS)
+# PROFILE_CFLAGS is _not_ included, however, because we use monstartup.
+INTERNAL_LDFLAGS = $(CFLAGS) $(GLOBAL_CFLAGS) $(MH_LDFLAGS) $(LDFLAGS) $(CONFIG_LDFLAGS)
 
 # If your system is missing alloca(), or, more likely, it's there but
 # it doesn't work, then refer to libiberty.
Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.97
diff -u -p -r1.97 NEWS
--- NEWS	2 Jan 2003 14:27:26 -0000	1.97
+++ NEWS	6 Jan 2003 15:04:02 -0000
@@ -3,6 +3,14 @@
 
 *** Changes since GDB 5.3:
 
+* Profiling support
+
+A new command, "maint set profile on/off", has been added.  This command can
+be used to enable or disable profiling while running GDB, to profile a
+session or a set of commands.  In addition there is a new configure switch,
+"--enable-profiling", which will cause GDB to be compiled with profiling
+data, for more informative profiling results.
+
 * Default MI syntax changed to "mi2".
 
 The default MI (machine interface) syntax, enabled by the command line
Index: configure.in
===================================================================
RCS file: /cvs/src/src/gdb/configure.in,v
retrieving revision 1.116
diff -u -p -r1.116 configure.in
--- configure.in	4 Jan 2003 23:47:12 -0000	1.116
+++ configure.in	6 Jan 2003 15:04:04 -0000
@@ -164,7 +164,7 @@ fi
 
 # Enable gdbtk.
 AC_ARG_ENABLE(gdbtk,
-[  --enable-gtk            enable gdbtk graphical user interface (GUI)],
+[  --enable-gdbtk          enable gdbtk graphical user interface (GUI)],
   [case $enableval in
     yes | no)
       ;;
@@ -183,6 +183,31 @@ case $host_os in
     enable_gdbtk=no ;;
 esac
 
+# Profiling support.
+AC_ARG_ENABLE(profiling,
+[  --enable-profiling      enable profiling of GDB],
+  [case $enableval in
+    yes | no)
+      ;;
+    *)
+      AC_MSG_ERROR([bad value $enableval for --enable-profile]) ;;
+  esac],
+ [enable_profiling=no])
+
+if test "$enable_profiling" = yes ; then
+  PROFILE_CFLAGS=-pg
+  OLD_CFLAGS="$CFLAGS"
+  CFLAGS="$CFLAGS $PROFILE_CFLAGS"
+
+  AC_CHECK_FUNC(monstartup, [],
+    AC_MSG_ERROR(monstartup necessary to use --enable-profiling.))
+
+  AC_CHECK_FUNC(_mcleanup, [],
+    AC_MSG_ERROR(_mcleanup necessary to use --enable-profiling.))
+
+  CFLAGS="$OLD_CFLAGS"
+fi
+
 # --------------------- #
 # Checks for programs.  #
 # --------------------- #
@@ -1132,6 +1157,7 @@ AC_SUBST(IGNORE_SIM)
 AC_SUBST(IGNORE_SIM_OBS)
 
 AC_SUBST(ENABLE_CFLAGS)
+AC_SUBST(PROFILE_CFLAGS)
 
 AC_SUBST(CONFIG_OBS)
 AC_SUBST(CONFIG_LIB_OBS)
Index: maint.c
===================================================================
RCS file: /cvs/src/src/gdb/maint.c,v
retrieving revision 1.30
diff -u -p -r1.30 maint.c
--- maint.c	24 Dec 2002 03:57:58 -0000	1.30
+++ maint.c	6 Jan 2003 15:04:04 -0000
@@ -639,18 +639,52 @@ maintenance_show_cmd (char *args, int fr
   cmd_show_list (maintenance_show_cmdlist, from_tty, "");
 }
 
-#ifdef NOTYET
 /* Profiling support.  */
 
 static int maintenance_profile_p;
+static int profiling_state;
+
+static void
+mcleanup_wrapper (void)
+{
+  extern void _mcleanup (void);
+
+  if (profiling_state)
+    _mcleanup ();
+}
 
 static void
 maintenance_set_profile_cmd (char *args, int from_tty, struct cmd_list_element *c)
 {
-  maintenance_profile_p = 0;
-  warning ("\"maintenance set profile\" command not supported.\n");
+  if (maintenance_profile_p == profiling_state)
+    return;
+
+  profiling_state = maintenance_profile_p;
+
+  if (maintenance_profile_p)
+    {
+      static int profiling_initialized;
+
+      extern void monstartup (unsigned long, unsigned long);
+      extern char _etext;
+      extern int main();
+
+      if (!profiling_initialized)
+	{
+	  atexit (mcleanup_wrapper);
+	  profiling_initialized = 1;
+	}
+
+      /* "main" is now always the first function in the text segment, so use
+	 its address for monstartup.  */
+      monstartup ((unsigned long) &main, (unsigned long) &_etext);
+    }
+  else
+    {
+      extern void _mcleanup (void);
+      _mcleanup ();
+    }
 }
-#endif
 
 void
 _initialize_maint_cmds (void)
@@ -807,16 +841,12 @@ passes without a response from the targe
 		      &showlist);
 
 
-#ifdef NOTYET
-  /* FIXME: cagney/2002-06-15: A patch implementing profiling is
-     pending, this just sets up the framework.  */
-  tmpcmd = add_setshow_boolean_cmd ("profile", class_maintenance,
-				    var_boolean, &maintenance_profile_p, "\
-Set internal profiling.\n\
-When enabled GDB is profiled.", "\
-Show internal profiling.\n",
-				    maintenance_set_profile_cmd, NULL,
-				    &maintenance_set_cmdlist,
-				    &maintenance_show_cmdlist);
-#endif
+  add_setshow_boolean_cmd ("profile", class_maintenance,
+			   &maintenance_profile_p,
+			   "Set internal profiling.\n"
+			   "When enabled GDB is profiled.",
+			   "Show internal profiling.\n",
+			   maintenance_set_profile_cmd, NULL,
+			   &maintenance_set_cmdlist,
+			   &maintenance_show_cmdlist);
 }
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.136
diff -u -p -r1.136 gdb.texinfo
--- doc/gdb.texinfo	5 Jan 2003 04:34:39 -0000	1.136
+++ doc/gdb.texinfo	6 Jan 2003 15:04:09 -0000
@@ -14385,6 +14385,24 @@ cooked-registers} includes the (cooked) 
 
 Takes an optional file parameter.
 
+@kindex maint set profile
+@kindex maint show profile
+@cindex profiling GDB
+@item maint set profile
+@itemx maint show profile
+Control profiling of @value{GDBN}.
+
+Profiling will be disabled until you use the @samp{maint set profile}
+command to enable it.  When you enable profiling, the system will begin
+collecting timing and execution count data; when you disable profiling or
+exit @value{GDBN}, the results will be written to a log file.  Remember that
+if you use profiling, @value{GDBN} will overwrite the profiling log file
+(often called @file{gmon.out}).  If you have a record of important profiling
+data in a @file{gmon.out} file, be sure to move it to a safe location.
+
+Configuring with @samp{--enable-profiling} arranges for @value{GDBN} to be
+compiled with the @samp{-pg} compiler option. 
+
 @end table