[PATCH] -stack-select-frame

[PATCH] -stack-select-frame

[Nick Roberts]

Last year I submitted a dreadful patch for -stack-select-frame
(thread subject: How does GDB/MI give the current frame).

Here, hopefully, is a much more sensible one.  The idea is the same: that
-stack-select-frame tells the frontend what the current or selected frame
is.

The current documentation for -stack-select-frame is slightly wrong because
the argument FRAMENUM is optional.  I've corrected this and explained what
the patch does.

Nick


2005-06-16  Nick Roberts  <nickrob@snap.net.nz>

	* mi/mi-cmd-stack.c (mi_cmd_stack_select_frame): Don't test for
	stack.  Print frame details.
	* gdb.texinfo (GDB/MI Stack Manipulation): Revise description
	of -stack-select-frame.


*** /home/nick/src/gdb/mi/mi-cmd-stack.c.~1.25.~	2005-02-13 00:36:20.000000000 +1300
--- /home/nick/src/gdb/mi/mi-cmd-stack.c	2005-06-16 14:28:29.000000000 +1200
***************
*** 329,337 ****
  enum mi_cmd_result
  mi_cmd_stack_select_frame (char *command, char **argv, int argc)
  {
-   if (!target_has_stack)
-     error (_("mi_cmd_stack_select_frame: No stack."));
- 
    if (argc > 1)
      error (_("mi_cmd_stack_select_frame: Usage: [FRAME_SPEC]"));
  
--- 329,334 ----
***************
*** 340,344 ****
--- 337,342 ----
      select_frame_command (0, 1 /* not used */ );
    else
      select_frame_command (argv[0], 1 /* not used */ );
+   print_stack_frame (get_selected_frame (NULL), 1, LOC_AND_ADDRESS);
    return MI_CMD_DONE;
  }


*** /home/nick/src/gdb/doc/gdb.texinfo.~1.261.~	2005-06-16 14:36:11.000000000 +1200
--- /home/nick/src/gdb/doc/gdb.texinfo	2005-06-16 15:01:18.000000000 +1200
***************
*** 19473,19483 ****
  @subsubheading Synopsis
  
  @smallexample
!  -stack-select-frame @var{framenum}
  @end smallexample
  
! Change the current frame.  Select a different frame @var{framenum} on
! the stack.
  
  @subsubheading @value{GDBN} Command
  
--- 19473,19484 ----
  @subsubheading Synopsis
  
  @smallexample
!  -stack-select-frame [ @var{framenum} ]
  @end smallexample
  
! Select a different frame @var{framenum} on the stack and print its
! details.  If the argument @var{framenum} is not specified, just print
! the details of the current frame.
  
  @subsubheading @value{GDBN} Command
  
***************
*** 19489,19495 ****
  @smallexample
  (@value{GDBP})
  -stack-select-frame 2
! ^done
  (@value{GDBP})
  @end smallexample
  
--- 19490,19497 ----
  @smallexample
  (@value{GDBP})
  -stack-select-frame 2
! ^done,frame=@{level="2",addr="0x000107a4",func="foo",
!   file="recursive2.c",fullname="/home/foo/bar/devo/myproject/recursive2.c",line=line="14"@},
  (@value{GDBP})
  @end smallexample

Re: [PATCH] -stack-select-frame

[Daniel Jacobowitz]

On Thu, Jun 16, 2005 at 03:37:08PM +1200, Nick Roberts wrote:
> 
> Last year I submitted a dreadful patch for -stack-select-frame
> (thread subject: How does GDB/MI give the current frame).
> 
> Here, hopefully, is a much more sensible one.  The idea is the same: that
> -stack-select-frame tells the frontend what the current or selected frame
> is.
> 
> The current documentation for -stack-select-frame is slightly wrong because
> the argument FRAMENUM is optional.  I've corrected this and explained what
> the patch does.

- You're changing the behavior of a command; do people use this
command?  Are you confident that they will handle the new output
gracefully?

- Please, let's not add new MI features without matching testcases.

> ! ^done,frame=@{level="2",addr="0x000107a4",func="foo",
> !   file="recursive2.c",fullname="/home/foo/bar/devo/myproject/recursive2.c",line=line="14"@},

The double line= is a typo, right?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -stack-select-frame

[Nick Roberts]

 > - You're changing the behavior of a command; do people use this
 > command?  Are you confident that they will handle the new output
 > gracefully?

I'm not aware of anyone using this command - I don't see how they currently
could really.  I think its less likely to break existing behaviour than this
change:

2005-05-17  Daniel Jacobowitz  <dan@codesourcery.com>
	    Dennis Brueni  <dennis@slickedit.com>

	* stack.c (print_frame): In MI mode, output a fullname attribute
	with the stack frame.

which must change the output for every frontend using MI.

I don't see how MI can evolve without changing its behaviour.  If it is
significantly different presumably a new level can be added as before.

 > - Please, let's not add new MI features without matching testcases.

I'll be happy to add testcases if there is a likelihood that my patch will be
approved.

 > > ! ^done,frame=@{level="2",addr="0x000107a4",func="foo",
 > > !   file="recursive2.c",fullname="/home/foo/bar/devo/myproject/recursive2.c",line=line="14"@},
 > 
 > The double line= is a typo, right?

I've just copied it from another part of the manual.  In the node
"GDB/MI Command Description Format" it says:

Manual>    Note the the line breaks shown in the examples are here only for
Manual> readability.  They don't appear in the real output.


Nick

Re: [PATCH] -stack-select-frame

[Daniel Jacobowitz]

On Thu, Jun 16, 2005 at 05:55:07PM +1200, Nick Roberts wrote:
>  > - You're changing the behavior of a command; do people use this
>  > command?  Are you confident that they will handle the new output
>  > gracefully?
> 
> I'm not aware of anyone using this command - I don't see how they currently
> could really.  I think its less likely to break existing behaviour than this
> change:
> 
> 2005-05-17  Daniel Jacobowitz  <dan@codesourcery.com>
> 	    Dennis Brueni  <dennis@slickedit.com>
> 
> 	* stack.c (print_frame): In MI mode, output a fullname attribute
> 	with the stack frame.
> 
> which must change the output for every frontend using MI.

My concern was that this added a new item to a response which previous
had zero.  Why do you think people couldn't use it as is?  Seems
perfectly usable to me; if you know the level of a frame you want to
select, then you probably already have the result of a backtrace which
includes a printout of the stack frame, so you probably don't need
another!

Could you explain why you think we need output here?

> I don't see how MI can evolve without changing its behaviour.  If it is
> significantly different presumably a new level can be added as before.

That was my point: whether we needed to do that.  I was just asking.

>  > > ! ^done,frame=@{level="2",addr="0x000107a4",func="foo",
>  > > !   file="recursive2.c",fullname="/home/foo/bar/devo/myproject/recursive2.c",line=line="14"@},
>  > 
>  > The double line= is a typo, right?
> 
> I've just copied it from another part of the manual.  In the node
> "GDB/MI Command Description Format" it says:
> 
> Manual>    Note the the line breaks shown in the examples are here only for
> Manual> readability.  They don't appear in the real output.

No, the bit that says "line=line="14"".

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -stack-select-frame

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Thu, 16 Jun 2005 15:37:08 +1200
> 
> Last year I submitted a dreadful patch for -stack-select-frame
> (thread subject: How does GDB/MI give the current frame).
> 
> Here, hopefully, is a much more sensible one.  The idea is the same: that
> -stack-select-frame tells the frontend what the current or selected frame
> is.
> 
> The current documentation for -stack-select-frame is slightly wrong because
> the argument FRAMENUM is optional.  I've corrected this and explained what
> the patch does.

Thanks.  The doco patch is approved, modulo the minor correction
pointed out by Daniel.

Re: [PATCH] -stack-select-frame

[Jason Molenda]

Hi Nick,

You wrote:

> I'm not aware of anyone using this command - I don't see how they  
> currently
> could really.  I think its less likely to break existing behaviour  
> than this
> change:

Xcode uses stack-select-frame -- I'm not sure how a real GUI could  
avoid using stack-select-frame.


On Jun 15, 2005, at 8:37 PM, Nick Roberts wrote:

> Here, hopefully, is a much more sensible one.  The idea is the  
> same: that
> -stack-select-frame tells the frontend what the current or selected  
> frame
> is.

I don't care, really, but isn't necessary in our experience at  
Apple.  The first thing a GUI has to do when execution has stopped is  
get the current stack.  It shows the stack to the developer, and then  
the developer decides that stack frame 2 is of interest to her, so  
she clicks there and the GUI sends stack-select-frame.  The GUI  
already has the stack-list-frames output from when it stopped -- it  
knows very well what's at frame 2.

If the GUI has short term memory problems, stack-list-frames is  
always at its disposal.

>     * mi/mi-cmd-stack.c (mi_cmd_stack_select_frame): Don't test for
>     stack.  Print frame details.

>   enum mi_cmd_result
>   mi_cmd_stack_select_frame (char *command, char **argv, int argc)
>   {
> -   if (!target_has_stack)
> -     error (_("mi_cmd_stack_select_frame: No stack."));
> -

Why are you suggesting this change?  If the GUI is requesting a stack  
when the inferior isn't running, that's an error.

Jason

Re: [PATCH] -stack-select-frame

[Nick Roberts]

 > My concern was that this added a new item to a response which previous
 > had zero.  Why do you think people couldn't use it as is?  Seems
 > perfectly usable to me; if you know the level of a frame you want to
 > select, then you probably already have the result of a backtrace which
 > includes a printout of the stack frame, so you probably don't need
 > another!

-stack-select-frame without an argument currently works like the CLI command
"frame" without the output.  All "frame" does is output the current frame,
so without output its a bit of a no-op.

-stack-select-frame with an argument just works like up, down or more
precisely "frame FRAMENUM".  So ,these CLI commands could be used directly in
the short term, and via "-interpreter-exec console" in the longer term, if the
frontend is not interested in the output.

 > Could you explain why you think we need output here?

If you have a backtrace then, no, you don't need the output and the frontend
can ignore it.  However, I presume "-stack-select-frame" runs more quickly
than "-stack-list-frames" so, if you don't need a backtrace, its probably best
not to require one.

 > >  > > ! ^done,frame=@{level="2",addr="0x000107a4",func="foo",
 > >  > > !   file="recursive2.c",fullname="/home/foo/bar/devo/myproject/recursive2.c",line=line="14"@},
 > >  > 
 > >  > The double line= is a typo, right?
 > > 
 > > I've just copied it from another part of the manual.  In the node
 > > "GDB/MI Command Description Format" it says:
 > > 
 > > Manual>    Note the the line breaks shown in the examples are here only for
 > > Manual> readability.  They don't appear in the real output.
 > 
 > No, the bit that says "line=line="14"".

Doh!

Nick

Re: [PATCH] -stack-select-frame

[Nick Roberts]

 > I don't care, really, but isn't necessary in our experience at  
 > Apple.  The first thing a GUI has to do when execution has stopped is  
 > get the current stack.  It shows the stack to the developer, and then  
 > the developer decides that stack frame 2 is of interest to her, so  
 > she clicks there and the GUI sends stack-select-frame.  The GUI  
 > already has the stack-list-frames output from when it stopped -- it  
 > knows very well what's at frame 2.

I thought it would save some time if the user doesn't need to see the
whole stack.


 > If the GUI has short term memory problems, stack-list-frames is  
 > always at its disposal.
 > 
 > >     * mi/mi-cmd-stack.c (mi_cmd_stack_select_frame): Don't test for
 > >     stack.  Print frame details.
 > 
 > >   enum mi_cmd_result
 > >   mi_cmd_stack_select_frame (char *command, char **argv, int argc)
 > >   {
 > > -   if (!target_has_stack)
 > > -     error (_("mi_cmd_stack_select_frame: No stack."));
 > > -

Because now GDB will report it as follows:

-stack-select-frame
&"No stack.\n"
^error,msg="No stack."
(gdb) 


Nick

Re: [PATCH] -stack-select-frame

[Bob Rossi]

On Fri, Jun 17, 2005 at 10:50:30AM +1200, Nick Roberts wrote:
>  > My concern was that this added a new item to a response which previous
>  > had zero.  Why do you think people couldn't use it as is?  Seems
>  > perfectly usable to me; if you know the level of a frame you want to
>  > select, then you probably already have the result of a backtrace which
>  > includes a printout of the stack frame, so you probably don't need
>  > another!
> 
> -stack-select-frame without an argument currently works like the CLI command
> "frame" without the output.  All "frame" does is output the current frame,
> so without output its a bit of a no-op.
> 
> -stack-select-frame with an argument just works like up, down or more
> precisely "frame FRAMENUM".  So ,these CLI commands could be used directly in
> the short term, and via "-interpreter-exec console" in the longer term, if the
> frontend is not interested in the output.
> 
>  > Could you explain why you think we need output here?
> 
> If you have a backtrace then, no, you don't need the output and the frontend
> can ignore it.  However, I presume "-stack-select-frame" runs more quickly
> than "-stack-list-frames" so, if you don't need a backtrace, its probably best
> not to require one.

Yes, that sounds nice. Especially on an infinate recursion crash. The
stack trace would take way to long to get back, so going frame by frame
would be much nicer.

Bob Rossi

Re: [PATCH] -stack-select-frame

[Jason Molenda]

On Jun 16, 2005, at 4:05 PM, Nick Roberts wrote:

> I thought it would save some time if the user doesn't need to see the
> whole stack.

FWIW we've done a lot of careful timing analysis, and the back &  
forth communication between our GUI and gdb is so fast as to be  
pointless to optimize.  We original considered adding special purpose  
"give Xcode everything it needs to know at a breakpoint hit" type  
commands but when we saw how fast the majority of MI commands can  
execute & be parsed by the GUI, it was obvious that this was not a  
useful area to optimize.  And frankly, in my anecdotal experience,  
MacOS X isn't the fastest OS at things like "two processes talking  
over a pipe".

(one of the parts of this profiling which is especially useful is  
that we have a "mi-timings-enabled" setting.  When it's enabled,  
every MI command reports how long gdb took to complete it, e.g. the  
"time=" bit at the end here:

-> 50-stack-list-frames 0 5
<- 50^done,stack=[frame= 
{level="0",addr="0x0009e7fc",fp="0xbfffe700",func=" [...] ,frame= 
{level="5",addr="0x936265d0",fp="0xbfffeee0",func="-[NSApplication  
run]"}],time= 
{wallclock="0.14353",user="0.00584",system="0.00335",start="1118952348.0 
03847",end="1118952348.147372"}

we have similar timestamps put out by the GUI so when there's a  
slowdown we can establish where time was being spent.)


>>>   enum mi_cmd_result
>>>   mi_cmd_stack_select_frame (char *command, char **argv, int argc)
>>>   {
>>> -   if (!target_has_stack)
>>> -     error (_("mi_cmd_stack_select_frame: No stack."));
>>> -
>>>
>
> Because now GDB will report it as follows:
>
> -stack-select-frame
> &"No stack.\n"
> ^error,msg="No stack."
> (gdb)


You're right, currently it behaves like this:

-stack-select-frame
&"mi_cmd_stack_select_frame: No stack.\n"
^error,msg="mi_cmd_stack_select_frame: No stack."
(gdb)

but select_frame_command () will throw the error eventually, so  
there's no need to do the additional check here.


J

Re: [PATCH] -stack-select-frame

[Daniel Jacobowitz]

On Fri, Jun 17, 2005 at 10:50:30AM +1200, Nick Roberts wrote:
>  > My concern was that this added a new item to a response which previous
>  > had zero.  Why do you think people couldn't use it as is?  Seems
>  > perfectly usable to me; if you know the level of a frame you want to
>  > select, then you probably already have the result of a backtrace which
>  > includes a printout of the stack frame, so you probably don't need
>  > another!
> 
> -stack-select-frame without an argument currently works like the CLI command
> "frame" without the output.  All "frame" does is output the current frame,
> so without output its a bit of a no-op.

OK, so obviously that's a loss.  Either we should:
 - reject it without an argument
 - make it print without an argument
 - make it print always

You did the last of those.  I'm trying to figure out if other users of
-stack-select-frame want that behavior.

>  > Could you explain why you think we need output here?
> 
> If you have a backtrace then, no, you don't need the output and the frontend
> can ignore it.  However, I presume "-stack-select-frame" runs more quickly
> than "-stack-list-frames" so, if you don't need a backtrace, its probably best
> not to require one.

We've already got -stack-info-frame.  If you want to avoid
-stack-list-frames, is it unreasonable to do the two round trips for
-stack-select-frame / -stack-info-frame?  From Jason's measurements, it
sounds like that isn't a problem.

Not that it would be a terrible change to print out the frame.  It's
just a question of whether there's benefit.  It'll make
-stack-select-frame (again, only marginally) slower.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -stack-select-frame

[Nick Roberts]

 > > -stack-select-frame without an argument currently works like the CLI
 > > command "frame" without the output.  All "frame" does is output the
 > > current frame, so without output its a bit of a no-op.
 > 
 > OK, so obviously that's a loss.  Either we should:
 >  - reject it without an argument
 >  - make it print without an argument
 >  - make it print always
 > 
 > You did the last of those.  I'm trying to figure out if other users of
 > -stack-select-frame want that behavior.
 > 
 > >  > Could you explain why you think we need output here?
 > > 
 > > If you have a backtrace then, no, you don't need the output and the
 > > frontend can ignore it.  However, I presume "-stack-select-frame" runs
 > > more quickly than "-stack-list-frames" so, if you don't need a backtrace,
 > > its probably best not to require one.
 > 
 > We've already got -stack-info-frame.  If you want to avoid
 > -stack-list-frames, is it unreasonable to do the two round trips for
 > -stack-select-frame / -stack-info-frame?  From Jason's measurements, it
 > sounds like that isn't a problem.

-stack-info-frame hasn't been implemented yet (I've think we've been here
before) but it would probably be quite easy to implement and I guess it
could work like I've made -stack-select-frame without an argument work.
However, the documentation suggests that it should work like "info frame",
so perhaps its expected to have more information.

Its also just occurred to me that "-stack-list-frames 0 0" is similar too.

 > Not that it would be a terrible change to print out the frame.  It's
 > just a question of whether there's benefit.  It'll make
 > -stack-select-frame (again, only marginally) slower.

How about installing your second choice, just printing the frame when there is
no argument?  I presume that when Apple use -stack-select-frame, it is always
with an argument (doco will change to match):

More generally I think it would be a good idea to mention in the manual
that MI is still undergoing change to reduce the obligation to maintain
legacy code.


*** /home/nick/src/gdb/mi/mi-cmd-stack.c.~1.25.~	2005-02-13 00:36:20.000000000 +1300
--- /home/nick/src/gdb/mi/mi-cmd-stack.c	2005-06-17 14:57:19.000000000 +1200
***************
*** 47,55 ****
    struct cleanup *cleanup_stack;
    struct frame_info *fi;
  
-   if (!target_has_stack)
-     error (_("mi_cmd_stack_list_frames: No stack."));
- 
    if (argc > 2 || argc == 1)
      error (_("mi_cmd_stack_list_frames: Usage: [FRAME_LOW FRAME_HIGH]"));
  
--- 47,52 ----
***************
*** 104,112 ****
    int i;
    struct frame_info *fi;
  
-   if (!target_has_stack)
-     error (_("mi_cmd_stack_info_depth: No stack."));
- 
    if (argc > 1)
      error (_("mi_cmd_stack_info_depth: Usage: [MAX_DEPTH]"));
  
--- 101,106 ----
***************
*** 329,343 ****
  enum mi_cmd_result
  mi_cmd_stack_select_frame (char *command, char **argv, int argc)
  {
-   if (!target_has_stack)
-     error (_("mi_cmd_stack_select_frame: No stack."));
- 
    if (argc > 1)
      error (_("mi_cmd_stack_select_frame: Usage: [FRAME_SPEC]"));
  
    /* with no args, don't change frame */
    if (argc == 0)
!     select_frame_command (0, 1 /* not used */ );
    else
      select_frame_command (argv[0], 1 /* not used */ );
    return MI_CMD_DONE;
--- 323,337 ----
  enum mi_cmd_result
  mi_cmd_stack_select_frame (char *command, char **argv, int argc)
  {
    if (argc > 1)
      error (_("mi_cmd_stack_select_frame: Usage: [FRAME_SPEC]"));
  
    /* with no args, don't change frame */
    if (argc == 0)
!     {
!       select_frame_command (0, 1 /* not used */ );
!       print_stack_frame (get_selected_frame (NULL), 1, LOC_AND_ADDRESS);
!     }
    else
      select_frame_command (argv[0], 1 /* not used */ );
    return MI_CMD_DONE;

Re: [PATCH] -stack-select-frame

[Daniel Jacobowitz]

On Fri, Jun 17, 2005 at 03:09:17PM +1200, Nick Roberts wrote:
>  > We've already got -stack-info-frame.  If you want to avoid
>  > -stack-list-frames, is it unreasonable to do the two round trips for
>  > -stack-select-frame / -stack-info-frame?  From Jason's measurements, it
>  > sounds like that isn't a problem.
> 
> -stack-info-frame hasn't been implemented yet (I've think we've been here
> before) but it would probably be quite easy to implement and I guess it
> could work like I've made -stack-select-frame without an argument work.

*snicker* that's what I get for reading the manual.  I assumed it was
implemented.

Maybe it is time to mark the unimplemented commands in the manual?

> However, the documentation suggests that it should work like "info frame",
> so perhaps its expected to have more information.

GDB Command
...........

The corresponding GDB command is `info frame' or `frame' (without
arguments).

I think we've got some leeway here.  I'd rather not expose the rest of
"info frame" to frontends without a demonstrated need.

>  > Not that it would be a terrible change to print out the frame.  It's
>  > just a question of whether there's benefit.  It'll make
>  > -stack-select-frame (again, only marginally) slower.
> 
> How about installing your second choice, just printing the frame when there is
> no argument?  I presume that when Apple use -stack-select-frame, it is always
> with an argument (doco will change to match):

If you're OK with only a literal "-stack-select-frame" providing the
frame information, how about implementing -stack-info-frame instead? 
The documentation for -stack-select-frame does not suggest the argument
is optional; we could make it mandatory.  It just seems cleaner to me
to have select be write-only and info be read-only.

> More generally I think it would be a good idea to mention in the manual
> that MI is still undergoing change to reduce the obligation to maintain
> legacy code.

I'm not real thrilled with doing this; we don't get to decide what
people do or do not use, so documenting things that people do use as
unstable does no one any favors.  However, I've been hearing a lot of
convincing points that we have more freedom here than I thought.  So
I'm getting more comfortable with changes.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -stack-select-frame

[Nick Roberts]

[Jason - Sorry if you get this twice.  My first message bounced at RedHat, for
some reason.]

 > > I thought it would save some time if the user doesn't need to see the
 > > whole stack.
 > 
 > FWIW we've done a lot of careful timing analysis, and the back &  
 > forth communication between our GUI and gdb is so fast as to be  
 > pointless to optimize.  We original considered adding special purpose  
 > "give Xcode everything it needs to know at a breakpoint hit" type  
 > commands but when we saw how fast the majority of MI commands can  
 > execute & be parsed by the GUI, it was obvious that this was not a  
 > useful area to optimize.  And frankly, in my anecdotal experience,  
 > MacOS X isn't the fastest OS at things like "two processes talking  
 > over a pipe".

You've clearly been more quantitative.  With my limited resources, I'm
just guessing what might work best.  I've suggested to Daniel a change
that, I hope, won't impact on Xcode.  I think you have your own copy
of GDB and, like you say, you don't really care, but I guess its best
not to diverge more than necessary.

 > (one of the parts of this profiling which is especially useful is  
 > that we have a "mi-timings-enabled" setting.  When it's enabled,  
 > every MI command reports how long gdb took to complete it, e.g. the  
 > "time=" bit at the end here:
 > 
 > -> 50-stack-list-frames 0 5
 > <- 50^done,stack=[frame= 
 > {level="0",addr="0x0009e7fc",fp="0xbfffe700",func=" [...] ,frame= 
 > {level="5",addr="0x936265d0",fp="0xbfffeee0",func="-[NSApplication  
 > run]"}],time= 
 > {wallclock="0.14353",user="0.00584",system="0.00335",start="1118952348.0 
 > 03847",end="1118952348.147372"}

Yes but what happens when the stack is much deeper, 20 or 30 say, like it can
be when you you are debugging Emacs, or GDB for that matter?

Nick

Re: [PATCH] -stack-select-frame

[Nick Roberts]

 > > -stack-info-frame hasn't been implemented yet (I've think we've been here
 > > before) but it would probably be quite easy to implement and I guess it
 > > could work like I've made -stack-select-frame without an argument work.
 > 
 > *snicker* that's what I get for reading the manual.  I assumed it was
 > implemented.
 > 
 > Maybe it is time to mark the unimplemented commands in the manual?

It _is_ documented as unimplemented in my copy.

   ...
 > If you're OK with only a literal "-stack-select-frame" providing the
 > frame information, how about implementing -stack-info-frame instead? 
 > The documentation for -stack-select-frame does not suggest the argument
 > is optional; we could make it mandatory.  It just seems cleaner to me
 > to have select be write-only and info be read-only.

The CLI command is both write-only and read-only.  Clearly there's no
 need to copy what might be existing bad practice and the command name
-stack-select-frame to provide frame information is unintuitive. I'll 
submit a patch to make -stack-info-frame do this.

Nick

Re: [PATCH] -stack-select-frame

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Fri, 17 Jun 2005 15:09:17 +1200
> Cc: gdb-patches@sources.redhat.com
> 
> More generally I think it would be a good idea to mention in the manual
> that MI is still undergoing change

We already do that:

       Note that GDB/MI is still under construction, so some of the
    features described below are incomplete and subject to change.

Re: [PATCH] -stack-select-frame

[Eli Zaretskii]

> Date: Thu, 16 Jun 2005 23:21:49 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: gdb-patches@sources.redhat.com
> 
> Maybe it is time to mark the unimplemented commands in the manual?

They should generally be excluded from the manual.  Feel free to ifdef
them away (using @ignore..@end ignore), with a comment saying that
they are not implemented yet.  If you don't have time, I will get to
that eventually.  Thanks for drawing my attention to this command; I
must have missed it when I excluded all the other unimplemented MI
commands while integrating gdbmi.texinfo into the manual.

> I'm not real thrilled with doing this; we don't get to decide what
> people do or do not use, so documenting things that people do use as
> unstable does no one any favors.  However, I've been hearing a lot of
> convincing points that we have more freedom here than I thought.  So
> I'm getting more comfortable with changes.

I, too, prefer stabilizing MI to saying that it's unstable.  But if no
one steps forward to do the work, I see nothing wrong in warning users
that a particular GDB area is under construction.

Re: [PATCH] -stack-select-frame

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Fri, 17 Jun 2005 19:39:00 +1200
> Cc: gdb-patches@sources.redhat.com
> 
>  > > -stack-info-frame hasn't been implemented yet (I've think we've been here
>  > > before) but it would probably be quite easy to implement and I guess it
>  > > could work like I've made -stack-select-frame without an argument work.
>  > 
>  > *snicker* that's what I get for reading the manual.  I assumed it was
>  > implemented.
>  > 
>  > Maybe it is time to mark the unimplemented commands in the manual?
> 
> It _is_ documented as unimplemented in my copy.

Well, not really ``documented as''.  The manual says that commands
whose "Example" section says "N.A." are not implemented, but a reader
who reads a description of a certain command will not necessarily
deduce that.

I think we should simply @ignore-out such commands.

Re: [PATCH] -stack-select-frame

[Daniel Jacobowitz]

On Fri, Jun 17, 2005 at 07:24:01PM +1200, Nick Roberts wrote:
>  > (one of the parts of this profiling which is especially useful is  
>  > that we have a "mi-timings-enabled" setting.  When it's enabled,  
>  > every MI command reports how long gdb took to complete it, e.g. the  
>  > "time=" bit at the end here:
>  > 
>  > -> 50-stack-list-frames 0 5
>  > <- 50^done,stack=[frame= 
>  > {level="0",addr="0x0009e7fc",fp="0xbfffe700",func=" [...] ,frame= 
>  > {level="5",addr="0x936265d0",fp="0xbfffeee0",func="-[NSApplication  
>  > run]"}],time= 
>  > {wallclock="0.14353",user="0.00584",system="0.00335",start="1118952348.0 
>  > 03847",end="1118952348.147372"}
> 
> Yes but what happens when the stack is much deeper, 20 or 30 say, like it can
> be when you you are debugging Emacs, or GDB for that matter?

Just a guess but: why ask GDB for more stack frames than fit in the
relevant window?  You can ask for more (to ensure smooth scrolling),
but do it while the user's doing something else.  An MI frontend
doesn't need to ask for all frames if it's worried about how long that
will take.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -stack-select-frame

[Daniel Jacobowitz]

On Fri, Jun 17, 2005 at 01:14:56PM +0200, Eli Zaretskii wrote:
> > From: Nick Roberts <nickrob@snap.net.nz>
> > Date: Fri, 17 Jun 2005 19:39:00 +1200
> > Cc: gdb-patches@sources.redhat.com
> > 
> >  > > -stack-info-frame hasn't been implemented yet (I've think we've been here
> >  > > before) but it would probably be quite easy to implement and I guess it
> >  > > could work like I've made -stack-select-frame without an argument work.
> >  > 
> >  > *snicker* that's what I get for reading the manual.  I assumed it was
> >  > implemented.
> >  > 
> >  > Maybe it is time to mark the unimplemented commands in the manual?
> > 
> > It _is_ documented as unimplemented in my copy.
> 
> Well, not really ``documented as''.  The manual says that commands
> whose "Example" section says "N.A." are not implemented, but a reader
> who reads a description of a certain command will not necessarily
> deduce that.

It took me a few minutes poking through the manual to find this, even
after you told me it was there...

> I think we should simply @ignore-out such commands.

I'm OK with that, especially if you prefer it.  I'd like to either do
that, or expand the N.A. to be explicit ("N.A.@: - not implemented
yet").

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -stack-select-frame

[Jason Molenda]

On Jun 17, 2005, at 12:24 AM, Nick Roberts wrote:

>> (one of the parts of this profiling which is especially useful is
>> that we have a "mi-timings-enabled" setting.  When it's enabled,
>> every MI command reports how long gdb took to complete it, e.g. the
>> "time=" bit at the end here:
>>
>> -> 50-stack-list-frames 0 5
>> <- 50^done,stack=[frame=
>> {level="0",addr="0x0009e7fc",fp="0xbfffe700",func=" [...] ,frame=
>> {level="5",addr="0x936265d0",fp="0xbfffeee0",func="-[NSApplication
>> run]"}],time=
>> {wallclock="0.14353",user="0.00584",system="0.00335",start="111895234 
>> 8.0
>> 03847",end="1118952348.147372"}
>>
>
> Yes but what happens when the stack is much deeper, 20 or 30 say,  
> like it can
> be when you you are debugging Emacs, or GDB for that matter?


Yes, stack crawling is slow, and stacks 30-50 frames deep are not at  
all unusual in GUI heavy-call-back applications like we have on MacOS  
X.  When we've worked on optimizing performance on something like a  
user pressing the "Next" button, -stack-list-frames is a serious  
problem.

We implemented a command, "-stack-list-frames-lite" here at Apple.   
It crawls the stack (there's a -limit option to specify how many  
frames to crawl) and shows the pc + fp for each frame.  A "stop at a  
breakpoint and press next a few times" scenario looks like

-thread-list-ids
-stack-list-frames-lite -limit 500
-stack-list-frames 0 7  (I had eight frames at this point)
-stack-select-frame 0
-stack-list-arguments 2 0 0
-stack-list-locals 2 1
-file-list-statics -file "*CURRENT FRAME*" -shlib "" "--no-values" - 
filter "^_OBJC" -constants 0
-exec-next

-thread-list-ids
-stack-list-frames-lite -limit 500
-stack-select-frame 0
-stack-list-frames 0 0
-var-update "var1" "var2" "var3"
-exec-next

and so on.  So we only do the (expensive) -stack-list-frames the  
first time we enter a function.

But the point I originally was trying to make is that the overhead of  
issuing MI commands is really, really, really low on MacOS X.  For  
instance, -stack-list-frames 0 0 took 0.00039 seconds wallclock to  
return its information in a quick test I just did.  -stack-list- 
frames-lite with 8 frames took 0.01422 seconds.  -var-update with  
three varobjs took 0.00493 seconds.

Yeah, I happen to be using a really fast G5 system right now, but we  
do all of our performance timings on slower boxes (currently around  
the 1GHz range) and my own personal goal is to keep the time it takes  
for a "Next" button press to finish should be no more than 0.2  
seconds, and it should be a *reliable* amount of time.  Users get  
into a rhythm of pressing "Next" and if it takes much longer than 0.2  
seconds, or if it takes different amounts of time, they get grumpy.

Oops, I'm off topic again.  MI commands fast.  It's not going to  
cause me any problems if you add more output to existing commands,  
but we've seen little benefit in trying to combine discrete commands  
here at Apple.

J

Re: [PATCH] -stack-select-frame

[Stan Shebs]

Nick Roberts wrote:

>[Jason - Sorry if you get this twice.  My first message bounced at RedHat, for
>some reason.]
>
> > > I thought it would save some time if the user doesn't need to see the
> > > whole stack.
> > 
> > FWIW we've done a lot of careful timing analysis, and the back &  
> > forth communication between our GUI and gdb is so fast as to be  
> > pointless to optimize.  We original considered adding special purpose  
> > "give Xcode everything it needs to know at a breakpoint hit" type  
> > commands but when we saw how fast the majority of MI commands can  
> > execute & be parsed by the GUI, it was obvious that this was not a  
> > useful area to optimize.  And frankly, in my anecdotal experience,  
> > MacOS X isn't the fastest OS at things like "two processes talking  
> > over a pipe".
>
>You've clearly been more quantitative.  With my limited resources, I'm
>just guessing what might work best.  I've suggested to Daniel a change
>that, I hope, won't impact on Xcode.  I think you have your own copy
>of GDB and, like you say, you don't really care, but I guess its best
>not to diverge more than necessary.
>
Ideally, future versions of Apple GDB will be much more like FSF GDB.
I'm hip-deep in a merge right now, and adding lots of local markers
so we can pick out candidate patches more easily; once all that's
done, we should be able to start contributing more (starting with
the basic Darwin native bits, it's a nuisance to have to use other
configs to test even trivial patches).

Stan

Re: [PATCH] -stack-select-frame

[Daniel Jacobowitz]

On Fri, Jun 17, 2005 at 03:34:59PM -0700, Stan Shebs wrote:
> Ideally, future versions of Apple GDB will be much more like FSF GDB.
> I'm hip-deep in a merge right now, and adding lots of local markers
> so we can pick out candidate patches more easily; once all that's
> done, we should be able to start contributing more (starting with
> the basic Darwin native bits, it's a nuisance to have to use other
> configs to test even trivial patches).

Can't wait!  I'm looking forward to this.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [PATCH] -stack-select-frame

[Nick Roberts]

 > Ideally, future versions of Apple GDB will be much more like FSF GDB.
 > I'm hip-deep in a merge right now, and adding lots of local markers
 > so we can pick out candidate patches more easily; once all that's
 > done, we should be able to start contributing more (starting with
 > the basic Darwin native bits, it's a nuisance to have to use other
 > configs to test even trivial patches).

This would speed things up considerably and be most welcome.  

Nick

Re: [PATCH] -stack-select-frame

[Eli Zaretskii]

> Date: Fri, 17 Jun 2005 09:33:11 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Nick Roberts <nickrob@snap.net.nz>, gdb-patches@sources.redhat.com
> 
> > I think we should simply @ignore-out such commands.
> 
> I'm OK with that, especially if you prefer it.  I'd like to either do
> that, or expand the N.A. to be explicit ("N.A.@: - not implemented
> yet").

The choice depends on whether we expect someone to work on
implementing at least some of these commands shortly.  If we do, it is
better to leave them visible in the manual, so that potential
volunteers would see them, which means the latter possibility.

If we don't think they will be implemented soon, it doesn't make sense
to me to leave them in the manual.

Re: [PATCH] -stack-select-frame

[Jim Ingham]

On Jun 17, 2005, at 3:15 AM, gdb-patches-digest- 
help@sources.redhat.com wrote:

>
>
>
> [Jason - Sorry if you get this twice.  My first message bounced at  
> RedHat, for
> some reason.]
>
>
>>> I thought it would save some time if the user doesn't need to see  
>>> the
>>> whole stack.
>>>
>>
>> FWIW we've done a lot of careful timing analysis, and the back &
>> forth communication between our GUI and gdb is so fast as to be
>> pointless to optimize.  We original considered adding special purpose
>> "give Xcode everything it needs to know at a breakpoint hit" type
>> commands but when we saw how fast the majority of MI commands can
>> execute & be parsed by the GUI, it was obvious that this was not a
>> useful area to optimize.  And frankly, in my anecdotal experience,
>> MacOS X isn't the fastest OS at things like "two processes talking
>> over a pipe".
>>
>
> You've clearly been more quantitative.  With my limited resources, I'm
> just guessing what might work best.  I've suggested to Daniel a change
> that, I hope, won't impact on Xcode.  I think you have your own copy
> of GDB and, like you say, you don't really care, but I guess its best
> not to diverge more than necessary.

Yes, thats right.  We would rather not introduce gratuitous divergences.

>
>
>> (one of the parts of this profiling which is especially useful is
>> that we have a "mi-timings-enabled" setting.  When it's enabled,
>> every MI command reports how long gdb took to complete it, e.g. the
>> "time=" bit at the end here:
>>
>> -> 50-stack-list-frames 0 5
>> <- 50^done,stack=[frame=
>> {level="0",addr="0x0009e7fc",fp="0xbfffe700",func=" [...] ,frame=
>> {level="5",addr="0x936265d0",fp="0xbfffeee0",func="-[NSApplication
>> run]"}],time=
>> {wallclock="0.14353",user="0.00584",system="0.00335",start="111895234 
>> 8.0
>> 03847",end="1118952348.147372"}
>>
>
> Yes but what happens when the stack is much deeper, 20 or 30 say,  
> like it can
> be when you you are debugging Emacs, or GDB for that matter?


You are right to worry about this a bit.  Getting a full backtrace  
when the stack is deep can be expensive, especially when you do it  
every step.  And using a complex kit like AppKit or Carbon where lots  
of the work is done through callbacks, etc, can lead to really deep  
stacks as well.  OTOH, the stack window is sitting there open, so we  
have to keep it up to date.  We got hit with this pretty early on in  
getting Xcode working.

What we did was add another command (-stack-list-frames-lite) that  
just returns the pairs (pc/sp) for the current stack.  You can  
usually implement this to run quite quickly on the target side  
without having to do the full register unwind, etc (*).  Then every  
time we stop, Xcode calls this -stack-list-frames-lite, and compares  
the results with the stack it had before the target started up.  Then  
it only needs to fetch the frames that have changed.  For extra  
credit, it could not fetch the frames for any that aren't visible in  
the current stack window in the GUI, though Xcode hasn't done that yet.

We still pay a cost when you arrive in a totally new stack.  But  
since the target usually has to run a bit for the stack to change a  
lot, this cost gets mixed in with the target running, and doesn't  
create such a bad perceived slowness.  But it removes most of the  
problems that stack tracing gave us when stepping, which is where a  
lag really annoys users.


(*) I think Andrew's design of the frame caching stuff should make  
this unnecessary if the architecture specific unwinders were smart  
enough to only unwind the one register they were asked to every  
time.  We haven't done that yet, but that's more an implememtation  
detail.

Jim


>
> Nick
>