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

[Jason Molenda <jason-swarelist@molenda.com>]

On Mon, Sep 10, 2001 at 11:48:22AM -0400, Andrew Cagney wrote:

> some brief, one handed, notes :-)

Oh jeez, don't tell me you broke your arm again?!  :-)

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

I'd disagree.  First, these functions are not very portable.  Our
very own RH 6.2 box that is sourceware.cygnus.com does not have
moncontrol() or monstart(), and -pg doesn't seem to be usable at
all.  My RH 7.1 box at home works fine.  MacOS X's FreeBSD works
fine.  Second, you definitely don't want to compile gdb with -pg
by default - the compiler inserts a bunch of bookkeeping code and
functions, and that'll be a real performance penalty.

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

Personally, I think it could just be "maint profile [on|off]", but that
could be ambiguous - are you profiling gdb or its inferior?

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

The "help maint" test in maint.exp globs it up.  You don't want to include
a check for maint profile-gdb in here because you'd get a FAIL when gdb
was not configured --enable-profiling.

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

All credit goes to Tom, of course.

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

I don't see what you're thinking of.

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

I just re-read the man page, it doesn't matter where it's disabled.  The
gmon.out file is going to be overwritten each time gdb is run no matter
what.

If you're profiling something that takes real CPU, the start-up functions
to get to control_main are background noise.  

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

The forward decl in maint.c?  Tom was following the convention of the
other functions in that file..

Jason