From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13179 invoked by alias); 27 Jan 2007 22:08:58 -0000 Received: (qmail 13171 invoked by uid 22791); 27 Jan 2007 22:08:58 -0000 X-Spam-Check-By: sourceware.org Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.8) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 27 Jan 2007 22:08:50 +0000 Received: from kahikatea.snap.net.nz (164.61.255.123.dynamic.snap.net.nz [123.255.61.164]) by viper.snap.net.nz (Postfix) with ESMTP id 9F6A03D8274; Sun, 28 Jan 2007 11:08:44 +1300 (NZDT) Received: by kahikatea.snap.net.nz (Postfix, from userid 500) id A42954F71E; Sun, 28 Jan 2007 11:08:46 +1300 (NZDT) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <17851.52589.491047.323275@kahikatea.snap.net.nz> Date: Sat, 27 Jan 2007 22:08:00 -0000 To: Eli Zaretskii Cc: gdb-patches@sources.redhat.com Subject: Re: [PATCH] MI: new timing command In-Reply-To: References: <17814.10139.269708.848818@kahikatea.snap.net.nz> <17814.58031.865155.682869@kahikatea.snap.net.nz> <20061231042547.GA3236@nevyn.them.org> <17815.18190.987950.612053@kahikatea.snap.net.nz> <20061231054946.GA4873@nevyn.them.org> <17815.27092.497145.908734@kahikatea.snap.net.nz> <20061231151527.GC16449@nevyn.them.org> <200612311524.kBVFObud010411@brahms.sibelius.xs4all.nl> <200612311609.kBVG9Fgh022431@brahms.sibelius.xs4all.nl> <17816.34925.514170.51734@farnswood.snap.net.nz> <17817.34304.221915.628057@kahikatea.snap.net.nz> <17836.26941.915573.399839@kahikatea.snap.net.nz> <17842.33386.836316.276127@kahikatea.snap.net.nz> X-Mailer: VM 7.19 under Emacs 22.0.93.2 X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-01/txt/msg00557.txt.bz2 > 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 > + > + /* 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 + #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 + ); + }