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

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

[Eli Zaretskii]

> Date: Sat, 22 Sep 2001 19:08:50 -0400
> From: Andrew Cagney <ac131313@cygnus.com>
> 
> 
> Turns out there is already a ``maint info'' so a ``maint set/show'' 
> makes good sense.  Try the attached.
> 
> Jason, I think it is pretty clear where to add the profiling bit :-)
> Eli, if the doco entry is going in gdb.texinfo, can you suggest where?

Hmm.. I don't see any doco entry in the patch you posted.  Did you
mean Jason's patches?  In that case, the chapter "Controlling GDB"
seems like a good place.  I'd say either create a new section
"Profiling GDB" or add the text to the section "Debugging Output".

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

[Fernando Nasser]

Andrew Cagney wrote:
> 
> > On Wed, 19 Sep 2001, Kevin Buettner wrote:
> >
> >
> >> I like having the maintenance commands all lumped under "maint".  That
> >> way I can use tab completion to see all of the maintenance related commands
> >> when I do
> >>
> >> (gdb) maint <TAB>
> >>
> >> I think a set/show would be useful for some activities; couldn't
> >> we add "maint set ..." and "maint show ..."  ?
> >
> >
> > That seems to be a good idea, IMHO.
> 
> Turns out there is already a ``maint info'' so a ``maint set/show''
> makes good sense.  Try the attached.
> 

Nice.

> Jason, I think it is pretty clear where to add the profiling bit :-)
> Eli, if the doco entry is going in gdb.texinfo, can you suggest where?
> Fernando, is this part ok?
> 

Yes, sure.

P.S.: There are some maint commands that should be maint set/show in 
the arm rdi thing.  Can you add fixing those to the TODO list, please?  


-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

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

[Andrew Cagney]

> P.S.: There are some maint commands that should be maint set/show in 
> the arm rdi thing.  Can you add fixing those to the TODO list, please?  

Just file change-request PRs: http://sources.redhat.com/gdb/bugs
Andrew

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

[Andrew Cagney]

>> Fernando, is this part ok?
>> 
> 
> 
> Yes, sure.

Thanks.  I've checked it in but with a tweek:

+ #ifdef NOTYET
+   /* FIXME: cagney/2001-09-24: A patch introducing a
+      add_set_boolean_cmd() is pending, the below should probably use
+      it.  A patch implementing profiling is pending, this just sets up
+      the framework.  */
+   tmpcmd = add_set_cmd ("profile", class_maintenance,
+                       var_boolean, &maintenance_profile_p,
+                       "Set internal profiling.\n\
+ When enabled GDB is profiled.",
+                       &maintenance_set_cmdlist);
+   tmpcmd->function.sfunc = maintenance_set_profile_cmd;
+   add_show_from_set (tmpcmd, &maintenance_show_cmdlist);
+ #endif

I wrapped the ``maint set profile'' stuff in #ifdef NOTYET for the moment.

Andrew

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

[Jason Molenda]

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.

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
From jason-swarelist@molenda.com Mon Sep 10 00:36:00 2001
From: Jason Molenda <jason-swarelist@molenda.com>
To: gdb-patches@sources.redhat.com
Subject: Typeo in NEWS
Date: Mon, 10 Sep 2001 00:36:00 -0000
Message-id: <20010910003611.A22499@shell17.ba.best.com>
X-SW-Source: 2001-09/msg00126.html
Content-length: 249

The NEWS file claims that gdb's code has been converted to "ANS/ISO C".
It should either be "ANSI/ISO C", or more correctly, just "ISO C".  

I won't argue strenuously for "ISO" over "ANSI/ISO", but the patch
I'm including implements the latter.

J
From eliz@is.elta.co.il Mon Sep 10 00:50:00 2001
From: Eli Zaretskii <eliz@is.elta.co.il>
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 00:50:00 -0000
Message-id: <Pine.SUN.3.91.1010910094003.18194H-100000@is>
References: <20010910003022.A21681@shell17.ba.best.com>
X-SW-Source: 2001-09/msg00127.html
Content-length: 1698

On Mon, 10 Sep 2001, Jason Molenda wrote:

> This is a refresh of Tom Tromey's gdb profiling patch, originally here:
> 	http://sources.redhat.com/ml/gdb-patches/2000-q1/msg00022.html

Thanks!  I think it's a very useful feature, but I have a few minor 
comments.

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

Isn't it better to use monstartup instead of moncontrol and -pg?  It 
looks like using monstartup could solve several problems:

  - you don't overwrite gmon.out unless you actually profile
  - you can link without -pg
  - you can make this option available by default, since it is invisible 
    unless you actually profile (I dislike features that require a 
    rebuild to become available)

We will have to add Autoconf tests to see if monstartup is available, but 
I think using moncontrol if --enable-profiling is given is not safe 
enough anyway.

In addition, I think the commands this patch adds should be documented in 
gdb.texinfo as well, since all commands one can type at GDB's prompt 
should be described there.

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

[Andrew Cagney]

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

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

[Jason Molenda]

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

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

[Andrew Cagney]

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

the PARAMS() bit - ISO C is assumed (the forward decl must be added when 
needed, beyond that is now optional)

andrew

[RFA] Version 2 of patch to add 'maint profile-gdb' command

[Jason Molenda]

Here is a minor rewrite of the profiling patch.  I removed the
PARAMS, fixed the NEWS entry, and added a check for moncontrol()
with -pg - if this fails to compile (as it fails on sources.redhat.com),
an error message is spit out and the configure aborts.  If a warning
is desirable, it's a minor change from calling AC_MSG_ERROR to
AC_MSG_WARN, but I found that to be too subtle in my testing (the
warning was lost among the configure output and people would be
confused why their --enable-profiling gdb isn't actually profiling.)
I amended the gdbint.texinfo documentation to note that the gmon.out
file is overwritten each time gdb is started.

The one notable change I did not incorporate was compiling with
-pg, linking without -pg, and using monstartup() to enable the
profiling.  I have two problems with this - one, I _suspect_ (but
I do not know for certain) that linking without -pg and just calling
monstartup() is going to be unportable.  The second problem is
that, off-hand, I don't know how a program should determine the
start and end addresses of its TEXT section portably (which are
required to use monstartup).

I agree that using monstartup() would be convenient, as developers
would not constantly overwrite their existing gmon.out file whenever
they start gdb, but I'd like to see this patch committed and someone
who understands the mechanisms of profiling across different systems
could tackle it in the future.  In my opinion, monstartup() as a nice
refinement which a motivated person could tackle in the future.  I'd
rather not complicate the basic feature with this at this point.
This patch was written nearly two years ago - it'd be good to get
it integrated.

NB - autoconf and autoheader must both be run after applying this patch.

Thanks for your help,

Jason
From eliz@is.elta.co.il Sun Sep 16 22:36:00 2001
From: Eli Zaretskii <eliz@is.elta.co.il>
To: jason-swarelist@molenda.com
Cc: gdb-patches@sources.redhat.com, tromey@cygnus.com
Subject: Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command
Date: Sun, 16 Sep 2001 22:36:00 -0000
Message-id: <200109170536.HAA21988@is.elta.co.il>
References: <20010910003022.A21681@shell17.ba.best.com> <3B9CE0C6.5060700@cygnus.com> <20010910115244.A25119@shell17.ba.best.com> <3B9FACDA.6070407@cygnus.com> <20010916174433.A3258@shell17.ba.best.com>
X-SW-Source: 2001-09/msg00204.html
Content-length: 1311

> Date: Sun, 16 Sep 2001 17:44:33 -0700
> From: Jason Molenda <jason-swarelist@molenda.com>
> 
> The one notable change I did not incorporate was compiling with
> -pg, linking without -pg, and using monstartup() to enable the
> profiling.  I have two problems with this - one, I _suspect_ (but
> I do not know for certain) that linking without -pg and just calling
> monstartup() is going to be unportable.  The second problem is
> that, off-hand, I don't know how a program should determine the
> start and end addresses of its TEXT section portably (which are
> required to use monstartup).

Emacs uses monstartup and manages to solve both of these problems.
See the file src/emacs.c (search for "etext") for the details.

> diff -u -p -r1.39 gdbint.texinfo
> --- doc/gdbint.texinfo	8 Sep 2001 10:53:45 -0000	1.39
> +++ doc/gdbint.texinfo	17 Sep 2001 00:34:26 -0000

I'd like the documentation of the "maint profile-gdb" command to be in
gdb.texinfo, not gdbint.texinfo.  The latter doesn't document user
commands, and users might not readily know to look in gdbint.  (It's
okay to have it described in both manuals, if you think it will be
useful.)

> +
> +@section Profiling GDB

Please replace "GDB" with "@value{GDBN}" everywhere.

Other than the last two comments, the patches to the docs are
approved.

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

[Andrew Cagney]

> +  add_cmd ("profile-gdb", class_maintenance, maint_profile_gdb,
> +	   "Enable or disable profiling.",
> +	   &maintenancelist);

Jason, should have thought of this earlier, sorry.  Should this be 
``set/show maint profile-gdb''?  ``set/show remote ...'' provides 
cut/paste for implementing this.

As far as I can tell it isn't possible to examine the value of profile-gdb.

Andrew

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

[Fernando Nasser]

Andrew Cagney wrote:
> 
> > +  add_cmd ("profile-gdb", class_maintenance, maint_profile_gdb,
> > +        "Enable or disable profiling.",
> > +        &maintenancelist);
> 
> Jason, should have thought of this earlier, sorry.  Should this be
> ``set/show maint profile-gdb''?  ``set/show remote ...'' provides
> cut/paste for implementing this.
> 
> As far as I can tell it isn't possible to examine the value of profile-gdb.
> 

It seems to be a general problem with "maint" things that are settings
(instead of an action).

We should eventually create a "set/show maint" thing similar to
"set/show debug" (humm, what is the difference here?).
I would leave it for a separate patch which would do it for all
the maint related settings.


BTW, "maint profile-gdb" is redundant.  maint commands are supposed to be
introspective already.  The target is always gdb itself, never the target.
Someone mentioned that the "-gdb" should be dropped and I agree.  But I do
not mind if it stays as it is as "maint profile" will work anyways.


-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

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

[Andrew Cagney]

>> Jason, should have thought of this earlier, sorry.  Should this be
>> ``set/show maint profile-gdb''?  ``set/show remote ...'' provides
>> cut/paste for implementing this.
>> 
>> As far as I can tell it isn't possible to examine the value of profile-gdb.
>> 
> 
> 
> It seems to be a general problem with "maint" things that are settings
> (instead of an action).
> 
> We should eventually create a "set/show maint" thing similar to
> "set/show debug" (humm, what is the difference here?).
> I would leave it for a separate patch which would do it for all
> the maint related settings.


If you're ok with ``set/show maint ...'' I can clone ``set/show remote 
...'' into what is needed.

Andrew

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

[Fernando Nasser]

Andrew Cagney wrote:
> 
> >> Jason, should have thought of this earlier, sorry.  Should this be
> >> ``set/show maint profile-gdb''?  ``set/show remote ...'' provides
> >> cut/paste for implementing this.
> >>
> >> As far as I can tell it isn't possible to examine the value of profile-gdb.
> >>
> >
> >
> > It seems to be a general problem with "maint" things that are settings
> > (instead of an action).
> >
> > We should eventually create a "set/show maint" thing similar to
> > "set/show debug" (humm, what is the difference here?).
> > I would leave it for a separate patch which would do it for all
> > the maint related settings.
> 
> If you're ok with ``set/show maint ...'' I can clone ``set/show remote
> ...'' into what is needed.
> 

I think it is a good idea. But we should leave the things that are
currently set with the maint command marked as deprecated and
with the appropriate message telling to use "set maint" instead.

Also, I would like someone else to support us on this move: Tom? Jason? Eli?

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

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

[Eli Zaretskii]

> Date: Wed, 19 Sep 2001 10:07:55 -0400
> From: Fernando Nasser <fnasser@cygnus.com>
> 
> Also, I would like someone else to support us on this move: Tom? Jason? Eli?

Sorry, I probably wasn't following this thread closely enough: what
move did you have in mind?

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

[Fernando Nasser]

Eli Zaretskii wrote:
> 
> > Date: Wed, 19 Sep 2001 10:07:55 -0400
> > From: Fernando Nasser <fnasser@cygnus.com>
> >
> > Also, I would like someone else to support us on this move: Tom? Jason? Eli?
> 
> Sorry, I probably wasn't following this thread closely enough: what
> move did you have in mind?

We have some maint commands that actually set values.  Either turn things
on/off or set control variables (for instance, name of a log file).
As these are just plain commands, there is no way to examine the current
settings.

Andrew has proposed creating a class of set/show commands "set/show maint"
to replace those.  It would be used for instance to turn gdb self profiling
on/off.  I proposed that we use the deprecate the existent maint commands that
currently set things directly and add the new "set/show maint" for them.

So, for maintenance we would have:

maint <action> [<args>]
set maint <control var> <value>
show maint [<control var>]

All these are introspective commands that will examine/affect gdb in order
to debug, test or even to understand it.

Anyway, it is a modification on the current "maintenance" interface, that is
why I asked for some support beyond the two of us.

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

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

[Eli Zaretskii]

> Date: Wed, 19 Sep 2001 12:26:45 -0400
> From: Fernando Nasser <fnasser@cygnus.com>
> 
> So, for maintenance we would have:
> 
> maint <action> [<args>]
> set maint <control var> <value>
> show maint [<control var>]

Ah, this seems a good idea.  Thanks for the explanations.

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

[Andrew Cagney]

> maint <action> [<args>]
> set maint <control var> <value>
> show maint [<control var>]

just an aside to this, there is also

	set/show debug ....

but I don't think we need to change that to ``set/show maint debug 
...''.  ``set/show debug ...'' is very specific enable/disableing 
internal debug tracing.

enjoy,
Andrew

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

[Kevin Buettner]

On Sep 19, 12:26pm, Fernando Nasser wrote:

> Andrew has proposed creating a class of set/show commands "set/show maint"
> to replace those.  It would be used for instance to turn gdb self profiling
> on/off.  I proposed that we use the deprecate the existent maint commands that
> currently set things directly and add the new "set/show maint" for them.
> 
> So, for maintenance we would have:
> 
> maint <action> [<args>]
> set maint <control var> <value>
> show maint [<control var>]
> 
> All these are introspective commands that will examine/affect gdb in order
> to debug, test or even to understand it.
> 
> Anyway, it is a modification on the current "maintenance" interface, that is
> why I asked for some support beyond the two of us.

I like having the maintenance commands all lumped under "maint".  That
way I can use tab completion to see all of the maintenance related commands
when I do

    (gdb) maint <TAB>

I think a set/show would be useful for some activities; couldn't
we add "maint set ..." and "maint show ..."  ?

Kevin

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

[Michael Snyder]

Eli Zaretskii wrote:

> I'd like the documentation of the "maint profile-gdb" command to be in
> gdb.texinfo, not gdbint.texinfo.  The latter doesn't document user
> commands, and users might not readily know to look in gdbint.  (It's
> okay to have it described in both manuals, if you think it will be
> useful.)

At first I thought this was weird, but it's not.
Maintainer commands are documented in the user docs, not
the internals docs.  It has to do with maintaining gdb, 
not with gdb internals.  Good call.

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

[Andrew Cagney]

> 
> Emacs uses monstartup and manages to solve both of these problems.
> See the file src/emacs.c (search for "etext") for the details.

from emacs 20.7:

>   /* Gerd Moellmann <gerd@acm.org> says this makes profiling work on
>      FreeBSD.  It might work on some other systems too.
>      Give it a try and tell me if it works on your system.  */
> #ifdef __FreeBSD__
> #ifdef PROFILING
>   if (initialized)
>     {
>       extern void _mcleanup ();       
>       extern char etext;
>       extern Lisp_Object Fredraw_frame ();
>       atexit (_mcleanup);
>       /* This uses Fredraw_frame because that function
>          comes first in the Emacs executable.
>          It might be better to use something that gives
>          the start of the text segment, but start_of_text
>          is not defined on all systems now.  */
>       monstartup (Fredraw_frame, &etext);
>     }
>   else
>     moncontrol (0);
> #endif
> #endif

I get the feeling that this is work in progress.  I'd, for the moment, 
suggest using Jason's mechanism - it isn't perfect  but it works. 
Someone can later make it more fancy.

Andrew

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

[Eli Zaretskii]

> Date: Tue, 18 Sep 2001 01:43:02 -0400
> From: Andrew Cagney <ac131313@cygnus.com>
> 
> > Emacs uses monstartup and manages to solve both of these problems.
> > See the file src/emacs.c (search for "etext") for the details.
> 
> from emacs 20.7:
> 
> >   /* Gerd Moellmann <gerd@acm.org> says this makes profiling work on
> >      FreeBSD.  It might work on some other systems too.
> >      Give it a try and tell me if it works on your system.  */
[...]
> I get the feeling that this is work in progress.

This file is almost 2 years old.  Since then, this code was proven to
run unmodified on quite a few platforms.

Anyway, I don't want to continue arguing about this.  As long as you
know that the problems Jason raised can be solved with simple and
quite portable code, all the rest is your judgement call.

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

[Jason Molenda]

Sorry for the delay following up, I was sidetracked by some other 
things for a few days.

On Tue, Sep 18, 2001 at 09:51:16AM +0300, Eli Zaretskii wrote:

> > from emacs 20.7:
> > 
> > >   /* Gerd Moellmann <gerd@acm.org> says this makes profiling work on
> > >      FreeBSD.  It might work on some other systems too.
> > >      Give it a try and tell me if it works on your system.  */
> [...]
> > I get the feeling that this is work in progress.
> 
> This file is almost 2 years old.  Since then, this code was proven to
> run unmodified on quite a few platforms.

I'm a little hesitant to use this emacs code... besides having an
#ifdef FreeBSD around it, it uses the address of a certain function
for the beginning of the text section -- hardcoded.  So unless
someone can guess which one of gdb's functions is going to end up
at the beginning of .text, I don't see how that'd work.

It looks like emacs (in 20.7) is guarding all of these calls to
monstartup() and moncontrol() with #ifdef PROFILING.

I don't follow emacs development at all, so it's possible the
development sources are significantly different.

> Anyway, I don't want to continue arguing about this.  

I appreciate the time you're taking with this.  Okay, I would have
been happier if someone had just said "Approved!" and it had gone
in immediately :-), but it'll be a better patch by the time it does
get committed.

> As long as you
> know that the problems Jason raised can be solved with simple and
> quite portable code, all the rest is your judgement call.

I'm willing to believe that there are portable ways to do everything
we're talking about, but _I_ don't know them, and the emacs 20.7 code
doesn't show any examples of this...

I'll be updating the profiling patch and sending it again soon.

Jason

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

[Andrew Cagney]

>> > > /* Gerd Moellmann <gerd@acm.org> says this makes profiling work on
>> > > FreeBSD.  It might work on some other systems too.
>> > > Give it a try and tell me if it works on your system.  */

For what its worth.   I think this part of this discussion thread should 
be converted into a CR - something about profile always creating a 
profile file and notes on possible solutions.

Andrew

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

[Eli Zaretskii]

> Date: Mon, 24 Sep 2001 17:41:38 -0400
> From: Andrew Cagney <ac131313@cygnus.com>
> 
> For what its worth.   I think this part of this discussion thread should 
> be converted into a CR

What's a CR?  You didn't mean CR the character, \r, did you? ;-)

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

[Andrew Cagney]

> 
> What's a CR?  You didn't mean CR the character, \r, did you? 

Sorry, Change Reqest, one of the PR categories in the bug database. If 
I'm not careful, I'll be refering to them as SPRs (system performance 
reports) next ....

Andrew

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

[Eli Zaretskii]

> Date: Mon, 24 Sep 2001 13:32:15 -0700
> From: Jason Molenda <jason@molenda.com>
> 
> I'm a little hesitant to use this emacs code... besides having an
> #ifdef FreeBSD around it, it uses the address of a certain function
> for the beginning of the text section -- hardcoded.  So unless
> someone can guess which one of gdb's functions is going to end up
> at the beginning of .text, I don't see how that'd work.

The first function in the binary is the first function the linker
sees.  If this is platform-dependent in GDB, we could simply arrange
for an empty function that every platform links in.

> Okay, I would have been happier if someone had just said "Approved!"
> and it had gone in immediately :-)

I can't be that someone: it's not my responsibility ;-)