Re: [PATCH] Remove time waiting for user from walltime in stats

[Aaron Gamble <agamble at google dot com>]

re: http://sourceware.org/ml/gdb-patches/2012-05/msg00790.html

Here is an updated version of the patch to also remove the time spend
at y/n decision/question prompts.

    * utils.c (prompt_for_continue_wait_time): New static global.
    (make_command_stats_cleanup): Initialize it.
    (report_command_stats): Subtract time waiting for user.
    (prompt_for_continue): Track time waiting for user.
    (defaulted_query): Track time waiting for user.

--- a/gdb/utils.c	2012-05-20 14:22:01.000000000 -0700
+++ b/gdb/utils.c	2012-05-22 12:00:08.000000000 -0700
@@ -104,6 +104,14 @@
 static void set_screen_size (void);
 static void set_width (void);

+/* Time spent in prompt_for_continue in the currently executing command
+   waiting for user to respond.
+   Initialized in make_command_stats_cleanup.
+   Modified in prompt_for_continue and defaulted_query.
+   Used in report_command_stats.  */
+
+static struct timeval prompt_for_continue_wait_time;
+
 /* A flag indicating whether to timestamp debugging messages.  */

 static int debug_timestamp = 0;
@@ -535,6 +543,10 @@
       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"),
@@ -568,6 +580,7 @@
 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
@@ -578,6 +591,9 @@
   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);
 }
@@ -1188,6 +1204,9 @@
   int def_value;
   char def_answer, not_def_answer;
   char *y_string, *n_string, *question;
+  /* Used to add duration we waited for user to respond to
+     prompt_for_continue_wait_time.  */
+  struct timeval prompt_started, prompt_ended, prompt_delta;

   /* Set up according to which answer is the default.  */
   if (defchar == '\0')
@@ -1244,6 +1263,9 @@

   /* Format the question outside of the loop, to avoid reusing args.  */
   question = xstrvprintf (ctlstr, args);
+
+  /* Used for calculating time spend waiting for user.  */
+  gettimeofday (&prompt_started, NULL);

   while (1)
     {
@@ -1321,6 +1343,12 @@
       printf_filtered (_("Please answer %s or %s.\n"),
 		       y_string, n_string);
     }
+
+  /* Add time spend in this routine to prompt_for_continue_wait_time.  */
+  gettimeofday (&prompt_ended, NULL);
+  timeval_sub (&prompt_delta, &prompt_ended, &prompt_started);
+  timeval_add (&prompt_for_continue_wait_time,
+               &prompt_for_continue_wait_time, &prompt_delta);

   xfree (question);
   if (annotation_level > 1)
@@ -1795,6 +1823,11 @@
 {
   char *ignore;
   char cont_prompt[120];
+  /* Used to add duration we waited for user to respond to
+     prompt_for_continue_wait_time.  */
+  struct timeval prompt_started, prompt_ended, prompt_delta;
+
+  gettimeofday (&prompt_started, NULL);

   if (annotation_level > 1)
     printf_unfiltered (("\n\032\032pre-prompt-for-continue\n"));
@@ -1821,6 +1854,12 @@
      whereas control-C to gdb_readline will cause the user to get dumped
      out to DOS.  */
   ignore = gdb_readline_wrapper (cont_prompt);
+
+  /* Add time spend in this routine to prompt_for_continue_wait_time.  */
+  gettimeofday (&prompt_ended, NULL);
+  timeval_sub (&prompt_delta, &prompt_ended, &prompt_started);
+  timeval_add (&prompt_for_continue_wait_time,
+               &prompt_for_continue_wait_time, &prompt_delta);

   if (annotation_level > 1)
     printf_unfiltered (("\n\032\032post-prompt-for-continue\n"));




On Mon, May 21, 2012 at 2:01 PM, Doug Evans <dje@google.com> wrote:
> On Mon, May 21, 2012 at 1:43 PM, Aaron Gamble <agamble@google.com> wrote:
>> Oops. Corrected ChangeLog entry:
>>
>> ? ? ?* utils.c (prompt_for_continue_wait_time): New static global.
>> ? ? ? (make_command_stats_cleanup): Initialize it.
>> ? ? ? (report_command_stats): Subtract time waiting for user.
>> ? ? ? (prompt_for_continue): Track time waiting for user.
>>
>>
>> -Aaron
>>
>> On Mon, May 21, 2012 at 1:21 PM, Aaron Gamble <agamble@google.com> wrote:
>>> Hi,
>>>
>>> Here is a patch to remove the time a gdb is waiting for a user to
>>> respond to paging from gdb's internal statistics. Currently if a user
>>> is reading paged output and waits 10 seconds to read/respond, those 10
>>> seconds are added to the walltime for the command issued.
>>>
>>> Time spent waiting for the user is stored in a global static variable
>>> in utils.c and is subtracted from the wall time when the stats are
>>> printed.
>>>
>>> 2012-05-21 ?Aaron Gamble ?<agamble@google.com>
>>>
>>> ? ? * gdb/utils.c: Added global static variable to track time spend
>>> waiting for user in paging
>
> The patch is ok with me.
>
> Give it a few days to see if anyone wants to comment.