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

[Doug Evans <dje@google.com>]

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