[RFA] Remove command_line_handler_continuation.

[RFA] Remove command_line_handler_continuation.

[Vladimir Prus]

With my previous patch to move all bpstat_do_actions calls into
event handler, all that command_line_handler_continuation does
is to print timings, if those are enabled. However, those timings
don't make much sense. Timings are good for debugging GDB performance,
and in case of command_line_handler_continuation, the times will report
the time when the application run.

So, this patch removes that continuation. OK?

- Volodya

	* top.c (command_line_handler_continuation): Remove.
	(execute_command): Do not install the above.
---
 gdb/top.c |   55 -------------------------------------------------------
 1 files changed, 0 insertions(+), 55 deletions(-)

diff --git a/gdb/top.c b/gdb/top.c
index feefcf0..e0293df 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -364,43 +364,6 @@ do_chdir_cleanup (void *old_dir)
 }
 #endif
 
-/* Do any commands attached to breakpoint we stopped at. Only if we
-   are always running synchronously. Or if we have just executed a
-   command that doesn't start the target.  */
-static void
-command_line_handler_continuation (struct continuation_arg *arg, int error)
-{
-  extern int display_time;
-  extern int display_space;
-
-  long time_at_cmd_start  = arg->data.longint;
-  long space_at_cmd_start = arg->next->data.longint;
-
-  if (error)
-    return;
-
-  if (display_time)
-    {
-      long cmd_time = get_run_time () - time_at_cmd_start;
-
-      printf_unfiltered (_("Command execution time: %ld.%06ld\n"),
-			 cmd_time / 1000000, cmd_time % 1000000);
-    }
-  if (display_space)
-    {
-#ifdef HAVE_SBRK
-      char *lim = (char *) sbrk (0);
-      long space_now = lim - lim_at_start;
-      long space_diff = space_now - space_at_cmd_start;
-
-      printf_unfiltered (_("Space used: %ld (%c%ld for this command)\n"),
-			 space_now,
-			 (space_diff >= 0 ? '+' : '-'),
-			 space_diff);
-#endif
-    }
-}
-
 /* Execute the line P as a command.
    Pass FROM_TTY as second argument to the defining function.  */
 
@@ -534,24 +497,6 @@ execute_command (char *p, int from_tty)
 	  warned = 1;
 	}
     }
-
-  /* Set things up for this function to be compete later, once the
-     execution has completed, if we are doing an execution command,
-     otherwise, just go ahead and finish. */
-  if (target_can_async_p () && target_executing)
-    {
-      arg1 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg2 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg1->next = arg2;
-      arg2->next = NULL;     
-      arg1->data.longint = time_at_cmd_start;
-#ifdef HAVE_SBRK
-      arg2->data.longint = space_at_cmd_start;
-#endif
-      add_continuation (command_line_handler_continuation, arg1);
-    }
 }
 
 /* Read commands from `instream' and execute them
-- 
1.5.3.5

Re: [RFA] Remove command_line_handler_continuation.

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Thu, 24 Apr 2008 19:33:40 +0300
> 
> 
> With my previous patch to move all bpstat_do_actions calls into
> event handler, all that command_line_handler_continuation does
> is to print timings, if those are enabled. However, those timings
> don't make much sense. Timings are good for debugging GDB performance,
> and in case of command_line_handler_continuation, the times will report
> the time when the application run.
> 
> So, this patch removes that continuation. OK?

Doesn't this require some change to the manual, where these timings
are described?

Re: [RFA] Remove command_line_handler_continuation.

[Vladimir Prus]

Eli Zaretskii wrote:

>> From: Vladimir Prus <vladimir@codesourcery.com>
>> Date: Thu, 24 Apr 2008 19:33:40 +0300
>> 
>> 
>> With my previous patch to move all bpstat_do_actions calls into
>> event handler, all that command_line_handler_continuation does
>> is to print timings, if those are enabled. However, those timings
>> don't make much sense. Timings are good for debugging GDB performance,
>> and in case of command_line_handler_continuation, the times will report
>> the time when the application run.
>> 
>> So, this patch removes that continuation. OK?
> 
> Doesn't this require some change to the manual, where these timings
> are described?

I can only find this bit in the manual:

   -statistics 
    This option causes GDB to print statistics about time and memory usage after it completes each  
    command and returns to the prompt.

Is this what should be changed, or there's something else I've missed?

- Volodya

Re: [RFA] Remove command_line_handler_continuation.

[Eli Zaretskii]

> From:  Vladimir Prus <vladimir@codesourcery.com>
> Date:  Sat, 26 Apr 2008 14:55:15 +0400
> 
> > Doesn't this require some change to the manual, where these timings
> > are described?
> 
> I can only find this bit in the manual:
> 
>    -statistics 
>     This option causes GDB to print statistics about time and memory usage after it completes each  
>     command and returns to the prompt.
> 
> Is this what should be changed, or there's something else I've missed?

I thought "maint time" was also relevant.  Is it?

Re: [RFA] Remove command_line_handler_continuation.

[Vladimir Prus]

On Saturday 26 April 2008 16:14:40 Eli Zaretskii wrote:
> > From:  Vladimir Prus <vladimir@codesourcery.com>
> > Date:  Sat, 26 Apr 2008 14:55:15 +0400
> > 
> > > Doesn't this require some change to the manual, where these timings
> > > are described?
> > 
> > I can only find this bit in the manual:
> > 
> >    -statistics 
> >     This option causes GDB to print statistics about time and memory usage after it completes each  
> >     command and returns to the prompt.
> > 
> > Is this what should be changed, or there's something else I've missed?
> 
> I thought "maint time" was also relevant.  Is it?

It is, but is likewise very briefly documented, so it's not clear to me that saying that such
statistics is not output after run command makes sense.

- Volodya

Re: [RFA] Remove command_line_handler_continuation.

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat, 26 Apr 2008 17:40:58 +0400
> Cc: gdb-patches@sourceware.org
> 
> it's not clear to me that saying that such statistics is not output
> after run command makes sense.

I think it does.  "maint time" is for GDB hackers, and to a hacker
such a detail might be important.

Re: [RFA] Remove command_line_handler_continuation.

[Daniel Jacobowitz]

On Thu, Apr 24, 2008 at 07:33:40PM +0300, Vladimir Prus wrote:
> 
> With my previous patch to move all bpstat_do_actions calls into
> event handler, all that command_line_handler_continuation does
> is to print timings, if those are enabled. However, those timings
> don't make much sense. Timings are good for debugging GDB performance,
> and in case of command_line_handler_continuation, the times will report
> the time when the application run.
> 
> So, this patch removes that continuation. OK?

OK once you and Eli agree about documenting the timings.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] Remove command_line_handler_continuation.

[Vladimir Prus]

On Friday 02 May 2008 19:03:14 Daniel Jacobowitz wrote:
> On Thu, Apr 24, 2008 at 07:33:40PM +0300, Vladimir Prus wrote:
> > 
> > With my previous patch to move all bpstat_do_actions calls into
> > event handler, all that command_line_handler_continuation does
> > is to print timings, if those are enabled. However, those timings
> > don't make much sense. Timings are good for debugging GDB performance,
> > and in case of command_line_handler_continuation, the times will report
> > the time when the application run.
> > 
> > So, this patch removes that continuation. OK?
> 
> OK once you and Eli agree about documenting the timings.

Here's a revision with doc clarification. Eli, does it look sufficient?

- Volodya

Re: [RFA] Remove command_line_handler_continuation.

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

On Saturday 03 May 2008 19:58:36 Vladimir Prus wrote:
> On Friday 02 May 2008 19:03:14 Daniel Jacobowitz wrote:
> > On Thu, Apr 24, 2008 at 07:33:40PM +0300, Vladimir Prus wrote:
> > > 
> > > With my previous patch to move all bpstat_do_actions calls into
> > > event handler, all that command_line_handler_continuation does
> > > is to print timings, if those are enabled. However, those timings
> > > don't make much sense. Timings are good for debugging GDB performance,
> > > and in case of command_line_handler_continuation, the times will report
> > > the time when the application run.
> > > 
> > > So, this patch removes that continuation. OK?
> > 
> > OK once you and Eli agree about documenting the timings.
> 
> Here's a revision with doc clarification. Eli, does it look sufficient?

And here's the patch, for real.

- Volodya

[-- Attachment #2: 0001-Remove-command_line_handler_continuation.patch --]
[-- Type: text/x-diff, Size: 3731 bytes --]

From a0ac4ea192e90df539fa167c1310f3f1a8184c02 Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Thu, 13 Mar 2008 12:18:12 +0300
Subject: [RFA] Remove command_line_handler_continuation.
To: gdb-patches@sources.redhat.com
X-KMail-Transport: CodeSourcery
X-KMail-Identity: 901867920

	[gdb]
	* top.c (command_line_handler_continuation): Remove.
	(execute_command): Do not install the above.

	[gdb/doc]
	* gdb.texinfo (Maintenance Commands): Clarify that "maint time"
	will not report the time of commands that run the target.
---
 gdb/doc/gdb.texinfo |    4 +++
 gdb/top.c           |   55 ---------------------------------------------------
 2 files changed, 4 insertions(+), 55 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 636a84b..45a7412 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23437,6 +23437,10 @@ switch (@pxref{Mode Options}).
 Control whether to display the execution time for each command.  If
 set to a nonzero value, @value{GDBN} will display how much time it
 took to execute each command, following the command's own output.
+The time is not printed for the commands that run the target, since
+there's no mechanism currently to compute how much time was spend
+by @value{GDBN} and how much time was spend by the program been debugged.
+it's not possibly currently 
 This can also be requested by invoking @value{GDBN} with the
 @option{--statistics} command-line switch (@pxref{Mode Options}).
 
diff --git a/gdb/top.c b/gdb/top.c
index feefcf0..e0293df 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -364,43 +364,6 @@ do_chdir_cleanup (void *old_dir)
 }
 #endif
 
-/* Do any commands attached to breakpoint we stopped at. Only if we
-   are always running synchronously. Or if we have just executed a
-   command that doesn't start the target.  */
-static void
-command_line_handler_continuation (struct continuation_arg *arg, int error)
-{
-  extern int display_time;
-  extern int display_space;
-
-  long time_at_cmd_start  = arg->data.longint;
-  long space_at_cmd_start = arg->next->data.longint;
-
-  if (error)
-    return;
-
-  if (display_time)
-    {
-      long cmd_time = get_run_time () - time_at_cmd_start;
-
-      printf_unfiltered (_("Command execution time: %ld.%06ld\n"),
-			 cmd_time / 1000000, cmd_time % 1000000);
-    }
-  if (display_space)
-    {
-#ifdef HAVE_SBRK
-      char *lim = (char *) sbrk (0);
-      long space_now = lim - lim_at_start;
-      long space_diff = space_now - space_at_cmd_start;
-
-      printf_unfiltered (_("Space used: %ld (%c%ld for this command)\n"),
-			 space_now,
-			 (space_diff >= 0 ? '+' : '-'),
-			 space_diff);
-#endif
-    }
-}
-
 /* Execute the line P as a command.
    Pass FROM_TTY as second argument to the defining function.  */
 
@@ -534,24 +497,6 @@ execute_command (char *p, int from_tty)
 	  warned = 1;
 	}
     }
-
-  /* Set things up for this function to be compete later, once the
-     execution has completed, if we are doing an execution command,
-     otherwise, just go ahead and finish. */
-  if (target_can_async_p () && target_executing)
-    {
-      arg1 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg2 =
-	(struct continuation_arg *) xmalloc (sizeof (struct continuation_arg));
-      arg1->next = arg2;
-      arg2->next = NULL;     
-      arg1->data.longint = time_at_cmd_start;
-#ifdef HAVE_SBRK
-      arg2->data.longint = space_at_cmd_start;
-#endif
-      add_continuation (command_line_handler_continuation, arg1);
-    }
 }
 
 /* Read commands from `instream' and execute them
-- 
1.5.3.5

Re: [RFA] Remove command_line_handler_continuation.

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat, 3 May 2008 20:15:10 +0400
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  gdb-patches@sources.redhat.com
> 
> > > OK once you and Eli agree about documenting the timings.
> > 
> > Here's a revision with doc clarification. Eli, does it look sufficient?
> 
> And here's the patch, for real.

Yes, this is fine.  Thanks.

Re: [RFA] Remove command_line_handler_continuation.

[Daniel Jacobowitz]

On Sat, May 03, 2008 at 08:15:10PM +0400, Vladimir Prus wrote:
> +The time is not printed for the commands that run the target, since
> +there's no mechanism currently to compute how much time was spend

spend -> spent

> +by @value{GDBN} and how much time was spend by the program been debugged.

here too.

> +it's not possibly currently 

stray line.

>  This can also be requested by invoking @value{GDBN} with the
>  @option{--statistics} command-line switch (@pxref{Mode Options}).

-- 
Daniel Jacobowitz
CodeSourcery