Re: [RFA] patch to add 'maint profile-gdb' command

Andrew Cagney
2001-09-10T08:48:00Z

> 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
>
>