stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: [PATCH] wall clock timing
Date Sat, 14 Oct 2006 01:00:29 GMT
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 <unistd.h> /* for close, dup, exec, fork */
>  #  include <sys/wait.h>
> +#  include <sys/times.h> /* for times - is this XSI? */
>  #  ifdef _XOPEN_UNIX
>  #    include <sys/resource.h> /* for setlimit(), RLIMIT_CORE, ... */
> -#    include <sys/time.h> /* for gettimeofday */
>  #  endif
>  #else
>  #  include <windows.h> /* 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 = &delta;
> -#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 <assert.h>
>  #include <stdio.h>      /* for fflush(), printf(), puts(), ... */
> -#if !defined (_WIN32) && !defined (_WIN64)
> -#  include <unistd.h> /* 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 <sys/types.h> /* for clock_t */
> +
>  #if !defined (_WIN32) && !defined (_WIN64)
>  #  include <unistd.h> /* For _XOPEN_UNIX */
>  #endif
>  
>  #ifdef _XOPEN_UNIX
>  #  include <sys/resource.h> /* for struct rlimit */
> -#  include <sys/time.h> /* 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. */


Mime
View raw message