Re: [RFA] Refactor 'maint time' command statistics.

[Paul Hilfinger <Hilfinger at cs dot berkeley dot edu>]

Oops. My mistake.  I left off a necessary change in the gdb.gdb/selftest.exp
test to compensate for modifications to captured_main.  Here is the revision,
with Tom's modifications.  If there is no further objection, I will commit
in a day or so.

P. Hilfinger


Consolidate code for displaying per-command time and space statistics to avoid
duplication.  Piggyback on cleanups so that statistics get printed even when
commands terminate as a result of an error.

Changelog

    * gdb/defs.h (make_command_stats_cleanup): Declare.
    (set_display_time): Declare.
    (set_display_space): Declare.
    * gdb/event-top.c (command_handler): Use make_command_stats_cleanup.
    * gdb/main.c (display_time, display_space): Move definitions to utils.c.
    (captured_main): Use make_command_stats_cleanup to get start-up
    statistics.
    Use set_display_time and set_display_space for processing OPT_STATISTICS
    case.
    * gdb/maint.c (maintenance_time_display): Use set_display_time.
    (maintenance_space_display): Use set_display_space.
    * gdb/top.c (execute_command): Remove obsolete 'maint time' code.
    (command_loop): Use make_command_stats_cleanup.
    * gdb/utils.c (struct cmd_stats): Structure for storing initial time
    and space usage.
    (display_time, display_space): Move definitions here from utils.c.
    (set_display_time): New function.
    (set_display_space): New function.
    (make_command_stats_cleanup): New function.
    (report_command_stats): New auxiliary function for
    make_command_stats_cleanup.
    * gdb/testsuite/gdb.gdb/selftest.exp: Adjust expected message for
    capturing start-up runtime.
---
 gdb/defs.h                         |    6 ++
 gdb/event-top.c                    |   40 +--------------
 gdb/main.c                         |   34 ++-----------
 gdb/maint.c                        |    8 +--
 gdb/testsuite/gdb.gdb/selftest.exp |    4 +-
 gdb/top.c                          |   59 +----------------------
 gdb/utils.c                        |   94 ++++++++++++++++++++++++++++++++++++
 7 files changed, 112 insertions(+), 133 deletions(-)

diff --git a/gdb/defs.h b/gdb/defs.h
index a727a4b..9b2decd 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -307,6 +307,10 @@ extern int subset_compare (char *, char *);
 
 extern char *safe_strerror (int);
 
+extern void set_display_time (int);
+
+extern void set_display_space (int);
+
 #define	ALL_CLEANUPS	((struct cleanup *)0)
 
 extern void do_cleanups (struct cleanup *);
@@ -369,6 +373,8 @@ extern void free_current_contents (void *);
 
 extern void null_cleanup (void *);
 
+extern struct cleanup *make_command_stats_cleanup (int);
+
 extern int myread (int, char *, int);
 
 extern int query (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 718e245..0fc7f89 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -476,12 +476,7 @@ static void
 command_handler (char *command)
 {
   int stdin_is_tty = ISATTY (stdin);
-  long time_at_cmd_start;
-#ifdef HAVE_SBRK
-  long space_at_cmd_start = 0;
-#endif
-  extern int display_time;
-  extern int display_space;
+  struct cleanup *stat_chain;
 
   quit_flag = 0;
   if (instream == stdin && stdin_is_tty)
@@ -498,43 +493,14 @@ command_handler (char *command)
       execute_command ("quit", stdin == instream);
     }
 
-  time_at_cmd_start = get_run_time ();
-
-  if (display_space)
-    {
-#ifdef HAVE_SBRK
-      char *lim = (char *) sbrk (0);
-
-      space_at_cmd_start = lim - lim_at_start;
-#endif
-    }
+  stat_chain = make_command_stats_cleanup (1);
 
   execute_command (command, instream == stdin);
 
   /* Do any commands attached to breakpoint we stopped at.  */
   bpstat_do_actions ();
 
-  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
-    }
+  do_cleanups (stat_chain);
 }
 
 /* Handle a complete line of input. This is called by the callback
diff --git a/gdb/main.c b/gdb/main.c
index c5c712a..bfd1213 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -43,14 +43,6 @@
 #include "cli/cli-cmds.h"
 #include "python/python.h"
 
-/* If nonzero, display time usage both at startup and for each command.  */
-
-int display_time;
-
-/* If nonzero, display space usage both at startup and for each command.  */
-
-int display_space;
-
 /* The selected interpreter.  This will be used as a set command
    variable, so it should always be malloc'ed - since
    do_setshow_command will free it. */
@@ -299,7 +291,7 @@ captured_main (void *data)
   int i;
   int save_auto_load;
 
-  long time_at_startup = get_run_time ();
+  struct cleanup *pre_stat_chain = make_command_stats_cleanup (0);
 
 #if defined (HAVE_SETLOCALE) && defined (HAVE_LC_MESSAGES)
   setlocale (LC_MESSAGES, "");
@@ -485,8 +477,8 @@ captured_main (void *data)
 	    break;
 	  case OPT_STATISTICS:
 	    /* Enable the display of both time and space usage.  */
-	    display_time = 1;
-	    display_space = 1;
+	    set_display_time (1);
+	    set_display_space (1);
 	    break;
 	  case OPT_TUI:
 	    /* --tui is equivalent to -i=tui.  */
@@ -907,25 +899,7 @@ Can't attach to process and specify a core file at the same time."));
     }
 
   /* Show time and/or space usage.  */
-
-  if (display_time)
-    {
-      long init_time = get_run_time () - time_at_startup;
-
-      printf_unfiltered (_("Startup time: %ld.%06ld\n"),
-			 init_time / 1000000, init_time % 1000000);
-    }
-
-  if (display_space)
-    {
-#ifdef HAVE_SBRK
-      extern char **environ;
-      char *lim = (char *) sbrk (0);
-
-      printf_unfiltered (_("Startup size: data size %ld\n"),
-			 (long) (lim - (char *) &environ));
-#endif
-    }
+  do_cleanups (pre_stat_chain);
 
   /* NOTE: cagney/1999-11-07: There is probably no reason for not
      moving this loop and the code found in captured_command_loop()
diff --git a/gdb/maint.c b/gdb/maint.c
index 1673e93..bdcf900 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -171,23 +171,19 @@ maintenance_demangle (char *args, int from_tty)
 static void
 maintenance_time_display (char *args, int from_tty)
 {
-  extern int display_time;
-
   if (args == NULL || *args == '\0')
     printf_unfiltered (_("\"maintenance time\" takes a numeric argument.\n"));
   else
-    display_time = strtol (args, NULL, 10);
+    set_display_time (strtol (args, NULL, 10));
 }
 
 static void
 maintenance_space_display (char *args, int from_tty)
 {
-  extern int display_space;
-
   if (args == NULL || *args == '\0')
     printf_unfiltered ("\"maintenance space\" takes a numeric argument.\n");
   else
-    display_space = strtol (args, NULL, 10);
+    set_display_space (strtol (args, NULL, 10));
 }
 
 /* The "maintenance info" command is defined as a prefix, with
diff --git a/gdb/testsuite/gdb.gdb/selftest.exp b/gdb/testsuite/gdb.gdb/selftest.exp
index d07d147..1aed252 100644
--- a/gdb/testsuite/gdb.gdb/selftest.exp
+++ b/gdb/testsuite/gdb.gdb/selftest.exp
@@ -92,8 +92,8 @@ proc do_steps_and_nexts {} {
 		set description "step over ttyarg initialization"
 		set command "step"
 	    }
-	    -re ".*time_at_startup = get_run_time.*$gdb_prompt $" {
-		set description "next over get_run_time and everything it calls"
+	    -re ".*pre_stat_chain = make_command_stats_cleanup.*$gdb_prompt $" {
+		set description "next over make_command_stats_cleanup and everything it calls"
 		set command "next"
 	    }
 	    -re ".*START_PROGRESS.*$gdb_prompt $" {
diff --git a/gdb/top.c b/gdb/top.c
index 3687cf7..93447fe 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -356,25 +356,6 @@ execute_command (char *p, int from_tty)
   enum language flang;
   static int warned = 0;
   char *line;
-  long time_at_cmd_start = 0;
-#ifdef HAVE_SBRK
-  long space_at_cmd_start = 0;
-#endif
-  extern int display_space;
-
-  if (target_can_async_p ())
-    {
-      time_at_cmd_start = get_run_time ();
-
-      if (display_space)
-	{
-#ifdef HAVE_SBRK
-	  char *lim = (char *) sbrk (0);
-
-	  space_at_cmd_start = lim - lim_at_start;
-#endif
-	}
-    }
 
   prepare_execute_command ();
 
@@ -486,12 +467,6 @@ command_loop (void)
   struct cleanup *old_chain;
   char *command;
   int stdin_is_tty = ISATTY (stdin);
-  long time_at_cmd_start;
-#ifdef HAVE_SBRK
-  long space_at_cmd_start = 0;
-#endif
-  extern int display_time;
-  extern int display_space;
 
   while (instream && !feof (instream))
     {
@@ -510,16 +485,7 @@ command_loop (void)
       if (command == 0)
 	return;
 
-      time_at_cmd_start = get_run_time ();
-
-      if (display_space)
-	{
-#ifdef HAVE_SBRK
-	  char *lim = (char *) sbrk (0);
-
-	  space_at_cmd_start = lim - lim_at_start;
-#endif
-	}
+      make_command_stats_cleanup (1);
 
       execute_command (command, instream == stdin);
 
@@ -527,29 +493,6 @@ command_loop (void)
       bpstat_do_actions ();
 
       do_cleanups (old_chain);
-
-      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 (%s%ld for this command)\n"),
-			     space_now,
-			     (space_diff >= 0 ? "+" : ""),
-			     space_diff);
-#endif
-	}
     }
 }
 
diff --git a/gdb/utils.c b/gdb/utils.c
index 472cf47..a8beb37 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -513,6 +513,100 @@ null_cleanup (void *arg)
 {
 }
 
+/* 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_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_time;
+
+      printf_unfiltered (msg_type == 0
+			 ? _("Startup time: %ld.%06ld\n")
+			 : _("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 - start_stats->start_space;
+
+      printf_unfiltered (msg_type == 0
+			 ? _("Space used: %ld (%c%ld during startup)\n")
+			 : _("Space used: %ld (%c%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)
+{
+  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_time = get_run_time ();
+
+  return make_cleanup_dtor (report_command_stats, new_stat, xfree);
+}
+
 /* Continuations are implemented as cleanups internally.  Inherit from
    cleanups.  */
 struct continuation
-- 
1.7.0.4