Re: [PATCH] MI: new timing command

Eli Zaretskii
2007-01-27T14:53:45Z

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Sun, 21 Jan 2007 09:58:18 +1300
> Cc: gdb-patches@sources.redhat.com
>
> > I tried, but unfortunately, I cannot compile the patched version. It
> > seems like at least one part of your patch was never sent to the list:
> >
> > * mi/mi-parse.h: Include <sys/resource.h> if present.
> > (mi_timestamp): New structure.
> > (mi_parse): Add mi_timestamp* member.
> >
> > Without this, mi-main.c doesn't compile, because it misses the
> > definition of the mi_timestamp structure.
>
> Yes, sorry. It was part of an earlier patch but got left out somehow.

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).

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.

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.

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.

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

TIA