From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cagney <ac131313@cygnus.com>
To: Jason Molenda <jason-swarelist@molenda.com>
Cc: gdb-patches@sources.redhat.com, Tom Tromey <tromey@cygnus.com>
Subject: Re: [RFA] patch to add 'maint profile-gdb' command
Date: Mon, 10 Sep 2001 08:48:00 -0000
Message-id: <3B9CE0C6.5060700@cygnus.com>
References: <20010910003022.A21681@shell17.ba.best.com>
X-SW-Source: 2001-09/msg00132.html

> This is a refresh of Tom Tromey's gdb profiling patch, originally here:
> 	http://sources.redhat.com/ml/gdb-patches/2000-q1/msg00022.html
> 
> Instead of profiling all of gdb, Tom's patch lets you profile a
> specific command (or commands).  You enable profiling with the
> 'maint profile-gdb on' before the command(s) of interest, and
> 'maint profile-gdb off' (or exit) when you're finished.
> 
> configure and config.in both need to be regenerated after applying
> this patch.  Your build must be compiled with --enable-profiling
> for this feature to be enabled.

some brief, one handed, notes :-)

i think the command should always be present.  looking at the patch, it 
appears to have started out that way.

a check of *BSD's man pages also points to monstartup() and profil().  i 
dont think you should be trying to implement those.  however, i do think 
think it is important to ensure that the cli interface doesnt preclude 
adding them to the command set.  hence, i wonder if ``-gdb'' is needed 
in the command name?  Perhaps ``(gdb) maint profile control [on|off]''? 
  This is really a cli maintainer question and outside my domain.

are you sure no changes to maint.exp are needed? not needing them feels 
wrong.

suggest adding a ``post 5.1'' line to news - otherwise i'll get 
confused.  thanks for thinking of the news file.

(Eli?) should the doco include a reference to moncontrol(3)?

suggest a comment against the moncontrol() call in main.c explaining the 
problems.  would having the call in main() be better - turn it off at 
the earliest possible moment?

gdb is pure iso c, the params forward decl isn't needed.  you may want 
to look at http://sources.redhat.com/gdb/ari/ for a check list of things 
recently zapped from gdb.

and thanks for picking this up, get the above sorted out and the patch 
proper becomes obvious.

enjoy,
	andrew

> My only comments on this patch are (1) the documentation entry
> could note that your gmon.out file will be overwritten each time
> gdb is started, even if you don't do a profile-gdb on command[1], and
> (2) the configure.in check for $enable_profiling could be embedded
> in the AC_ARG_ENABLE() autoconf call.  It doesn't make any practical
> difference, but it looks like tradition in gdb's configure.in is
> to include this code inside the AC_ARG_ENABLE call.
> 
>  [1]  A bit of profiling happens before it can be turned
>       off in captured_main().  This initial profiling will overwrite
>       away any existing gmon.out.  At least it does with the gprof
>       on Linux and FreeBSD systems.
> 
> No testsuite regressions are added with this patch.
> 
> This patch does not require approval for the 5.1 branch - it is
> not something end users have cause to enable.
> 
> This patch does add a couple of ifdefs in main.c, aint.c to guard
> the code, but this is necessary.  Obviously you can't compile in
> profiling all the time (performance, portability), and you can't
> make calls to the profiling system calls if you aren't compiling
> -pg.
> 
> Jason
> 
>