[doc RFA] New commands: mt set per-command on|off

[doc RFA] New commands: mt set per-command on|off

[Doug Evans]

Hi.
This patch adds a new option to display some simple symtab stats
akin to how "mt time|space 1" work.  For consistency with the
rest of gdb I named it:

maint set per-command symtab on|off
maint show per-command symtab

and then added new commands:

maint set per-command space|time on|off
maint show per-command space|time

I kept "maint space|time value" since some may be used to typing that.

I also made "maint set per-command on|off"
and "maint show per-command" DTRT.

I did not add symtab stats to the output of --statistics,
it's not clear to me that is useful enough.

This needs a doc RFA.

2013-03-15  Doug Evans  <dje@google.com>

	New commands "mt set per-command {space,time,symtab} {on,off}".
	* NEWS: Add entry.
	* event-top.c: #include "maint.h".
	* main.c: #include "maint.h".
	* maint.c: #include <sys/time.h>, <time.h>, block.h, top.h,
	timeval-utils.h, maint.h, cli/cli-setshow.h.
	(per_command_time, per_command_space): New static globals.
	(per_command_symtab): New static global.
	(per_command_setlist, per_command_showlist): New static globals.
	(struct cmd_stats): Move here from utils.c.
	(set_per_command_time): Renamed from set_display_time in utils.c
	and moved here.  All callers updated.
	(set_per_command_space): Renamed from set_display_space in utils.c
	and moved here.  All callers updated.
	(count_symtabs_and_blocks): New function.
	(report_command_stats): Moved here from utils.c.  Add support for
	printing symtab stats.  Only print data if enabled before command
	executed.
	(make_command_stats_cleanup): Ditto.
	(sert_per_command_cmd, show_per_command_cmd): New functions.
	(_initialize_maint_cmds): Add new commands
	mt set per-command {space,time,symtab} {on,off}.
	* maint.h: New file.
	* top.c: #include "maint.h".
	* utils.c (reset_prompt_for_continue_wait_time): New function.
	(get_prompt_for_continue_wait_time): New function.
	* utils.h (reset_prompt_for_continue_wait_time): Declare
	(get_prompt_for_continue_wait_time): Declare.
	(make_command_stats_cleanup): Moved to maint.h.
	(set_display_time, set_display_space): Moved to maint.h and renamed
	to set_per_command_time, set_per_command_space.
	* cli/cli-setshow.c (parse_cli_boolean_value): Renamed from
	parse_binary_operation and made non-static.  Don't call error,
	just return an error marker.  All callers updated.
	* cli/cli-setshow.h (parse_cli_boolean_value): Declare.

	doc/
	* gdb.texinfo (Maintenance Commands): Add docs for
	"mt set per-command {space,time,symtab} {on,off}".

	testsuite/
	* gdb.base/maint.exp: Update tests for per-command stats.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.575
diff -u -p -r1.575 NEWS
--- NEWS	14 Mar 2013 09:02:27 -0000	1.575
+++ NEWS	15 Mar 2013 22:03:31 -0000
@@ -3,6 +3,12 @@
 
 *** Changes since GDB 7.6
 
+* New commands:
+maint set|show per-command
+maint set|show per-command space
+maint set|show per-command time
+maint set|show per-command symtab
+
 * The command 'tsave' can now support new option '-ctf' to save trace
   buffer in Common Trace Format.
 
Index: event-top.c
===================================================================
RCS file: /cvs/src/src/gdb/event-top.c,v
retrieving revision 1.92
diff -u -p -r1.92 event-top.c
--- event-top.c	31 Jan 2013 18:37:37 -0000	1.92
+++ event-top.c	15 Mar 2013 21:44:24 -0000
@@ -36,6 +36,7 @@
 #include "continuations.h"
 #include "gdbcmd.h"		/* for dont_repeat() */
 #include "annotate.h"
+#include "maint.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
Index: main.c
===================================================================
RCS file: /cvs/src/src/gdb/main.c,v
retrieving revision 1.120
diff -u -p -r1.120 main.c
--- main.c	25 Jan 2013 00:46:19 -0000	1.120
+++ main.c	15 Mar 2013 21:44:25 -0000
@@ -42,6 +42,7 @@
 #include "python/python.h"
 #include "objfiles.h"
 #include "auto-load.h"
+#include "maint.h"
 
 /* The selected interpreter.  This will be used as a set command
    variable, so it should always be malloc'ed - since
@@ -538,8 +539,8 @@ captured_main (void *data)
 	    break;
 	  case OPT_STATISTICS:
 	    /* Enable the display of both time and space usage.  */
-	    set_display_time (1);
-	    set_display_space (1);
+	    set_per_command_time (1);
+	    set_per_command_space (1);
 	    break;
 	  case OPT_TUI:
 	    /* --tui is equivalent to -i=tui.  */
Index: maint.c
===================================================================
RCS file: /cvs/src/src/gdb/maint.c,v
retrieving revision 1.91
diff -u -p -r1.91 maint.c
--- maint.c	7 Mar 2013 21:57:29 -0000	1.91
+++ maint.c	15 Mar 2013 21:44:26 -0000
@@ -24,9 +24,12 @@
 #include "arch-utils.h"
 #include <ctype.h>
 #include <signal.h>
+#include <sys/time.h>
+#include <time.h>
 #include "command.h"
 #include "gdbcmd.h"
 #include "symtab.h"
+#include "block.h"
 #include "gdbtypes.h"
 #include "demangle.h"
 #include "gdbcore.h"
@@ -36,9 +39,13 @@
 #include "objfiles.h"
 #include "value.h"
 #include "gdb_assert.h"
+#include "top.h"
+#include "timeval-utils.h"
+#include "maint.h"
 
 #include "cli/cli-decode.h"
 #include "cli/cli-utils.h"
+#include "cli/cli-setshow.h"
 
 extern void _initialize_maint_cmds (void);
 
@@ -164,7 +171,7 @@ maintenance_time_display (char *args, in
   if (args == NULL || *args == '\0')
     printf_unfiltered (_("\"maintenance time\" takes a numeric argument.\n"));
   else
-    set_display_time (strtol (args, NULL, 10));
+    set_per_command_time (strtol (args, NULL, 10));
 }
 
 static void
@@ -173,7 +180,7 @@ maintenance_space_display (char *args, i
   if (args == NULL || *args == '\0')
     printf_unfiltered ("\"maintenance space\" takes a numeric argument.\n");
   else
-    set_display_space (strtol (args, NULL, 10));
+    set_per_command_space (strtol (args, NULL, 10));
 }
 
 /* The "maintenance info" command is defined as a prefix, with
@@ -725,7 +732,243 @@ maintenance_set_profile_cmd (char *args,
   error (_("Profiling support is not available on this system."));
 }
 #endif
+\f
+/* If nonzero, display time usage both at startup and for each command.  */
 
+static int per_command_time;
+
+/* If nonzero, display space usage both at startup and for each command.  */
+
+static int per_command_space;
+
+/* If nonzero, display basic symtab stats for each command.  */
+
+static int per_command_symtab;
+
+/* mt per-command commands.  */
+
+static struct cmd_list_element *per_command_setlist;
+static struct cmd_list_element *per_command_showlist;
+
+/* Records a run time and space usage to be used as a base for
+   reporting elapsed time or change in space.  */
+
+struct cmd_stats 
+{
+  /* Zero if the saved time is from the beginning of GDB execution.
+     One if from the beginning of an individual command execution.  */
+  int msg_type;
+  /* Track whether the stat was enabled at the start of the command
+     so that we can avoid printing anything if it gets turned on by
+     the current command.  */
+  int time_enabled : 1;
+  int space_enabled : 1;
+  int symtab_enabled : 1;
+  long start_cpu_time;
+  struct timeval start_wall_time;
+  long start_space;
+  /* Total number of symtabs (over all objfiles).  */
+  int start_nr_symtabs;
+  /* Of those, a count of just the primary ones.  */
+  int start_nr_primary_symtabs;
+  /* Total number of blocks.  */
+  int start_nr_blocks;
+};
+
+/* Set whether to display time statistics to NEW_VALUE
+   (non-zero means true).  */
+
+void
+set_per_command_time (int new_value)
+{
+  per_command_time = new_value;
+}
+
+/* Set whether to display space statistics to NEW_VALUE
+   (non-zero means true).  */
+
+void
+set_per_command_space (int new_value)
+{
+  per_command_space = new_value;
+}
+
+/* Count the number of symtabs and blocks.  */
+
+static void
+count_symtabs_and_blocks (int *nr_symtabs_ptr, int *nr_primary_symtabs_ptr,
+			  int *nr_blocks_ptr)
+{
+  struct objfile *o;
+  struct symtab *s;
+  int nr_symtabs = 0;
+  int nr_primary_symtabs = 0;
+  int nr_blocks = 0;
+
+  ALL_SYMTABS (o, s)
+    {
+      ++nr_symtabs;
+      if (s->primary)
+	{
+	  ++nr_primary_symtabs;
+	  nr_blocks += BLOCKVECTOR_NBLOCKS (BLOCKVECTOR (s));
+	}
+    }
+
+  *nr_symtabs_ptr = nr_symtabs;
+  *nr_primary_symtabs_ptr = nr_primary_symtabs;
+  *nr_blocks_ptr = nr_blocks;
+}
+
+/* As indicated by display_time and display_space, report GDB's elapsed time
+   and space usage from the base time and space provided in ARG, which
+   must be a pointer to a struct cmd_stat.  This function is intended
+   to be called as a cleanup.  */
+
+static void
+report_command_stats (void *arg)
+{
+  struct cmd_stats *start_stats = (struct cmd_stats *) arg;
+  int msg_type = start_stats->msg_type;
+
+  if (start_stats->time_enabled)
+    {
+      long cmd_time = get_run_time () - start_stats->start_cpu_time;
+      struct timeval now_wall_time, delta_wall_time, wait_time;
+
+      gettimeofday (&now_wall_time, NULL);
+      timeval_sub (&delta_wall_time,
+		   &now_wall_time, &start_stats->start_wall_time);
+
+      /* Subtract time spend in prompt_for_continue from walltime.  */
+      wait_time = get_prompt_for_continue_wait_time ();
+      timeval_sub (&delta_wall_time, &delta_wall_time, &wait_time);
+
+      printf_unfiltered (msg_type == 0
+			 ? _("Startup time: %ld.%06ld (cpu), %ld.%06ld (wall)\n")
+			 : _("Command execution time: %ld.%06ld (cpu), %ld.%06ld (wall)\n"),
+			 cmd_time / 1000000, cmd_time % 1000000,
+			 (long) delta_wall_time.tv_sec,
+			 (long) delta_wall_time.tv_usec);
+    }
+
+  if (start_stats->space_enabled)
+    {
+#ifdef HAVE_SBRK
+      char *lim = (char *) sbrk (0);
+
+      long space_now = lim - lim_at_start;
+      long space_diff = space_now - start_stats->start_space;
+
+      printf_unfiltered (msg_type == 0
+			 ? _("Space used: %ld (%s%ld during startup)\n")
+			 : _("Space used: %ld (%s%ld for this command)\n"),
+			 space_now,
+			 (space_diff >= 0 ? "+" : ""),
+			 space_diff);
+#endif
+    }
+
+  if (start_stats->symtab_enabled)
+    {
+      int nr_symtabs, nr_primary_symtabs, nr_blocks;
+
+      count_symtabs_and_blocks (&nr_symtabs, &nr_primary_symtabs, &nr_blocks);
+      printf_unfiltered (_("#symtabs: %d (+%d),"
+			   " #primary symtabs: %d (+%d),"
+			   " #blocks: %d (+%d)\n"),
+			 nr_symtabs,
+			 nr_symtabs - start_stats->start_nr_symtabs,
+			 nr_primary_symtabs,
+			 nr_primary_symtabs - start_stats->start_nr_primary_symtabs,
+			 nr_blocks,
+			 nr_blocks - start_stats->start_nr_blocks);
+    }
+}
+
+/* Create a cleanup that reports time and space used since its creation.
+   MSG_TYPE is zero for gdb startup, otherwise it is one(1) to report
+   data for individual commands.  */
+
+struct cleanup *
+make_command_stats_cleanup (int msg_type)
+{
+  struct cmd_stats *new_stat;
+
+  /* Early exit if we're not reporting any stats.  */
+  if (!per_command_time
+      && !per_command_space
+      && !per_command_symtab)
+    return make_cleanup (null_cleanup, 0);
+
+  new_stat = XZALLOC (struct cmd_stats);
+
+  new_stat->msg_type = msg_type;
+
+  if (per_command_space)
+    {
+#ifdef HAVE_SBRK
+      char *lim = (char *) sbrk (0);
+      new_stat->start_space = lim - lim_at_start;
+      new_stat->space_enabled = 1;
+#endif
+    }
+
+  if (per_command_time)
+    {
+      new_stat->start_cpu_time = get_run_time ();
+      gettimeofday (&new_stat->start_wall_time, NULL);
+      new_stat->time_enabled = 1;
+    }
+
+  if (per_command_symtab)
+    {
+      int nr_symtabs, nr_primary_symtabs, nr_blocks;
+
+      count_symtabs_and_blocks (&nr_symtabs, &nr_primary_symtabs, &nr_blocks);
+      new_stat->start_nr_symtabs = nr_symtabs;
+      new_stat->start_nr_primary_symtabs = nr_primary_symtabs;
+      new_stat->start_nr_blocks = nr_blocks;
+      new_stat->symtab_enabled = 1;
+    }
+
+  /* Initalize timer to keep track of how long we waited for the user.  */
+  reset_prompt_for_continue_wait_time ();
+
+  return make_cleanup_dtor (report_command_stats, new_stat, xfree);
+}
+
+/* Handle unknown "mt set per-command" arguments.
+   In this case have "mt set per-command on|off" affect every setting.  */
+
+static void
+set_per_command_cmd (char *args, int from_tty)
+{
+  struct cmd_list_element *list;
+  size_t length;
+  int val;
+
+  val = parse_cli_boolean_value (args);
+  if (val < 0)
+    error (_("Bad value for 'mt set per-command no'."));
+
+  for (list = per_command_setlist; list != NULL; list = list->next)
+    if (list->var_type == var_boolean)
+      {
+	gdb_assert (list->type == set_cmd);
+	do_set_command (args, from_tty, list);
+      }
+}
+
+/* Command "show per-command" displays summary of all the current
+   "show per-command " settings.  */
+
+static void
+show_per_command_cmd (char *args, int from_tty)
+{
+  cmd_show_list (per_command_showlist, from_tty, "");
+}
+\f
 void
 _initialize_maint_cmds (void)
 {
@@ -802,12 +1045,56 @@ Call internal GDB demangler routine to d
 and prints the result."),
 	   &maintenancelist);
 
+  add_prefix_cmd ("per-command", class_maintenance, set_per_command_cmd, _("\
+Per-command statistics settings."),
+		    &per_command_setlist, "set per-command ",
+		    1/*allow-unknown*/, &maintenance_set_cmdlist);
+
+  add_prefix_cmd ("per-command", class_maintenance, show_per_command_cmd, _("\
+Show per-command statistics settings."),
+		    &per_command_showlist, "show per-command ",
+		    0/*allow-unknown*/, &maintenance_show_cmdlist);
+
+  add_setshow_boolean_cmd ("time", class_maintenance,
+			   &per_command_time, _("\
+Set whether to display per-command execution time."), _("\
+Show whether to display per-command execution time."),
+			   _("\
+If enabled, the execution time for each command will be\n\
+displayed following the command's output."),
+			   NULL, NULL,
+			   &per_command_setlist, &per_command_showlist);
+
+  add_setshow_boolean_cmd ("space", class_maintenance,
+			   &per_command_space, _("\
+Set whether to display per-command space usage."), _("\
+Show whether to display per-command space usage."),
+			   _("\
+If enabled, the space usage for each command will be\n\
+displayed following the command's output."),
+			   NULL, NULL,
+			   &per_command_setlist, &per_command_showlist);
+
+  add_setshow_boolean_cmd ("symtab", class_maintenance,
+			   &per_command_symtab, _("\
+Set whether to display per-command symtab stats."), _("\
+Show whether to display per-command symtab stats."),
+			   _("\
+If enabled, the basic symtab stats for each command will be\n\
+displayed following the command's output."),
+			   NULL, NULL,
+			   &per_command_setlist, &per_command_showlist);
+
+  /* This is equivalent to "mt set per-command time on".
+     Kept because some people are used to typing "mt time 1".  */
   add_cmd ("time", class_maintenance, maintenance_time_display, _("\
 Set the display of time usage.\n\
 If nonzero, will cause the execution time for each command to be\n\
 displayed, following the command's output."),
 	   &maintenancelist);
 
+  /* This is equivalent to "mt set per-command space on".
+     Kept because some people are used to typing "mt space 1".  */
   add_cmd ("space", class_maintenance, maintenance_space_display, _("\
 Set the display of space usage.\n\
 If nonzero, will cause the execution space for each command to be\n\
Index: maint.h
===================================================================
RCS file: maint.h
diff -N maint.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ maint.h	15 Mar 2013 21:44:26 -0000
@@ -0,0 +1,31 @@
+/* Support for GDB maintenance commands.
+   Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef MAINT_H
+#define MAINT_H
+
+extern void set_per_command_time (int);
+
+extern void set_per_command_space (int);
+
+/* Note: There's no set_per_command_symtab on purpose.
+   Symtab stats aren't yet as useful for --statistics output.  */
+
+extern struct cleanup *make_command_stats_cleanup (int);
+
+#endif /* MAINT_H */
Index: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.226
diff -u -p -r1.226 top.c
--- top.c	25 Jan 2013 14:17:10 -0000	1.226
+++ top.c	15 Mar 2013 21:44:26 -0000
@@ -47,6 +47,7 @@
 #include "python/python.h"
 #include "interps.h"
 #include "observer.h"
+#include "maint.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.294
diff -u -p -r1.294 utils.c
--- utils.c	7 Mar 2013 19:24:32 -0000	1.294
+++ utils.c	15 Mar 2013 21:44:26 -0000
@@ -544,117 +544,6 @@ free_current_contents (void *ptr)
       *location = NULL;
     }
 }
-
-/* If nonzero, display time usage both at startup and for each command.  */
-
-static int display_time;
-
-/* If nonzero, display space usage both at startup and for each command.  */
-
-static int display_space;
-
-/* Records a run time and space usage to be used as a base for
-   reporting elapsed time or change in space.  In addition,
-   the msg_type field indicates whether the saved time is from the
-   beginning of GDB execution (0) or the beginning of an individual 
-   command execution (1).  */
-struct cmd_stats 
-{
-  int msg_type;
-  long start_cpu_time;
-  struct timeval start_wall_time;
-  long start_space;
-};
-
-/* Set whether to display time statistics to NEW_VALUE (non-zero 
-   means true).  */
-void
-set_display_time (int new_value)
-{
-  display_time = new_value;
-}
-
-/* Set whether to display space statistics to NEW_VALUE (non-zero
-   means true).  */
-void
-set_display_space (int new_value)
-{
-  display_space = new_value;
-}
-
-/* As indicated by display_time and display_space, report GDB's elapsed time
-   and space usage from the base time and space provided in ARG, which
-   must be a pointer to a struct cmd_stat.  This function is intended
-   to be called as a cleanup.  */
-static void
-report_command_stats (void *arg)
-{
-  struct cmd_stats *start_stats = (struct cmd_stats *) arg;
-  int msg_type = start_stats->msg_type;
-
-  if (display_time)
-    {
-      long cmd_time = get_run_time () - start_stats->start_cpu_time;
-      struct timeval now_wall_time, delta_wall_time;
-
-      gettimeofday (&now_wall_time, NULL);
-      timeval_sub (&delta_wall_time,
-		   &now_wall_time, &start_stats->start_wall_time);
-
-      /* Subtract time spend in prompt_for_continue from walltime.  */
-      timeval_sub (&delta_wall_time,
-                   &delta_wall_time, &prompt_for_continue_wait_time);
-
-      printf_unfiltered (msg_type == 0
-			 ? _("Startup time: %ld.%06ld (cpu), %ld.%06ld (wall)\n")
-			 : _("Command execution time: %ld.%06ld (cpu), %ld.%06ld (wall)\n"),
-			 cmd_time / 1000000, cmd_time % 1000000,
-			 (long) delta_wall_time.tv_sec,
-			 (long) delta_wall_time.tv_usec);
-    }
-
-  if (display_space)
-    {
-#ifdef HAVE_SBRK
-      char *lim = (char *) sbrk (0);
-
-      long space_now = lim - lim_at_start;
-      long space_diff = space_now - start_stats->start_space;
-
-      printf_unfiltered (msg_type == 0
-			 ? _("Space used: %ld (%s%ld during startup)\n")
-			 : _("Space used: %ld (%s%ld for this command)\n"),
-			 space_now,
-			 (space_diff >= 0 ? "+" : ""),
-			 space_diff);
-#endif
-    }
-}
-
-/* Create a cleanup that reports time and space used since its
-   creation.  Precise messages depend on MSG_TYPE:
-      0:  Initial time/space
-      1:  Individual command time/space.  */
-struct cleanup *
-make_command_stats_cleanup (int msg_type)
-{
-  static const struct timeval zero_timeval = { 0 };
-  struct cmd_stats *new_stat = XMALLOC (struct cmd_stats);
-  
-#ifdef HAVE_SBRK
-  char *lim = (char *) sbrk (0);
-  new_stat->start_space = lim - lim_at_start;
-#endif
-
-  new_stat->msg_type = msg_type;
-  new_stat->start_cpu_time = get_run_time ();
-  gettimeofday (&new_stat->start_wall_time, NULL);
-
-  /* Initalize timer to keep track of how long we waited for the user.  */
-  prompt_for_continue_wait_time = zero_timeval;
-
-  return make_cleanup_dtor (report_command_stats, new_stat, xfree);
-}
 \f
 
 
@@ -1924,6 +1813,24 @@ prompt_for_continue (void)
   dont_repeat ();		/* Forget prev cmd -- CR won't repeat it.  */
 }
 
+/* Initalize timer to keep track of how long we waited for the user.  */
+
+void
+reset_prompt_for_continue_wait_time (void)
+{
+  static const struct timeval zero_timeval = { 0 };
+
+  prompt_for_continue_wait_time = zero_timeval;
+}
+
+/* Fetch the cumulative time spent in prompt_for_continue.  */
+
+struct timeval
+get_prompt_for_continue_wait_time (void)
+{
+  return prompt_for_continue_wait_time;
+}
+
 /* Reinitialize filter; ie. tell it to reset to original values.  */
 
 void
Index: utils.h
===================================================================
RCS file: /cvs/src/src/gdb/utils.h,v
retrieving revision 1.5
diff -u -p -r1.5 utils.h
--- utils.h	3 Feb 2013 15:54:17 -0000	1.5
+++ utils.h	15 Mar 2013 21:44:26 -0000
@@ -53,6 +53,11 @@ extern char *safe_strerror (int);
    bfd_check_format_matches, and will be freed.  */
 
 extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
+
+/* Reset the prompt_for_continue clock.  */
+void reset_prompt_for_continue_wait_time (void);
+/* Return the time spent in prompt_for_continue.  */
+struct timeval get_prompt_for_continue_wait_time (void);
 \f
 /* Parsing utilites.  */
 
@@ -110,8 +115,6 @@ extern struct cleanup *make_cleanup_htab
 
 extern void free_current_contents (void *);
 
-extern struct cleanup *make_command_stats_cleanup (int);
-
 extern void init_page_info (void);
 
 extern struct cleanup *make_cleanup_restore_page_info (void);
@@ -136,10 +139,6 @@ char *ldirname (const char *filename);
 
 struct ui_file;
 
-extern void set_display_time (int);
-
-extern void set_display_space (int);
-
 extern int query (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 extern int nquery (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
 extern int yquery (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
Index: cli/cli-setshow.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-setshow.c,v
retrieving revision 1.55
diff -u -p -r1.55 cli-setshow.c
--- cli/cli-setshow.c	12 Feb 2013 19:03:55 -0000	1.55
+++ cli/cli-setshow.c	15 Mar 2013 21:44:26 -0000
@@ -29,10 +29,6 @@
 #include "cli/cli-cmds.h"
 #include "cli/cli-setshow.h"
 
-/* Prototypes for local functions.  */
-
-static int parse_binary_operation (char *);
-
 /* Return true if the change of command parameter should be notified.  */
 
 static int
@@ -76,8 +72,10 @@ parse_auto_binary_operation (const char 
   return AUTO_BOOLEAN_AUTO; /* Pacify GCC.  */
 }
 
-static int
-parse_binary_operation (char *arg)
+/* See cli-setshow.h.  */
+
+int
+parse_cli_boolean_value (char *arg)
 {
   int length;
 
@@ -100,10 +98,7 @@ parse_binary_operation (char *arg)
 	   || strncmp (arg, "disable", length) == 0)
     return 0;
   else
-    {
-      error (_("\"on\" or \"off\" expected."));
-      return 0;
-    }
+    return -1;
 }
 \f
 void
@@ -248,8 +243,10 @@ do_set_command (char *arg, int from_tty,
       break;
     case var_boolean:
       {
-	int val = parse_binary_operation (arg);
+	int val = parse_cli_boolean_value (arg);
 
+	if (val < 0)
+	  error (_("\"on\" or \"off\" expected."));
 	if (val != *(int *) c->var)
 	  {
 	    *(int *) c->var = val;
Index: cli/cli-setshow.h
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-setshow.h,v
retrieving revision 1.16
diff -u -p -r1.16 cli-setshow.h
--- cli/cli-setshow.h	12 Feb 2013 19:03:55 -0000	1.16
+++ cli/cli-setshow.h	15 Mar 2013 21:44:26 -0000
@@ -19,15 +19,15 @@
 
 struct cmd_list_element;
 
-/* Exported to cli/cli-cmds.c and gdb/top.c */
+/* Parse ARG, an option to a boolean variable.
+   Returns 1 for true, 0 for false, and -1 if invalid.  */
+extern int parse_cli_boolean_value (char *arg);
 
 extern void do_set_command (char *arg, int from_tty,
 			    struct cmd_list_element *c);
 extern void do_show_command (char *arg, int from_tty,
 			     struct cmd_list_element *c);
 
-/* Exported to cli/cli-cmds.c and gdb/top.c, language.c and valprint.c */
-
 extern void cmd_show_list (struct cmd_list_element *list, int from_tty,
 			   char *prefix);
 
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.1060
diff -u -p -r1.1060 gdb.texinfo
--- doc/gdb.texinfo	11 Mar 2013 19:43:52 -0000	1.1060
+++ doc/gdb.texinfo	15 Mar 2013 21:44:28 -0000
@@ -35715,24 +35715,35 @@ Control whether to show all non zero are
 at thread local base, when using the @samp{info w32 thread-information-block}
 command.
 
-@kindex maint space
-@cindex memory used by commands
-@item maint space
-Control whether to display memory usage for each command.  If set to a
-nonzero value, @value{GDBN} will display how much memory each command
-took, following the command's own output.  This can also be requested
-by invoking @value{GDBN} with the @option{--statistics} command-line
-switch (@pxref{Mode Options}).
+@kindex maint set per-command
+@kindex maint show per-command
+@item maint set per-command
+@itemx maint show per-command
+@cindex resources used by commands
 
-@kindex maint time
-@cindex time of command execution
-@item maint time
-Control whether to display the execution time of @value{GDBN} for each command.
-If set to a nonzero value, @value{GDBN} will display how much time it
+@value{GDBN} can display the resources used by each command.
+This is useful in debugging performance problems.
+
+@table @code
+@kindex maint set per-command space
+@item maint set per-command space [on|off]
+@itemx maint show per-command space
+Enable or disable the printing of the memory usage for each command.
+If enabled, @value{GDBN} will display how much memory each command
+took, following the command's own output.
+This can also be requested by invoking @value{GDBN} with the
+@option{--statistics} command-line switch (@pxref{Mode Options}).
+
+@kindex maint set per-command time
+@item maint set per-command time [on|off]
+@itemx maint show per-command time
+Enable or disable the printing of the execution time of @value{GDBN}
+for each command.
+If enabled, @value{GDBN} will display how much time it
 took to execute each command, following the command's own output.
 Both CPU time and wallclock time are printed.
 Printing both is useful when trying to determine whether the cost is
-CPU or, e.g., disk/network, latency.
+CPU or, e.g., disk/network latency.
 Note that the CPU time printed is for @value{GDBN} only, it does not include
 the execution time of the inferior because there's no mechanism currently
 to compute how much time was spent by @value{GDBN} and how much time was
@@ -35740,6 +35751,32 @@ spent by the program been debugged.
 This can also be requested by invoking @value{GDBN} with the
 @option{--statistics} command-line switch (@pxref{Mode Options}).
 
+@kindex maint set per-command symtab
+@item maint set per-command symtab [on|off]
+@itemx maint show per-command symtab
+Enable or disable the printing of basic symbol table statistics
+for each command.
+If enabled, @value{GDBN} will display the following information:
+
+@table @bullet
+@item number of symbol tables
+@item number of primary symbol tables
+@item number of blocks in the blockvector
+@end table
+@end table
+
+@kindex maint space
+@cindex memory used by commands
+@item maint space @var{value}
+An alias for @code{maint set per-command space}.
+A non-zero value enables it, zero disables it.
+
+@kindex maint time
+@cindex time of command execution
+@item maint time @var{value}
+An alias for @code{maint set per-command time}.
+A non-zero value enables it, zero disables it.
+
 @kindex maint translate-address
 @item maint translate-address @r{[}@var{section}@r{]} @var{addr}
 Find the symbol stored at the location specified by the address
Index: testsuite/gdb.base/maint.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/maint.exp,v
retrieving revision 1.61
diff -u -p -r1.61 maint.exp
--- testsuite/gdb.base/maint.exp	1 Jan 2013 06:33:25 -0000	1.61
+++ testsuite/gdb.base/maint.exp	15 Mar 2013 22:24:11 -0000
@@ -21,10 +21,8 @@
 
 
 #maintenance check-symtabs -- Check consistency of psymtabs and symtabs
-#maintenance space -- Set the display of space usage
 #maintenance set -- Set GDB internal variables used by the GDB maintainer
 #maintenance show -- Show GDB internal variables used by the GDB maintainer
-#maintenance time -- Set the display of time usage
 #maintenance demangle -- Demangle a C++ mangled name
 #maintenance dump-me -- Get fatal error; make debugger dump its core
 #maintenance print -- Maintenance command for printing GDB internal state
@@ -126,22 +124,10 @@ gdb_expect  {
     timeout         { fail "(timeout) maint check-symtabs" }
 }
 
-gdb_test "maint space" \
-    "\"maintenance space\" takes a numeric argument\\."
+gdb_test_no_output "maint set per-command on"
 
-gdb_test "maint space 1" \
-    "Space used: $decimal \\(\\+$decimal for this command\\)"
-
-gdb_test "maint time" \
-    "\"maintenance time\" takes a numeric argument\\..*Space used: $decimal \\(\\+$decimal for this command\\)" 
-
-gdb_test "maint time 1" \
-    "Command execution time: $decimal.*Space used: $decimal \\(\\+$decimal for this command\\)"
-
-gdb_test "maint time 0" \
-    "Space used: $decimal \\(\\+$decimal for this command\\)"
-
-gdb_test_no_output "maint space 0"
+gdb_test "maint set per-command off" \
+    "Command execution time: \[0-9.\]+ \\(cpu\\), \[0-9.\]+ \\(wall\\)\[\r\n\]+Space used: $decimal \\(\\+$decimal for this command\\)\[\r\n\]+#symtabs: $decimal \\(\\+$decimal\\), #primary symtabs: $decimal \\(\\+$decimal\\) #blocks: $decimal \\(\\+$decimal\\)"
 
 gdb_test "maint demangle" \
     "\"maintenance demangle\" takes an argument to demangle\\."

Re: [doc RFA] New commands: mt set per-command on|off

[Eli Zaretskii]

> Date: Fri, 15 Mar 2013 15:29:09 -0700
> From: Doug Evans <dje@google.com>
> 
> This patch adds a new option to display some simple symtab stats
> akin to how "mt time|space 1" work.  For consistency with the
> rest of gdb I named it:
> 
> maint set per-command symtab on|off
> maint show per-command symtab
> 
> and then added new commands:
> 
> maint set per-command space|time on|off
> maint show per-command space|time

Thanks.

> --- NEWS	14 Mar 2013 09:02:27 -0000	1.575
> +++ NEWS	15 Mar 2013 22:03:31 -0000
> @@ -3,6 +3,12 @@
>  
>  *** Changes since GDB 7.6
>  
> +* New commands:
> +maint set|show per-command
> +maint set|show per-command space
> +maint set|show per-command time
> +maint set|show per-command symtab

Perhaps consider adding a sentence or two about what these do.

> +  add_setshow_boolean_cmd ("symtab", class_maintenance,
> +			   &per_command_symtab, _("\
> +Set whether to display per-command symtab stats."), _("\
> +Show whether to display per-command symtab stats."),
> +			   _("\
> +If enabled, the basic symtab stats for each command will be\n\
> +displayed following the command's output."),

Suggest to use "statistics" instead of "stats" here.

> +@kindex maint set per-command
> +@kindex maint show per-command
> +@item maint set per-command
> +@itemx maint show per-command
> +@cindex resources used by commands
>  
> -@kindex maint time
> -@cindex time of command execution
> -@item maint time
> -Control whether to display the execution time of @value{GDBN} for each command.
> -If set to a nonzero value, @value{GDBN} will display how much time it
> +@value{GDBN} can display the resources used by each command.
> +This is useful in debugging performance problems.
> +
> +@table @code
> +@kindex maint set per-command space

There's no need for this (and other similar) @kindex entry because
you already have "@kindex maint set/show per-command" above.  Such
indices tend to clutter and bloat the index node without adding useful
information, because they all point to approximately the same place,
even in the printed manual.

> +Enable or disable the printing of the memory usage for each command.
> +If enabled, @value{GDBN} will display how much memory each command
> +took, following the command's own output.

This should make it more explicit that the memory printed is the one
used by GDB, not by the inferior.

> +Enable or disable the printing of basic symbol table statistics
> +for each command.
> +If enabled, @value{GDBN} will display the following information:
> +
> +@table @bullet
> +@item number of symbol tables
> +@item number of primary symbol tables
> +@item number of blocks in the blockvector
> +@end table

Is this really the statistics _for_ the last command, or is this the
statistics _after_ the last command?  IOW, is this the delta due to
the last command or just the current snapshot of the symtab usage
statistics?  If the latter, then saying "for the last command" above
is misleading.

Re: [doc RFA] New commands: mt set per-command on|off

[Eli Zaretskii]

> From: Doug Evans <dje@google.com>
> Date: Mon, 18 Mar 2013 14:43:22 -0700
> Cc: gdb-patches@sourceware.org
> 
>  > > +  add_setshow_boolean_cmd ("symtab", class_maintenance,
>  > > +			   &per_command_symtab, _("\
>  > > +Set whether to display per-command symtab stats."), _("\
>  > > +Show whether to display per-command symtab stats."),
>  > > +			   _("\
>  > > +If enabled, the basic symtab stats for each command will be\n\
>  > > +displayed following the command's output."),
>  > 
>  > Suggest to use "statistics" instead of "stats" here.
> 
> Done.

The patch you sent still says "stats".

Other than that, I'm happy now.  Thanks.

Re: [doc RFA] New commands: mt set per-command on|off

[Doug Evans]

On Wed, Mar 20, 2013 at 10:20 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Doug Evans <dje@google.com>
>> Date: Mon, 18 Mar 2013 14:43:22 -0700
>> Cc: gdb-patches@sourceware.org
>>
>>  > > +  add_setshow_boolean_cmd ("symtab", class_maintenance,
>>  > > +                           &per_command_symtab, _("\
>>  > > +Set whether to display per-command symtab stats."), _("\
>>  > > +Show whether to display per-command symtab stats."),
>>  > > +                           _("\
>>  > > +If enabled, the basic symtab stats for each command will be\n\
>>  > > +displayed following the command's output."),
>>  >
>>  > Suggest to use "statistics" instead of "stats" here.
>>
>> Done.
>
> The patch you sent still says "stats".
>
> Other than that, I'm happy now.  Thanks.

Blech, got one missed the others.  Thanks.
Committed.

doc/ build regression with texinfo-5.1 [Re: [doc RFA] New commands: mt set per-command on|off]

[Jan Kratochvil]

On Mon, 18 Mar 2013 22:43:22 +0100, Doug Evans wrote:
> 	doc/
> 	* gdb.texinfo (Maintenance Commands): Add docs for
> 	"mt set per-command {space,time,symtab} {on,off}".

texinfo-5.1-1.fc20.x86_64

1666412af       (Doug Evans     2013-03-21 17:37:29 +0000       35648)@table @bullet

commit 1666412afbfb0047bf0fb2f7e5789494eeed44f8
Author: Doug Evans <dje@google.com>
Date:   Thu Mar 21 17:37:29 2013 +0000
        New commands "mt set per-command {space,time,symtab} {on,off}".

$ make gdb.info
./gdb.texinfo:35648: command @bullet not accepting argument in brace should not be on @table line
make: *** [gdb.info] Error 1



Jan

Re: doc/ build regression with texinfo-5.1 [Re: [doc RFA] New commands: mt set per-command on|off]

[Eli Zaretskii]

> Date: Thu, 28 Mar 2013 21:49:58 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
> 
> On Mon, 18 Mar 2013 22:43:22 +0100, Doug Evans wrote:
> > 	doc/
> > 	* gdb.texinfo (Maintenance Commands): Add docs for
> > 	"mt set per-command {space,time,symtab} {on,off}".
> 
> texinfo-5.1-1.fc20.x86_64
> 
> 1666412af       (Doug Evans     2013-03-21 17:37:29 +0000       35648)@table @bullet
> 
> commit 1666412afbfb0047bf0fb2f7e5789494eeed44f8
> Author: Doug Evans <dje@google.com>
> Date:   Thu Mar 21 17:37:29 2013 +0000
>         New commands "mt set per-command {space,time,symtab} {on,off}".
> 
> $ make gdb.info
> ./gdb.texinfo:35648: command @bullet not accepting argument in brace should not be on @table line
> make: *** [gdb.info] Error 1

Fixed by using @enumerate.

Thanks.

Re: doc/ build regression with texinfo-5.1 [Re: [doc RFA] New commands: mt set per-command on|off]

[Jan Kratochvil]

On Thu, 28 Mar 2013 22:11:45 +0100, Eli Zaretskii wrote:
> > $ make gdb.info
> > ./gdb.texinfo:35648: command @bullet not accepting argument in brace should not be on @table line
> > make: *** [gdb.info] Error 1
> 
> Fixed by using @enumerate.

Just confirming it is fixed now.


Thanks,
Jan

Re: doc/ build regression with texinfo-5.1 [Re: [doc RFA] New commands: mt set per-command on|off]

[Doug Evans]

Jan Kratochvil writes:
 > On Thu, 28 Mar 2013 22:11:45 +0100, Eli Zaretskii wrote:
 > > > $ make gdb.info
 > > > ./gdb.texinfo:35648: command @bullet not accepting argument in brace should not be on @table line
 > > > make: *** [gdb.info] Error 1
 > > 
 > > Fixed by using @enumerate.
 > 
 > Just confirming it is fixed now.

Is this an incompatible change in texinfo, or is my texinfo too lax and not flagging this?

Re: doc/ build regression with texinfo-5.1 [Re: [doc RFA] New commands: mt set per-command on|off]

[Jan Kratochvil]

On Fri, 29 Mar 2013 17:13:37 +0100, Doug Evans wrote:
> Is this an incompatible change in texinfo, or is my texinfo too lax and not
> flagging this?

The latter, current gdb.texinfo is still buildable with texinfo-4.x
(makeinfo); which I assume you still run.

texinfo-5.0+ (makeinfo) is much more strict with what it accepts.


Jan

Re: doc/ build regression with texinfo-5.1 [Re: [doc RFA] New commands: mt set per-command on|off]

[Eli Zaretskii]

> Date: Fri, 29 Mar 2013 17:18:21 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
> 
> texinfo-5.0+ (makeinfo) is much more strict with what it accepts.

Right.

Regression for GDB global --statistics [Re: [doc RFA] New commands: mt set per-command on|off]

[Jan Kratochvil]

On Fri, 15 Mar 2013 23:29:09 +0100, Doug Evans wrote:
> This patch adds a new option to display some simple symtab stats
> akin to how "mt time|space 1" work.  For consistency with the
> rest of gdb I named it:
> 
> maint set per-command symtab on|off
> maint show per-command symtab

bd712aed2f88ab824d403c55a212c2be3f41a335 is the first bad commit
commit bd712aed2f88ab824d403c55a212c2be3f41a335
Author: Doug Evans <dje@google.com>
Date:   Thu Mar 21 17:37:30 2013 +0000
        New commands "mt set per-command {space,time,symtab} {on,off}".

before (gdb-7.6):
gdb -nx -q -statistics  </dev/null
Startup time: 0.031000 (cpu), 0.033828 (wall)
Space used: 4005888 (+4005888 during startup)
(gdb) quit

after (gdb-7.7+):
gdb -nx -q -statistics  </dev/null
(gdb) quit

Commands stats work now but no longer the global stats.  Command stats look
differently ("for this command" vs. "during startup"):
echo echo|gdb -nx -q -statistics 
(gdb) Command execution time: 0.000000 (cpu), 0.000008 (wall)
Space used: 4206592 (+0 for this command)
(gdb) quit

It has been found by Miroslav Franc.


Jan

[PATCH] Re: Regression for GDB global --statistics

[Doug Evans]

Jan Kratochvil writes:
 > On Fri, 15 Mar 2013 23:29:09 +0100, Doug Evans wrote:
 > > This patch adds a new option to display some simple symtab stats
 > > akin to how "mt time|space 1" work.  For consistency with the
 > > rest of gdb I named it:
 > > 
 > > maint set per-command symtab on|off
 > > maint show per-command symtab
 > 
 > bd712aed2f88ab824d403c55a212c2be3f41a335 is the first bad commit
 > commit bd712aed2f88ab824d403c55a212c2be3f41a335
 > Author: Doug Evans <dje@google.com>
 > Date:   Thu Mar 21 17:37:30 2013 +0000
 >         New commands "mt set per-command {space,time,symtab} {on,off}".
 > 
 > before (gdb-7.6):
 > gdb -nx -q -statistics  </dev/null
 > Startup time: 0.031000 (cpu), 0.033828 (wall)
 > Space used: 4005888 (+4005888 during startup)
 > (gdb) quit
 > 
 > after (gdb-7.7+):
 > gdb -nx -q -statistics  </dev/null
 > (gdb) quit
 > 
 > Commands stats work now but no longer the global stats.  Command stats look
 > differently ("for this command" vs. "during startup"):
 > echo echo|gdb -nx -q -statistics 
 > (gdb) Command execution time: 0.000000 (cpu), 0.000008 (wall)
 > Space used: 4206592 (+0 for this command)
 > (gdb) quit
 > 
 > It has been found by Miroslav Franc.

Thanks.

Here's a patch.

2014-07-09  Doug Evans  <dje@google.com>

	* maint.c (count_symtabs_and_blocks): Handle NULL
	current_program_space.
	(report_command_stats): Check global enabled flag in addition to
	recorded enabled flag.
	(make_command_stats_cleanup): Handle msg_type == 0, startup.

	testsuite/
	* gdb.base/maint.exp: Update testing of per-command stats.

diff --git a/gdb/maint.c b/gdb/maint.c
index 5f34af3..5b11f24 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -830,13 +830,16 @@ count_symtabs_and_blocks (int *nr_symtabs_ptr, int *nr_primary_symtabs_ptr,
   int nr_primary_symtabs = 0;
   int nr_blocks = 0;
 
-  ALL_SYMTABS (o, s)
+  if (current_program_space != NULL)
     {
-      ++nr_symtabs;
-      if (s->primary)
+      ALL_SYMTABS (o, s)
 	{
-	  ++nr_primary_symtabs;
-	  nr_blocks += BLOCKVECTOR_NBLOCKS (BLOCKVECTOR (s));
+	  ++nr_symtabs;
+	  if (s->primary)
+	    {
+	      ++nr_primary_symtabs;
+	      nr_blocks += BLOCKVECTOR_NBLOCKS (BLOCKVECTOR (s));
+	    }
 	}
     }
 
@@ -856,7 +859,7 @@ report_command_stats (void *arg)
   struct cmd_stats *start_stats = (struct cmd_stats *) arg;
   int msg_type = start_stats->msg_type;
 
-  if (start_stats->time_enabled)
+  if (start_stats->time_enabled && per_command_time)
     {
       long cmd_time = get_run_time () - start_stats->start_cpu_time;
       struct timeval now_wall_time, delta_wall_time, wait_time;
@@ -877,7 +880,7 @@ report_command_stats (void *arg)
 			 (long) delta_wall_time.tv_usec);
     }
 
-  if (start_stats->space_enabled)
+  if (start_stats->space_enabled && per_command_space)
     {
 #ifdef HAVE_SBRK
       char *lim = (char *) sbrk (0);
@@ -894,7 +897,7 @@ report_command_stats (void *arg)
 #endif
     }
 
-  if (start_stats->symtab_enabled)
+  if (start_stats->symtab_enabled && per_command_symtab)
     {
       int nr_symtabs, nr_primary_symtabs, nr_blocks;
 
@@ -920,8 +923,14 @@ make_command_stats_cleanup (int msg_type)
 {
   struct cmd_stats *new_stat;
 
-  /* Early exit if we're not reporting any stats.  */
-  if (!per_command_time
+  /* Early exit if we're not reporting any stats.  It can be expensive to
+     compute the pre-command values so don't collect them at all if we're
+     not reporting stats.  Alas this doesn't work in the startup case because
+     we don't know yet whether we will be reporting the stats.  For the
+     startup case collect the data anyway (it should be cheap at this point),
+     and leave it to the reporter to decide whether to print them.  */
+  if (msg_type != 0
+      && !per_command_time
       && !per_command_space
       && !per_command_symtab)
     return make_cleanup (null_cleanup, 0);
@@ -930,7 +939,7 @@ make_command_stats_cleanup (int msg_type)
 
   new_stat->msg_type = msg_type;
 
-  if (per_command_space)
+  if (msg_type == 0 || per_command_space)
     {
 #ifdef HAVE_SBRK
       char *lim = (char *) sbrk (0);
@@ -939,14 +948,14 @@ make_command_stats_cleanup (int msg_type)
 #endif
     }
 
-  if (per_command_time)
+  if (msg_type == 0 || per_command_time)
     {
       new_stat->start_cpu_time = get_run_time ();
       gettimeofday (&new_stat->start_wall_time, NULL);
       new_stat->time_enabled = 1;
     }
 
-  if (per_command_symtab)
+  if (msg_type == 0 || per_command_symtab)
     {
       int nr_symtabs, nr_primary_symtabs, nr_blocks;
 
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 64753b7..21d0a31 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -130,10 +130,11 @@ gdb_expect  {
 # tests here!!
 gdb_test_no_output "maint check-symtabs"
 
+# Test per-command stats.
 gdb_test_no_output "maint set per-command on"
-
-gdb_test "maint set per-command off" \
+gdb_test "pwd" \
     "Command execution time: \[0-9.\]+ \\(cpu\\), \[0-9.\]+ \\(wall\\)\[\r\n\]+Space used: $decimal \\(\\+$decimal for this command\\)\[\r\n\]+#symtabs: $decimal \\(\\+$decimal\\), #primary symtabs: $decimal \\(\\+$decimal\\), #blocks: $decimal \\(\\+$decimal\\)"
+gdb_test_no_output "maint set per-command off"
 
 gdb_test "maint demangle" \
     "\"maintenance demangle\" takes an argument to demangle\\."

Re: [PATCH] Re: Regression for GDB global --statistics

[Pedro Alves]

Hi Doug,

On 07/09/2014 10:10 PM, Doug Evans wrote:
> Here's a patch.
> 
> 2014-07-09  Doug Evans  <dje@google.com>
> 
> 	* maint.c (count_symtabs_and_blocks): Handle NULL
> 	current_program_space.

It's not obvious to me how this can be NULL, given we initialize
it so early.  If there's a good reason, could you please add
a comment here mentioning it?

Thanks,
Pedro Alves

Re: [PATCH] Re: Regression for GDB global --statistics

[Jan Kratochvil]

On Wed, 09 Jul 2014 23:10:56 +0200, Doug Evans wrote:
> --- a/gdb/testsuite/gdb.base/maint.exp
> +++ b/gdb/testsuite/gdb.base/maint.exp
> @@ -130,10 +130,11 @@ gdb_expect  {
>  # tests here!!
>  gdb_test_no_output "maint check-symtabs"
>  
> +# Test per-command stats.
>  gdb_test_no_output "maint set per-command on"
> -
> -gdb_test "maint set per-command off" \
> +gdb_test "pwd" \
>      "Command execution time: \[0-9.\]+ \\(cpu\\), \[0-9.\]+ \\(wall\\)\[\r\n\]+Space used: $decimal \\(\\+$decimal for this command\\)\[\r\n\]+#symtabs: $decimal \\(\\+$decimal\\), #primary symtabs: $decimal \\(\\+$decimal\\), #blocks: $decimal \\(\\+$decimal\\)"
> +gdb_test_no_output "maint set per-command off"

Is it needed to change this behavior?  It may break some user scripts working
fine with previous GDB stable releases.

Besides that the startup statistics regression is not tested here but I can
post then a testcase for it.


Thanks,
Jan

Re: [PATCH] Re: Regression for GDB global --statistics

[Doug Evans]

On Fri, Jul 11, 2014 at 2:25 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 09 Jul 2014 23:10:56 +0200, Doug Evans wrote:
>> --- a/gdb/testsuite/gdb.base/maint.exp
>> +++ b/gdb/testsuite/gdb.base/maint.exp
>> @@ -130,10 +130,11 @@ gdb_expect  {
>>  # tests here!!
>>  gdb_test_no_output "maint check-symtabs"
>>
>> +# Test per-command stats.
>>  gdb_test_no_output "maint set per-command on"
>> -
>> -gdb_test "maint set per-command off" \
>> +gdb_test "pwd" \
>>      "Command execution time: \[0-9.\]+ \\(cpu\\), \[0-9.\]+ \\(wall\\)\[\r\n\]+Space used: $decimal \\(\\+$decimal for this command\\)\[\r\n\]+#symtabs: $decimal \\(\\+$decimal\\), #primary symtabs: $decimal \\(\\+$decimal\\), #blocks: $decimal \\(\\+$decimal\\)"
>> +gdb_test_no_output "maint set per-command off"
>
> Is it needed to change this behavior?  It may break some user scripts working
> fine with previous GDB stable releases.

Change in behaviour of something related to a maintenance command?
Or were you referring to something else?

One problem I wish to avoid is not doing anything expensive unnecessarily.
And here that translates to not computing the values of statistics
from before a command executes (so that one can then print the delta
after it completes) if said statistics will not be printed.  That then
translates to any turning on of a statistics-reporting command (mt set
per-command foo) will not take effect until the next command.  How
will that be a problem?

> Besides that the startup statistics regression is not tested here but I can
> post then a testcase for it.

As you wish.  Thanks.

Re: [PATCH] Re: Regression for GDB global --statistics

[Doug Evans]

Pedro Alves writes:
 > Hi Doug,
 > 
 > On 07/09/2014 10:10 PM, Doug Evans wrote:
 > > Here's a patch.
 > > 
 > > 2014-07-09  Doug Evans  <dje@google.com>
 > > 
 > > 	* maint.c (count_symtabs_and_blocks): Handle NULL
 > > 	current_program_space.
 > 
 > It's not obvious to me how this can be NULL, given we initialize
 > it so early.  If there's a good reason, could you please add
 > a comment here mentioning it?

2014-07-11  Doug Evans  <dje@google.com>

	* maint.c (count_symtabs_and_blocks): Handle NULL
	current_program_space.
	(report_command_stats): Check global enabled flag in addition to
	recorded enabled flag.
	(make_command_stats_cleanup): Handle msg_type == 0, startup.

	testsuite/
	* gdb.base/maint.exp: Update testing of per-command stats.

diff --git a/gdb/maint.c b/gdb/maint.c
index 5f34af3..c7a937c 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -830,13 +830,19 @@ count_symtabs_and_blocks (int *nr_symtabs_ptr, int *nr_primary_symtabs_ptr,
   int nr_primary_symtabs = 0;
   int nr_blocks = 0;
 
-  ALL_SYMTABS (o, s)
+  /* When collecting statistics during startup, this is called before
+     pretty much anything in gdb has been initialized, and thus
+     current_program_space may be NULL.  */
+  if (current_program_space != NULL)
     {
-      ++nr_symtabs;
-      if (s->primary)
+      ALL_SYMTABS (o, s)
 	{
-	  ++nr_primary_symtabs;
-	  nr_blocks += BLOCKVECTOR_NBLOCKS (BLOCKVECTOR (s));
+	  ++nr_symtabs;
+	  if (s->primary)
+	    {
+	      ++nr_primary_symtabs;
+	      nr_blocks += BLOCKVECTOR_NBLOCKS (BLOCKVECTOR (s));
+	    }
 	}
     }
 
@@ -856,7 +862,7 @@ report_command_stats (void *arg)
   struct cmd_stats *start_stats = (struct cmd_stats *) arg;
   int msg_type = start_stats->msg_type;
 
-  if (start_stats->time_enabled)
+  if (start_stats->time_enabled && per_command_time)
     {
       long cmd_time = get_run_time () - start_stats->start_cpu_time;
       struct timeval now_wall_time, delta_wall_time, wait_time;
@@ -877,7 +883,7 @@ report_command_stats (void *arg)
 			 (long) delta_wall_time.tv_usec);
     }
 
-  if (start_stats->space_enabled)
+  if (start_stats->space_enabled && per_command_space)
     {
 #ifdef HAVE_SBRK
       char *lim = (char *) sbrk (0);
@@ -894,7 +900,7 @@ report_command_stats (void *arg)
 #endif
     }
 
-  if (start_stats->symtab_enabled)
+  if (start_stats->symtab_enabled && per_command_symtab)
     {
       int nr_symtabs, nr_primary_symtabs, nr_blocks;
 
@@ -920,8 +926,14 @@ make_command_stats_cleanup (int msg_type)
 {
   struct cmd_stats *new_stat;
 
-  /* Early exit if we're not reporting any stats.  */
-  if (!per_command_time
+  /* Early exit if we're not reporting any stats.  It can be expensive to
+     compute the pre-command values so don't collect them at all if we're
+     not reporting stats.  Alas this doesn't work in the startup case because
+     we don't know yet whether we will be reporting the stats.  For the
+     startup case collect the data anyway (it should be cheap at this point),
+     and leave it to the reporter to decide whether to print them.  */
+  if (msg_type != 0
+      && !per_command_time
       && !per_command_space
       && !per_command_symtab)
     return make_cleanup (null_cleanup, 0);
@@ -930,7 +942,7 @@ make_command_stats_cleanup (int msg_type)
 
   new_stat->msg_type = msg_type;
 
-  if (per_command_space)
+  if (msg_type == 0 || per_command_space)
     {
 #ifdef HAVE_SBRK
       char *lim = (char *) sbrk (0);
@@ -939,14 +951,14 @@ make_command_stats_cleanup (int msg_type)
 #endif
     }
 
-  if (per_command_time)
+  if (msg_type == 0 || per_command_time)
     {
       new_stat->start_cpu_time = get_run_time ();
       gettimeofday (&new_stat->start_wall_time, NULL);
       new_stat->time_enabled = 1;
     }
 
-  if (per_command_symtab)
+  if (msg_type == 0 || per_command_symtab)
     {
       int nr_symtabs, nr_primary_symtabs, nr_blocks;
 
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 64753b7..21d0a31 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -130,10 +130,11 @@ gdb_expect  {
 # tests here!!
 gdb_test_no_output "maint check-symtabs"
 
+# Test per-command stats.
 gdb_test_no_output "maint set per-command on"
-
-gdb_test "maint set per-command off" \
+gdb_test "pwd" \
     "Command execution time: \[0-9.\]+ \\(cpu\\), \[0-9.\]+ \\(wall\\)\[\r\n\]+Space used: $decimal \\(\\+$decimal for this command\\)\[\r\n\]+#symtabs: $decimal \\(\\+$decimal\\), #primary symtabs: $decimal \\(\\+$decimal\\), #blocks: $decimal \\(\\+$decimal\\)"
+gdb_test_no_output "maint set per-command off"
 
 gdb_test "maint demangle" \
     "\"maintenance demangle\" takes an argument to demangle\\."

[testsuite patch] Re: Regression for GDB global --statistics

[Jan Kratochvil]

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

On Fri, 11 Jul 2014 23:33:43 +0200, Doug Evans wrote:
> On Fri, Jul 11, 2014 at 2:25 PM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
> > On Wed, 09 Jul 2014 23:10:56 +0200, Doug Evans wrote:
> >> --- a/gdb/testsuite/gdb.base/maint.exp
> >> +++ b/gdb/testsuite/gdb.base/maint.exp
> >> @@ -130,10 +130,11 @@ gdb_expect  {
> >>  # tests here!!
> >>  gdb_test_no_output "maint check-symtabs"
> >>
> >> +# Test per-command stats.
> >>  gdb_test_no_output "maint set per-command on"
> >> -
> >> -gdb_test "maint set per-command off" \
> >> +gdb_test "pwd" \
> >>      "Command execution time: \[0-9.\]+ \\(cpu\\), \[0-9.\]+ \\(wall\\)\[\r\n\]+Space used: $decimal \\(\\+$decimal for this command\\)\[\r\n\]+#symtabs: $decimal \\(\\+$decimal\\), #primary symtabs: $decimal \\(\\+$decimal\\), #blocks: $decimal \\(\\+$decimal\\)"
> >> +gdb_test_no_output "maint set per-command off"
> >
> > Is it needed to change this behavior?  It may break some user scripts working
> > fine with previous GDB stable releases.
> 
> Change in behaviour of something related to a maintenance command?
> Or were you referring to something else?

I am referring to the required change above - which will be required even for
other expect-like scripts using "maint set per-command off".

But in the end I am OK with it, it makes sense to no longer print the stats
after "maint set per-command off".


> One problem I wish to avoid is not doing anything expensive unnecessarily.
> And here that translates to not computing the values of statistics
> from before a command executes (so that one can then print the delta
> after it completes) if said statistics will not be printed.  That then
> translates to any turning on of a statistics-reporting command (mt set
> per-command foo) will not take effect until the next command.  How
> will that be a problem?

I was considering a problem that "maint set per-command off" output has changed.
No matter what it was before and no matter what it is now.


> > Besides that the startup statistics regression is not tested here but I can
> > post then a testcase for it.
> 
> As you wish.  Thanks.

Done below.


Thanks,
Jan

[-- Attachment #2: stats.patch --]
[-- Type: text/plain, Size: 1503 bytes --]

gdb/testsuite/
2014-07-12  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/statistics.exp: New file.

diff --git a/gdb/testsuite/gdb.base/statistics.exp b/gdb/testsuite/gdb.base/statistics.exp
new file mode 100644
index 0000000..22d6f04
--- /dev/null
+++ b/gdb/testsuite/gdb.base/statistics.exp
@@ -0,0 +1,29 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+gdb_exit
+
+# Inlined default_gdb_start.
+set use_gdb_stub [target_info exists use_gdb_stub]
+set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts] -statistics"]
+if { $res < 0 || $res == "" } {
+    perror "Spawning $GDB failed."
+    return
+}
+set gdb_spawn_id -1
+gdb_test "" "Startup time: \[0-9.\]+ \\(cpu\\), \[0-9.\]+ \\(wall\\)\r\nSpace used: \[0-9.\]+ \\(\\+\[0-9.\]+ during startup\\)" "startup time and space"
+gdb_exit

Re: [PATCH] Re: Regression for GDB global --statistics

[Pedro Alves]

On 07/11/2014 11:04 PM, Doug Evans wrote:
> Pedro Alves writes:

>  > It's not obvious to me how this can be NULL, given we initialize
>  > it so early.  If there's a good reason, could you please add
>  > a comment here mentioning it?

...

> -  ALL_SYMTABS (o, s)
> +  /* When collecting statistics during startup, this is called before
> +     pretty much anything in gdb has been initialized, and thus
> +     current_program_space may be NULL.  */
> +  if (current_program_space != NULL)

Great, thanks!

-- 
Pedro Alves

Re: [PATCH] Re: Regression for GDB global --statistics

[Doug Evans]

On Tue, Jul 15, 2014 at 10:37 AM, Pedro Alves <palves@redhat.com> wrote:
> On 07/11/2014 11:04 PM, Doug Evans wrote:
>> Pedro Alves writes:
>
>>  > It's not obvious to me how this can be NULL, given we initialize
>>  > it so early.  If there's a good reason, could you please add
>>  > a comment here mentioning it?
>
> ...
>
>> -  ALL_SYMTABS (o, s)
>> +  /* When collecting statistics during startup, this is called before
>> +     pretty much anything in gdb has been initialized, and thus
>> +     current_program_space may be NULL.  */
>> +  if (current_program_space != NULL)
>
> Great, thanks!
>
> --
> Pedro Alves

Apologies for the repeat (note to self: try to remember to not send
email to list from phone).

Committed to trunk and 7.8.

Jan: Please commit your testcase (no point in resending both messages, bleah).

[testsuite commit] [testsuite patch] Re: Regression for GDB global --statistics

[Jan Kratochvil]

On Thu, 17 Jul 2014 13:49:19 +0200, Doug Evans wrote:
> On Jul 12, 2014 6:42 PM, "Jan Kratochvil" <jan.kratochvil@redhat.com> wrote:
> > gdb/testsuite/
> > 2014-07-12  Jan Kratochvil  <jan.kratochvil@redhat.com>
> >
> >         * gdb.base/statistics.exp: New file.
> 
> LGTM.
> Can you commit this to the 7.8 branch as well. Thanks.
> [I've checked in the fix to 7.8.]

Checked in the testcase to trunk:
	17d0c5c8f07d43142bb35157a0df720b17b8a5ed
	e6cf2ae8bb4c675ebfafb137bf13e92ba96225e8 (mistaken commit fixup)
and to 7.8:
	b4f0a5e936f3fa7912762f3bd0ed2df0fea1888a


Jan