From stdcxx-dev-return-2276-apmail-incubator-stdcxx-dev-archive=incubator.apache.org@incubator.apache.org Sat Oct 14 01:00:30 2006 Return-Path: Delivered-To: apmail-incubator-stdcxx-dev-archive@www.apache.org Received: (qmail 64762 invoked from network); 14 Oct 2006 01:00:30 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur.apache.org with SMTP; 14 Oct 2006 01:00:30 -0000 Received: (qmail 17751 invoked by uid 500); 14 Oct 2006 01:00:30 -0000 Delivered-To: apmail-incubator-stdcxx-dev-archive@incubator.apache.org Received: (qmail 17740 invoked by uid 500); 14 Oct 2006 01:00:30 -0000 Mailing-List: contact stdcxx-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: stdcxx-dev@incubator.apache.org Delivered-To: mailing list stdcxx-dev@incubator.apache.org Received: (qmail 17729 invoked by uid 99); 14 Oct 2006 01:00:30 -0000 Received: from asf.osuosl.org (HELO asf.osuosl.org) (140.211.166.49) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 13 Oct 2006 18:00:30 -0700 X-ASF-Spam-Status: No, hits=0.0 required=10.0 tests= X-Spam-Check-By: apache.org Received-SPF: pass (asf.osuosl.org: local policy) Received: from [208.30.140.160] (HELO moroha.quovadx.com) (208.30.140.160) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 13 Oct 2006 18:00:28 -0700 Received: from qxvcexch01.ad.quovadx.com (qxvcexch01.ad.quovadx.com [192.168.170.59]) by moroha.quovadx.com (8.13.6/8.13.6) with ESMTP id k9E0xhd5015262 for ; Sat, 14 Oct 2006 00:59:44 GMT Received: from [10.70.3.113] ([10.70.3.113]) by qxvcexch01.ad.quovadx.com with Microsoft SMTPSVC(6.0.3790.1830); Fri, 13 Oct 2006 19:00:19 -0600 Message-ID: <453036AD.2020905@roguewave.com> Date: Fri, 13 Oct 2006 19:00:29 -0600 From: Martin Sebor Organization: Rogue Wave Software User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.13) Gecko/20060417 X-Accept-Language: en-us, en MIME-Version: 1.0 To: stdcxx-dev@incubator.apache.org Subject: Re: [PATCH] wall clock timing References: <4522EE82.7090204@roguewave.com> <452554EA.1020206@roguewave.com> <452C207B.4000305@roguewave.com> In-Reply-To: <452C207B.4000305@roguewave.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 14 Oct 2006 01:00:19.0724 (UTC) FILETIME=[1E93CCC0:01C6EF2C] X-Virus-Checked: Checked by ClamAV on apache.org X-Spam-Rating: minotaur.apache.org 1.6.2 0/1000/N Andrew Black wrote: [...] > Attached is a patch that attempts to simplify the logic flow by using > times() rather than getrusage() and gettimeofday(). Given that we found > Open Solaris implements times() in terms of getrusage() and > gettimeofday(), any speed gains will likely be negligible, but the code > should be cleaner. I believe assumptions about the underlying data type > for clock_t are removed, but I'm not certain. > > --Andrew Black > > Log: [...] > * display.h (unistd.h) [!_WIN32 && !_WIN64]: Remove dependency. > (print_status_plain): Alter logic used to print times, reflecting > change to target_status. I don't see these changes in the patch. What's up? Martin > > > ------------------------------------------------------------------------ > > Index: exec.cpp > =================================================================== > --- exec.cpp (revision 454401) > +++ exec.cpp (working copy) > @@ -40,9 +40,9 @@ > #if !defined (_WIN32) && !defined (_WIN64) > # include /* for close, dup, exec, fork */ > # include > +# include /* for times - is this XSI? */ > # ifdef _XOPEN_UNIX > # include /* for setlimit(), RLIMIT_CORE, ... */ > -# include /* for gettimeofday */ > # endif > #else > # include /* for PROCESS_INFORMATION, ... */ > @@ -734,61 +734,46 @@ > /** > Calculates the amount of resources used by the child processes. > > - This method uses the getrusage() system call to calculate the resources > - used by the child process. However, getrusage() only is able to calcualte > - agragate usage by all child processes, not usage by a specific child process. > - Therefore, we must keep a running tally of how many resources had been used > + This method uses the times() system call to calculate the resources used > + by the child process. However, times() only is able to calcualte agragate > + usage by all child processes, not usage by a specific child process. > + Therefore, we must keep a running tally of how much resources had been used > the previous time we calculated the usage. This difference is the resources > that were used by the process that just completed. > > @param result target_status structure to populate with process usage. > + @param h_clk starting (wall clock) time > + @param h_tms starting (system/user) time > */ > static void > -calculate_usage (struct target_status* result) > +calculate_usage (struct target_status* result, const clock_t h_clk, > + const struct tms* const h_tms) > { > -#ifdef _XOPEN_UNIX > - static bool init = 0; > - static struct rusage history; > - static rw_timeval user, sys; > - struct rusage now; > + static clock_t wall, user, sys; > + struct tms c_tms; > + clock_t c_clk; > > assert (0 != result); > + assert (0 != h_tms); > > - if (!init) { > - memset (&history, 0, sizeof history); > - memset (&user, 0, sizeof user); > - memset (&sys, 0, sizeof sys); > - init = 1; > - } > + wall = user = sys = 0; > > - if (0 != getrusage (RUSAGE_CHILDREN, &now)) { > - warn ("Failed to retrieve child resource usage: %s", strerror (errno)); > + c_clk = times (&c_tms); > + > + if (-1 == wall) { > + warn ("Failed to retrieve ending times: %s", strerror (errno)); > return; > } > > /* time calculations */ > - user.tv_sec = now.ru_utime.tv_sec - history.ru_utime.tv_sec; > - user.tv_usec = now.ru_utime.tv_usec - history.ru_utime.tv_usec; > - sys.tv_sec = now.ru_stime.tv_sec - history.ru_stime.tv_sec; > - sys.tv_usec = now.ru_stime.tv_usec - history.ru_stime.tv_usec; > + wall = c_clk - h_clk; > + user = c_tms.tms_cutime - h_tms->tms_cutime; > + sys = c_tms.tms_cstime - h_tms->tms_cstime; > > - /* Adjust seconds/microseconds */ > - if (now.ru_utime.tv_usec < history.ru_utime.tv_usec) { > - --user.tv_sec; > - user.tv_usec += 1000000; > - } > - if (now.ru_stime.tv_usec < history.ru_stime.tv_usec) { > - --sys.tv_sec; > - sys.tv_usec += 1000000; > - } > - > /* Tag result as having run */ > + result->wall = &wall; > result->user = &user; > result->sys = &sys; > - > - /* Cache the values retrieved */ > - memcpy (&history, &now, sizeof (struct rusage)); > -#endif /* _XOPEN_UNIX */ > } > > void exec_file (const struct target_opts* options, struct target_status* result) > @@ -872,27 +857,14 @@ > strerror (errno)); > } > else { > -#ifdef _XOPEN_UNIX > - struct timeval start; > - static struct timeval delta; > - gettimeofday(&start, 0); > -#endif /* _XOPEN_UNIX */ > /* parent */ > + struct tms h_tms; > + clock_t h_clk = times (&h_tms); > wait_for_child (child_pid, options->timeout, result); > - calculate_usage (result); > -#ifdef _XOPEN_UNIX > - gettimeofday(&delta, 0); > - delta.tv_sec -= start.tv_sec; > - delta.tv_usec -= start.tv_usec; > - > - /* Adjust seconds/microseconds */ > - if (delta.tv_usec < 0) { > - --delta.tv_sec; > - delta.tv_usec += 1000000; > - } > - /* Link the delta */ > - result->wall = δ > -#endif /* _XOPEN_UNIX */ > + if (-1 != h_clk) > + calculate_usage (result, h_clk, &h_tms); > + else > + warn ("Failed to retrieve start times: %s", strerror (errno)); > } > } > #else /* _WIN{32,64} */ > @@ -1099,8 +1071,8 @@ > SECURITY_ATTRIBUTES child_sa = /* SA for inheritable handle. */ > {sizeof (SECURITY_ATTRIBUTES), NULL, TRUE}; > DWORD real_timeout, wait_code; > - FILETIME start, end, delta; > - static rw_timeval convert; > + FILETIME start, end; > + static clock_t wall; > > assert (0 != options); > assert (0 != options->argv); > @@ -1198,22 +1170,13 @@ > > /* Calculate wall clock time elapsed while the process ran */ > GetSystemTimeAsFileTime(&end); > - delta.dwHighDateTime = end.dwHighDateTime - start.dwHighDateTime; > - delta.dwLowDateTime = end.dwLowDateTime - start.dwLowDateTime; > > - /* Handle subtraction across the boundry */ > - if (end.dwLowDateTime < start.dwLowDateTime) > - --delta.dwHighDateTime; > + /* We're ignoring dwHighDateTime, as it's outside the percision of clock_t > + */ > + wall = end.dwLowDateTime - start.dwLowDateTime; > > - /* Convert from 128 bit number to seconds/microseconds */ > - convert.tv_usec = delta.dwLowDateTime % 10000000; > - convert.tv_sec = (delta.dwLowDateTime - convert.tv_usec) / 10000000; > - /* The low half of the date has a resolution of 100 nanoseconds, and > - a magnitude of ~584 years. Therefore, the upper half shouldn't > - matter. */ > - > /* Link the delta */ > - result->wall = &convert; > + result->wall = &wall; > > if (0 == GetExitCodeProcess (child.hProcess, (LPDWORD)&result->exit)) { > warn_last_error ("Retrieving child process exit code"); > Index: cmdopt.cpp > =================================================================== > --- cmdopt.cpp (revision 454401) > +++ cmdopt.cpp (working copy) > @@ -58,11 +58,21 @@ > const char default_path_sep = '/'; > const char suffix_sep = '.'; > const size_t exe_suffix_len = 0; > +#if defined (_SC_CLK_TCK) > +const float TICKS_PER_SEC = sysconf (_SC_CLK_TCK); > +#elif defined (CLK_TCK) > +const float TICKS_PER_SEC = CLK_TCK; > +#elif defined (CLOCKS_PER_SEC) > +const float TICKS_PER_SEC = CLOCKS_PER_SEC; > #else > +# error Unable to determine number of clock ticks in a second. > +#endif > +#else > const char escape_code = '^'; > const char default_path_sep = '\\'; > const char suffix_sep = '.'; > const size_t exe_suffix_len = 4; /* strlen(".exe") == 4 */ > +const float TICKS_PER_SEC = 10000000; /* 100 nanosecond units in a second */ > #endif > > static const char > Index: display.cpp > =================================================================== > --- display.cpp (revision 454401) > +++ display.cpp (working copy) > @@ -26,9 +26,6 @@ > > #include > #include /* for fflush(), printf(), puts(), ... */ > -#if !defined (_WIN32) && !defined (_WIN64) > -# include /* for _XOPEN_UNIX */ > -#endif > > #include "cmdopt.h" /* for target_name -should this be moved? */ > #include "exec.h" /* for get_signame */ > @@ -86,15 +83,13 @@ > > /* Print timings, if available */ > if (valid_timing) > - printf (" %3ld.%03ld %3ld.%03ld", status->user->tv_sec, > - status->user->tv_usec/1000, status->sys->tv_sec, > - status->sys->tv_usec/1000); > + printf (" %7.3f %7.3f", (float) *status->user / TICKS_PER_SEC, > + (float) *status->sys / TICKS_PER_SEC); > else if (status->wall) > printf (" "); > > if (status->wall) > - printf (" %3ld.%03ld\n", status->wall->tv_sec, > - status->wall->tv_usec/1000); > + printf (" %7.3f\n", (float) *status->wall / TICKS_PER_SEC); > else > puts (""); > } > Index: cmdopt.h > =================================================================== > --- cmdopt.h (revision 454401) > +++ cmdopt.h (working copy) > @@ -34,6 +34,7 @@ > extern const char default_path_sep; /**< Primary path seperator */ > extern const char suffix_sep; /**< File suffix seperator. */ > extern const size_t exe_suffix_len; /**< Length of executable suffix. */ > +extern const float TICKS_PER_SEC; /**< Number of clock ticks in a second. */ > > /** > Parses command line arguments for switches and options. > Index: target.h > =================================================================== > --- target.h (revision 454401) > +++ target.h (working copy) > @@ -27,21 +27,18 @@ > #ifndef RW_TARGET_H > #define RW_TARGET_H > > +#include /* for clock_t */ > + > #if !defined (_WIN32) && !defined (_WIN64) > # include /* For _XOPEN_UNIX */ > #endif > > #ifdef _XOPEN_UNIX > # include /* for struct rlimit */ > -# include /* for struct timeval */ > /** > Abstraction typedef for struct rlimit using real struct > */ > typedef struct rlimit rw_rlimit; > -/** > - Abstraction typedef for struct timeval using real struct > -*/ > -typedef struct timeval rw_timeval; > #else /* _XOPEN_UNIX */ > /** > Placeholder rlim_t for use in rw_rlimit > @@ -58,25 +55,6 @@ > Abstraction typedef for struct rlimit using placeholder struct > */ > typedef struct rw_rlimit rw_rlimit; > -/** > - Placeholder time_t for use in rw_timeval > -*/ > -typedef long rw_time_t; > -/** > - Placeholder suseconds_t for use in rw_timeval > -*/ > -typedef long rw_suseconds_t; > -/** > - Placeholder struct timeval to use if _XOPEN_UNIX isn't defined > -*/ > -struct rw_timeval { > - rw_time_t tv_sec; > - rw_suseconds_t tv_usec; > -}; > -/** > - Abstraction typedef for struct timeval using placeholder struct > -*/ > -typedef struct rw_timeval rw_timeval; > #endif /* _XOPEN_UNIX */ > > #ifndef RLIM_INFINITY > @@ -138,9 +116,9 @@ > int signaled; /**< 1 if exit is a signal number, 0 denoting exit code > otherwise */ > enum ProcessStatus status; /**< Textual process status. */ > - const rw_timeval* user; /**< Elapsed user time spent in execution. */ > - const rw_timeval* sys; /**< Elapsed system time spent in execution. */ > - const rw_timeval* wall; /**< Wall clock time spent in execution. */ > + const clock_t* user; /**< Elapsed user time spent in execution. */ > + const clock_t* sys; /**< Elapsed system time spent in execution. */ > + const clock_t* wall; /**< Wall clock time spent in execution. */ > unsigned warn; /**< Number of (test) warnings. */ > unsigned assert; /**< Number of (test) assertions. */ > unsigned failed; /**< Number of failed (test) assertions. */