Re: [PATCH] MI: new timing command

[Nick Roberts <nickrob@snap.net.nz>]

 > Okay, I tried this now, and on a system which has sys/resource.h and
 > getrusage, it works both with HAVE_SYS_RESOURCE_H and HAVE_GETRUSAGE
 > and without them (I hacked config.h to get it compiled both ways).

Hopefully you hacked config.h just to save time as my change to configure.ac
should handle that.

 > However, I don't see how it can work on systems without sys/resource.h
 > and without getrusage: your patch uses struct rusage unconditionally:
 > 
 >     mi-parse.h:
 > 
 >     + #include <sys/resource.h>
 >     + 
 >     + /* Timestamps for current command and last asynchronous command */
 >     + struct mi_timestamp {
 >     +     struct timeval wallclock;
 >     +     struct rusage rusage;
 >     + };
 >     + 
 >
 >     m-main.c:
 > 
 >     + static void 
 >     + timestamp (struct mi_timestamp *tv)
 >     +   {
 >     +     long usec;
 >     + #ifdef HAVE_GETRUSAGE
 >     +     gettimeofday (&tv->wallclock, NULL);
 >     +     getrusage (RUSAGE_SELF, &tv->rusage);
 >     + #else
 >     +     usec = get_run_time ();
 >     +     tv->wallclock.tv_sec = usec/1000000;
 >     +     tv->wallclock.utv_sec = usec - 1000000*tv->wallclock.tv_sec;
 >     +     tv->rusage.ru_utime.tv_sec = 0;
 >     +     tv->rusage.ru_utime.tv_usec = 0;
 >     +     tv->rusage.ru_stime.tv_sec = 0;
 >     +     tv->rusage.ru_stime.tv_usec = 0;
 >     + #endif
 >     +   }
 > 
 > 
 > Note that mi-parse.h includes sys/resource.h unconditionally, which
 > will fail to compile on systems that don't have that header; and both
 > mi-parse.h and mi-main.c use struct rusage.

Duh!  I see that now.  

 > Also, in mi-main.c, there's no need to use gettimeofday only in
 > the HAVE_GETRUSAGE branch, as its availability is independent of
 > HAVE_GETRUSAGE.

Well it was explained to me that get_run_time basically returns wallclock
time when getrusage isn't defined.  If that is the case, I could just
gettimeofday for both cases and never use get_run_time instead.

 > I suggest to use gettimeofday to compute wallclock time on all
 > platforms, and avoid using struct rusage in struct mi_timestamp,
 > e.g. like this:
 > 
 >     struct mi_timestamp {
 > 	struct timeval wallclock;
 > 	struct timeval utime;
 > 	struct timeval stime'
 >     }
 >
 > Then in mi-main.c:timestamp you could copy the values from what
 > getrusage returns to the utime and stime members of mi_timestamp, when
 > getrusage is available, and if not, assign the value returned by
 > get_run_time to members of utime.

Won't the members wallclock and utime be basically the same now?

 > If you submit a modified patch that does the above, I will test it on
 > a platform that doesn't have getrusage.

Would something like below work?  When getrusage isn't defined I just use
wallclock.  There's no need to define utime or stime in this case as the
values are always 0.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


*** mi-parse.h	28 Jan 2007 10:54:38 +1300	1.6
--- mi-parse.h	28 Jan 2007 10:39:02 +1300	
***************
*** 24,29 ****
--- 24,41 ----
  
  /* MI parser */
  
+ #if defined HAVE_SYS_RESOURCE_H
+ #include <sys/resource.h>
+ #endif
+ 
+ /* Timestamps for current command and last asynchronous command.  */
+ struct mi_timestamp {
+     struct timeval wallclock;
+ #ifdef HAVE_GETRUSAGE
+     struct rusage rusage;
+ #endif
+ };
+ 
  enum mi_command_type
    {
      MI_COMMAND, CLI_COMMAND
*************** struct mi_parse
*** 35,40 ****
--- 47,53 ----
      char *command;
      char *token;
      const struct mi_cmd *cmd;
+     struct mi_timestamp *cmd_start;
      char *args;
      char **argv;
      int argc;


*** mi-main.c	23 Jan 2007 20:21:56 +1300	1.91
--- mi-main.c	28 Jan 2007 10:44:28 +1300	
*************** _initialize_mi_main (void)
*** 1457,1459 ****
--- 1513,1562 ----
    DEPRECATED_REGISTER_GDBARCH_SWAP (old_regs);
    deprecated_register_gdbarch_swap (NULL, 0, mi_setup_architecture_data);
  }
+ 
+ static void 
+ timestamp (struct mi_timestamp *tv)
+   {
+     long usec;
+ #ifdef HAVE_GETRUSAGE
+     gettimeofday (&tv->wallclock, NULL);
+     getrusage (RUSAGE_SELF, &tv->rusage);
+ #else
+     usec = get_run_time ();
+     tv->wallclock.tv_sec = usec/1000000;
+     tv->wallclock.utv_sec = usec - 1000000*tv->wallclock.tv_sec;
+ #endif
+   }
+ 
+ static void 
+ print_diff_now (struct mi_timestamp *start)
+   {
+     struct mi_timestamp now;
+     timestamp (&now);
+     print_diff (start, &now);
+   }
+ 
+ static long 
+ timeval_diff (struct timeval start, struct timeval end)
+   {
+     return ((end.tv_sec - start.tv_sec) * 1000000)
+       + (end.tv_usec - start.tv_usec);
+   }
+ 
+ static void 
+ print_diff (struct mi_timestamp *start, struct mi_timestamp *end)
+   {
+     fprintf_unfiltered
+       (raw_stdout,
+        ",time={wallclock=\"%0.5f\",user=\"%0.5f\",system=\"%0.5f\"}", 
+        timeval_diff (start->wallclock, end->wallclock) / 1000000.0, 
+ #ifdef HAVE_GETRUSAGE
+        timeval_diff (start->rusage.ru_utime, end->rusage.ru_utime)
+        / 1000000.0, 
+        timeval_diff (start->rusage.ru_stime, end->rusage.ru_stime)
+        / 1000000.0
+ #else
+        0, 0
+ #endif
+        );
+   }