Thread: timing for \copy

The attached patch implements timing for psql's \copy command, the
absence of which I have whined about previously.

If there's no objection I will apply this in a few days. I'm not sure
that a docs change is needed.

cheers

andrew
Index: src/bin/psql/command.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.174
diff -c -r1.174 command.c
*** src/bin/psql/command.c    6 Oct 2006 17:14:00 -0000    1.174
--- src/bin/psql/command.c    13 Dec 2006 16:32:07 -0000
***************
*** 20,25 ****
--- 20,26 ----
  #include <sys/types.h>            /* for umask() */
  #include <sys/stat.h>            /* for stat() */
  #include <fcntl.h>                /* open() flags */
+ #include <sys/time.h>           /* for gettimeofday() */
  #include <unistd.h>                /* for geteuid(), getpid(), stat() */
  #else
  #include <win32.h>
***************
*** 28,33 ****
--- 29,35 ----
  #include <direct.h>
  #include <sys/types.h>            /* for umask() */
  #include <sys/stat.h>            /* for stat() */
+ #include <sys/timeb.h>            /* for _ftime()  - used in GETTIMEOFDAY */
  #endif

  #include "libpq-fe.h"
***************
*** 303,312 ****
      /* \copy */
      else if (pg_strcasecmp(cmd, "copy") == 0)
      {
          char       *opt = psql_scan_slash_option(scan_state,
                                                   OT_WHOLE_LINE, NULL, false);
!
          success = do_copy(opt);
          free(opt);
      }

--- 305,331 ----
      /* \copy */
      else if (pg_strcasecmp(cmd, "copy") == 0)
      {
+         /* Default fetch-it-all-and-print mode */
+         TimevalStruct before,
+                     after;
+         double        elapsed_msec = 0;
+
          char       *opt = psql_scan_slash_option(scan_state,
                                                   OT_WHOLE_LINE, NULL, false);
!         if (pset.timing)
!             GETTIMEOFDAY(&before);
!
          success = do_copy(opt);
+
+         if (pset.timing)
+         {
+             GETTIMEOFDAY(&after);
+             elapsed_msec = DIFF_MSEC(&after, &before);
+             if (success)
+                 printf(_("Time: %.3f ms\n"), elapsed_msec);
+
+         }
+
          free(opt);
      }

Index: src/bin/psql/common.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/common.c,v
retrieving revision 1.130
diff -c -r1.130 common.c
*** src/bin/psql/common.c    4 Oct 2006 00:30:05 -0000    1.130
--- src/bin/psql/common.c    13 Dec 2006 16:32:08 -0000
***************
*** 28,55 ****
  #include "mbprint.h"


- /* Workarounds for Windows */
- /* Probably to be moved up the source tree in the future, perhaps to be replaced by
-  * more specific checks like configure-style HAVE_GETTIMEOFDAY macros.
-  */
- #ifndef WIN32
-
- typedef struct timeval TimevalStruct;
-
- #define GETTIMEOFDAY(T) gettimeofday(T, NULL)
- #define DIFF_MSEC(T, U) \
-     ((((int) ((T)->tv_sec - (U)->tv_sec)) * 1000000.0 + \
-       ((int) ((T)->tv_usec - (U)->tv_usec))) / 1000.0)
- #else
-
- typedef struct _timeb TimevalStruct;
-
- #define GETTIMEOFDAY(T) _ftime(T)
- #define DIFF_MSEC(T, U) \
-     (((T)->time - (U)->time) * 1000.0 + \
-      ((T)->millitm - (U)->millitm))
- #endif
-

  static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
  static bool command_no_begin(const char *query);
--- 28,33 ----
Index: src/bin/psql/common.h
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/common.h,v
retrieving revision 1.51
diff -c -r1.51 common.h
*** src/bin/psql/common.h    4 Oct 2006 00:30:05 -0000    1.51
--- src/bin/psql/common.h    13 Dec 2006 16:32:08 -0000
***************
*** 63,66 ****
--- 63,88 ----

  extern char *expand_tilde(char **filename);

+ /* Workarounds for Windows */
+ /* Probably to be moved up the source tree in the future, perhaps to be replaced by
+  * more specific checks like configure-style HAVE_GETTIMEOFDAY macros.
+  */
+ #ifndef WIN32
+
+ typedef struct timeval TimevalStruct;
+
+ #define GETTIMEOFDAY(T) gettimeofday(T, NULL)
+ #define DIFF_MSEC(T, U) \
+     ((((int) ((T)->tv_sec - (U)->tv_sec)) * 1000000.0 + \
+       ((int) ((T)->tv_usec - (U)->tv_usec))) / 1000.0)
+ #else
+
+ typedef struct _timeb TimevalStruct;
+
+ #define GETTIMEOFDAY(T) _ftime(T)
+ #define DIFF_MSEC(T, U) \
+     (((T)->time - (U)->time) * 1000.0 + \
+      ((T)->millitm - (U)->millitm))
+ #endif
+
  #endif   /* COMMON_H */

On Wed, 2006-12-13 at 11:36 -0500, Andrew Dunstan wrote:
> +         if (pset.timing)
> +         {
> +             GETTIMEOFDAY(&after);
> +             elapsed_msec = DIFF_MSEC(&after, &before);
> +             if (success)
> +                 printf(_("Time: %.3f ms\n"), elapsed_msec);
> +
> +         }

Not exactly a big deal, but

    if (pset.timing && success)
    {
        /* ... */
    }

seems a bit clearer, and avoids unnecessary work on failure. You can
also move "elapsed_msec" and "after" inside the "if" block.

Also, I believe headers should #include the headers that they themselves
depend on, so the inclusion of the getttimeofday() or _ftime() headers
could be moved to common.h

-Neil

Neil Conway wrote:
> You can
> also move "elapsed_msec" and "after" inside the "if" block.
>
>
>

Thanks.

I'll do the other stuff, but I prefer to keep related declarations
together. I'm not a huge fan of blindly pushing every declaration to the
lowest level possible.

I'll commit after I have made sure it works on Windows.

cheers

andrew