Re: Performance improvements for src/port/snprintf.c - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Performance improvements for src/port/snprintf.c
Date
Msg-id 2238.1534716465@sss.pgh.pa.us
Whole thread Raw
In response to Performance improvements for src/port/snprintf.c  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Performance improvements for src/port/snprintf.c
List pgsql-hackers
I wrote:
> [ snprintf-speedups-1.patch ]

Here's a slightly improved version of that, with two changes:

* Given the current state of the what-about-%m thread, it's no longer
academic how well this performs relative to glibc's version.  I poked
at that and found that a lot of the discrepancy came from glibc using
strchrnul() to find the next format specifier --- apparently, that
function is a *lot* faster than the equivalent manual loop.  So this
version uses that if available.

* I thought of a couple of easy wins for fmtfloat.  We can pass the
precision spec down to the platform's sprintf using "*" notation instead
of converting it to text and back, and that also simplifies matters enough
that we can avoid using an sprintf call to build the simplified format
string.  This seems to get us down to the vicinity of a 10% speed penalty
on microbenchmarks of just float conversion, which is enough to satisfy
me given the other advantages of switching to our own snprintf.

            regards, tom lane

diff --git a/configure b/configure
index 836d68d..dff9f0c 100755
*** a/configure
--- b/configure
*************** fi
*** 15032,15038 ****
  LIBS_including_readline="$LIBS"
  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`

! for ac_func in cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll
posix_fallocatepstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open symlink
sync_file_rangeutime utimes wcstombs_l 
  do :
    as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
  ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
--- 15032,15038 ----
  LIBS_including_readline="$LIBS"
  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`

! for ac_func in cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll
posix_fallocatepstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul symlink
sync_file_rangeutime utimes wcstombs_l 
  do :
    as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
  ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.in b/configure.in
index 6e14106..c00bb8f 100644
*** a/configure.in
--- b/configure.in
*************** PGAC_FUNC_WCSTOMBS_L
*** 1535,1541 ****
  LIBS_including_readline="$LIBS"
  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`

! AC_CHECK_FUNCS([cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll
posix_fallocatepstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open symlink
sync_file_rangeutime utimes wcstombs_l]) 

  AC_REPLACE_FUNCS(fseeko)
  case $host_os in
--- 1535,1541 ----
  LIBS_including_readline="$LIBS"
  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`

! AC_CHECK_FUNCS([cbrt clock_gettime dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll
posix_fallocatepstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul symlink
sync_file_rangeutime utimes wcstombs_l]) 

  AC_REPLACE_FUNCS(fseeko)
  case $host_os in
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 827574e..da9cfa7 100644
*** a/src/include/pg_config.h.in
--- b/src/include/pg_config.h.in
***************
*** 519,524 ****
--- 519,527 ----
  /* Define to 1 if you have the <stdlib.h> header file. */
  #undef HAVE_STDLIB_H

+ /* Define to 1 if you have the `strchrnul' function. */
+ #undef HAVE_STRCHRNUL
+
  /* Define to 1 if you have the `strerror' function. */
  #undef HAVE_STRERROR

diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index 46ce49d..73d7424 100644
*** a/src/include/pg_config.h.win32
--- b/src/include/pg_config.h.win32
***************
*** 390,395 ****
--- 390,398 ----
  /* Define to 1 if you have the <stdlib.h> header file. */
  #define HAVE_STDLIB_H 1

+ /* Define to 1 if you have the `strchrnul' function. */
+ /* #undef HAVE_STRCHRNUL */
+
  /* Define to 1 if you have the `strerror' function. */
  #ifndef HAVE_STRERROR
  #define HAVE_STRERROR 1
diff --git a/src/port/snprintf.c b/src/port/snprintf.c
index 851e2ae..66151c2 100644
*** a/src/port/snprintf.c
--- b/src/port/snprintf.c
*************** flushbuffer(PrintfTarget *target)
*** 295,301 ****
  }


! static void fmtstr(char *value, int leftjust, int minlen, int maxwidth,
         int pointflag, PrintfTarget *target);
  static void fmtptr(void *value, PrintfTarget *target);
  static void fmtint(int64 value, char type, int forcesign,
--- 295,303 ----
  }


! static bool find_arguments(const char *format, va_list args,
!                PrintfArgValue *argvalues);
! static void fmtstr(const char *value, int leftjust, int minlen, int maxwidth,
         int pointflag, PrintfTarget *target);
  static void fmtptr(void *value, PrintfTarget *target);
  static void fmtint(int64 value, char type, int forcesign,
*************** static void fmtfloat(double value, char
*** 307,317 ****
           PrintfTarget *target);
  static void dostr(const char *str, int slen, PrintfTarget *target);
  static void dopr_outch(int c, PrintfTarget *target);
  static int    adjust_sign(int is_negative, int forcesign, int *signvalue);
! static void adjust_padlen(int minlen, int vallen, int leftjust, int *padlen);
! static void leading_pad(int zpad, int *signvalue, int *padlen,
              PrintfTarget *target);
! static void trailing_pad(int *padlen, PrintfTarget *target);


  /*
--- 309,320 ----
           PrintfTarget *target);
  static void dostr(const char *str, int slen, PrintfTarget *target);
  static void dopr_outch(int c, PrintfTarget *target);
+ static void dopr_outchmulti(int c, int slen, PrintfTarget *target);
  static int    adjust_sign(int is_negative, int forcesign, int *signvalue);
! static int    compute_padlen(int minlen, int vallen, int leftjust);
! static void leading_pad(int zpad, int signvalue, int *padlen,
              PrintfTarget *target);
! static void trailing_pad(int padlen, PrintfTarget *target);


  /*
*************** static void trailing_pad(int *padlen, Pr
*** 320,329 ****
  static void
  dopr(PrintfTarget *target, const char *format, va_list args)
  {
!     const char *format_start = format;
      int            ch;
      bool        have_dollar;
-     bool        have_non_dollar;
      bool        have_star;
      bool        afterstar;
      int            accum;
--- 323,331 ----
  static void
  dopr(PrintfTarget *target, const char *format, va_list args)
  {
!     const char *first_pct = NULL;
      int            ch;
      bool        have_dollar;
      bool        have_star;
      bool        afterstar;
      int            accum;
*************** dopr(PrintfTarget *target, const char *f
*** 335,559 ****
      int            precision;
      int            zpad;
      int            forcesign;
-     int            last_dollar;
      int            fmtpos;
      int            cvalue;
      int64        numvalue;
      double        fvalue;
      char       *strvalue;
-     int            i;
-     PrintfArgType argtypes[NL_ARGMAX + 1];
      PrintfArgValue argvalues[NL_ARGMAX + 1];

      /*
!      * Parse the format string to determine whether there are %n$ format
!      * specs, and identify the types and order of the format parameters.
       */
!     have_dollar = have_non_dollar = false;
!     last_dollar = 0;
!     MemSet(argtypes, 0, sizeof(argtypes));

!     while ((ch = *format++) != '\0')
      {
!         if (ch != '%')
!             continue;
!         longflag = longlongflag = pointflag = 0;
!         fmtpos = accum = 0;
!         afterstar = false;
! nextch1:
!         ch = *format++;
!         if (ch == '\0')
!             break;                /* illegal, but we don't complain */
!         switch (ch)
          {
!             case '-':
!             case '+':
!                 goto nextch1;
!             case '0':
!             case '1':
!             case '2':
!             case '3':
!             case '4':
!             case '5':
!             case '6':
!             case '7':
!             case '8':
!             case '9':
!                 accum = accum * 10 + (ch - '0');
!                 goto nextch1;
!             case '.':
!                 pointflag = 1;
!                 accum = 0;
!                 goto nextch1;
!             case '*':
!                 if (afterstar)
!                     have_non_dollar = true; /* multiple stars */
!                 afterstar = true;
!                 accum = 0;
!                 goto nextch1;
!             case '$':
!                 have_dollar = true;
!                 if (accum <= 0 || accum > NL_ARGMAX)
!                     goto bad_format;
!                 if (afterstar)
!                 {
!                     if (argtypes[accum] &&
!                         argtypes[accum] != ATYPE_INT)
!                         goto bad_format;
!                     argtypes[accum] = ATYPE_INT;
!                     last_dollar = Max(last_dollar, accum);
!                     afterstar = false;
!                 }
!                 else
!                     fmtpos = accum;
!                 accum = 0;
!                 goto nextch1;
!             case 'l':
!                 if (longflag)
!                     longlongflag = 1;
!                 else
!                     longflag = 1;
!                 goto nextch1;
!             case 'z':
! #if SIZEOF_SIZE_T == 8
! #ifdef HAVE_LONG_INT_64
!                 longflag = 1;
! #elif defined(HAVE_LONG_LONG_INT_64)
!                 longlongflag = 1;
! #else
! #error "Don't know how to print 64bit integers"
! #endif
  #else
!                 /* assume size_t is same size as int */
  #endif
-                 goto nextch1;
-             case 'h':
-             case '\'':
-                 /* ignore these */
-                 goto nextch1;
-             case 'd':
-             case 'i':
-             case 'o':
-             case 'u':
-             case 'x':
-             case 'X':
-                 if (fmtpos)
-                 {
-                     PrintfArgType atype;

!                     if (longlongflag)
!                         atype = ATYPE_LONGLONG;
!                     else if (longflag)
!                         atype = ATYPE_LONG;
!                     else
!                         atype = ATYPE_INT;
!                     if (argtypes[fmtpos] &&
!                         argtypes[fmtpos] != atype)
!                         goto bad_format;
!                     argtypes[fmtpos] = atype;
!                     last_dollar = Max(last_dollar, fmtpos);
!                 }
!                 else
!                     have_non_dollar = true;
!                 break;
!             case 'c':
!                 if (fmtpos)
!                 {
!                     if (argtypes[fmtpos] &&
!                         argtypes[fmtpos] != ATYPE_INT)
!                         goto bad_format;
!                     argtypes[fmtpos] = ATYPE_INT;
!                     last_dollar = Max(last_dollar, fmtpos);
!                 }
!                 else
!                     have_non_dollar = true;
!                 break;
!             case 's':
!             case 'p':
!                 if (fmtpos)
!                 {
!                     if (argtypes[fmtpos] &&
!                         argtypes[fmtpos] != ATYPE_CHARPTR)
!                         goto bad_format;
!                     argtypes[fmtpos] = ATYPE_CHARPTR;
!                     last_dollar = Max(last_dollar, fmtpos);
!                 }
!                 else
!                     have_non_dollar = true;
!                 break;
!             case 'e':
!             case 'E':
!             case 'f':
!             case 'g':
!             case 'G':
!                 if (fmtpos)
!                 {
!                     if (argtypes[fmtpos] &&
!                         argtypes[fmtpos] != ATYPE_DOUBLE)
!                         goto bad_format;
!                     argtypes[fmtpos] = ATYPE_DOUBLE;
!                     last_dollar = Max(last_dollar, fmtpos);
!                 }
!                 else
!                     have_non_dollar = true;
                  break;
!             case '%':
                  break;
          }

          /*
!          * If we finish the spec with afterstar still set, there's a
!          * non-dollar star in there.
           */
!         if (afterstar)
!             have_non_dollar = true;
!     }
!
!     /* Per spec, you use either all dollar or all not. */
!     if (have_dollar && have_non_dollar)
!         goto bad_format;
!
!     /*
!      * In dollar mode, collect the arguments in physical order.
!      */
!     for (i = 1; i <= last_dollar; i++)
!     {
!         switch (argtypes[i])
!         {
!             case ATYPE_NONE:
!                 goto bad_format;
!             case ATYPE_INT:
!                 argvalues[i].i = va_arg(args, int);
!                 break;
!             case ATYPE_LONG:
!                 argvalues[i].l = va_arg(args, long);
!                 break;
!             case ATYPE_LONGLONG:
!                 argvalues[i].ll = va_arg(args, int64);
!                 break;
!             case ATYPE_DOUBLE:
!                 argvalues[i].d = va_arg(args, double);
!                 break;
!             case ATYPE_CHARPTR:
!                 argvalues[i].cptr = va_arg(args, char *);
!                 break;
!         }
!     }
!
!     /*
!      * At last we can parse the format for real.
!      */
!     format = format_start;
!     while ((ch = *format++) != '\0')
!     {
!         if (target->failed)
!             break;

!         if (ch != '%')
!         {
!             dopr_outch(ch, target);
!             continue;
!         }
          fieldwidth = precision = zpad = leftjust = forcesign = 0;
          longflag = longlongflag = pointflag = 0;
          fmtpos = accum = 0;
--- 337,397 ----
      int            precision;
      int            zpad;
      int            forcesign;
      int            fmtpos;
      int            cvalue;
      int64        numvalue;
      double        fvalue;
      char       *strvalue;
      PrintfArgValue argvalues[NL_ARGMAX + 1];

      /*
!      * Initially, we suppose the format string does not use %n$.  The first
!      * time we come to a conversion spec that has that, we'll call
!      * find_arguments() to check for consistent use of %n$ and fill the
!      * argvalues array with the argument values in the correct order.
       */
!     have_dollar = false;

!     while (*format != '\0')
      {
!         /* Locate next conversion specifier */
!         if (*format != '%')
          {
!             const char *next_pct = format + 1;
!
!             /*
!              * If strchrnul exists (it's a glibc-ism), it's a good bit faster
!              * than the equivalent manual loop.  Note: this doesn't compile
!              * cleanly without -D_GNU_SOURCE, but we normally use that on
!              * glibc platforms.
!              */
! #ifdef HAVE_STRCHRNUL
!             next_pct = strchrnul(next_pct, '%');
  #else
!             while (*next_pct != '\0' && *next_pct != '%')
!                 next_pct++;
  #endif

!             /* Dump literal data we just scanned over */
!             dostr(format, next_pct - format, target);
!             if (target->failed)
                  break;
!
!             if (*next_pct == '\0')
                  break;
+             format = next_pct;
          }

          /*
!          * Remember start of first conversion spec; if we find %n$, then it's
!          * sufficient for find_arguments() to start here, without rescanning
!          * earlier literal text.
           */
!         if (first_pct == NULL)
!             first_pct = format;

!         /* Process conversion spec starting at *format */
!         format++;
          fieldwidth = precision = zpad = leftjust = forcesign = 0;
          longflag = longlongflag = pointflag = 0;
          fmtpos = accum = 0;
*************** nextch2:
*** 597,603 ****
              case '*':
                  if (have_dollar)
                  {
!                     /* process value after reading n$ */
                      afterstar = true;
                  }
                  else
--- 435,445 ----
              case '*':
                  if (have_dollar)
                  {
!                     /*
!                      * We'll process value after reading n$.  Note it's OK to
!                      * assume have_dollar is set correctly, because in a valid
!                      * format string the initial % must have had n$ if * does.
!                      */
                      afterstar = true;
                  }
                  else
*************** nextch2:
*** 628,633 ****
--- 470,483 ----
                  accum = 0;
                  goto nextch2;
              case '$':
+                 /* First dollar sign? */
+                 if (!have_dollar)
+                 {
+                     /* Yup, so examine all conversion specs in format */
+                     if (!find_arguments(first_pct, args, argvalues))
+                         goto bad_format;
+                     have_dollar = true;
+                 }
                  if (afterstar)
                  {
                      /* fetch and process star value */
*************** nextch2:
*** 806,811 ****
--- 656,665 ----
                  dopr_outch('%', target);
                  break;
          }
+
+         /* Check for failure after each conversion spec */
+         if (target->failed)
+             break;
      }

      return;
*************** bad_format:
*** 815,822 ****
      target->failed = true;
  }

  static void
! fmtstr(char *value, int leftjust, int minlen, int maxwidth,
         int pointflag, PrintfTarget *target)
  {
      int            padlen,
--- 669,903 ----
      target->failed = true;
  }

+ /*
+  * find_arguments(): sort out the arguments for a format spec with %n$
+  *
+  * If format is valid, return true and fill argvalues[i] with the value
+  * for the conversion spec that has %i$ or *i$.  Else return false.
+  */
+ static bool
+ find_arguments(const char *format, va_list args,
+                PrintfArgValue *argvalues)
+ {
+     int            ch;
+     bool        afterstar;
+     int            accum;
+     int            longlongflag;
+     int            longflag;
+     int            fmtpos;
+     int            i;
+     int            last_dollar;
+     PrintfArgType argtypes[NL_ARGMAX + 1];
+
+     /* Initialize to "no dollar arguments known" */
+     last_dollar = 0;
+     MemSet(argtypes, 0, sizeof(argtypes));
+
+     /*
+      * This loop must accept the same format strings as the one in dopr().
+      * However, we don't need to analyze them to the same level of detail.
+      *
+      * Since we're only called if there's a dollar-type spec somewhere, we can
+      * fail immediately if we find a non-dollar spec.  Per the C99 standard,
+      * all argument references in the format string must be one or the other.
+      */
+     while (*format != '\0')
+     {
+         /* Locate next conversion specifier */
+         if (*format != '%')
+         {
+             /* Unlike dopr, we can just quit if there's no more specifiers */
+             format = strchr(format + 1, '%');
+             if (format == NULL)
+                 break;
+         }
+
+         /* Process conversion spec starting at *format */
+         format++;
+         longflag = longlongflag = 0;
+         fmtpos = accum = 0;
+         afterstar = false;
+ nextch1:
+         ch = *format++;
+         if (ch == '\0')
+             break;                /* illegal, but we don't complain */
+         switch (ch)
+         {
+             case '-':
+             case '+':
+                 goto nextch1;
+             case '0':
+             case '1':
+             case '2':
+             case '3':
+             case '4':
+             case '5':
+             case '6':
+             case '7':
+             case '8':
+             case '9':
+                 accum = accum * 10 + (ch - '0');
+                 goto nextch1;
+             case '.':
+                 accum = 0;
+                 goto nextch1;
+             case '*':
+                 if (afterstar)
+                     return false;    /* previous star missing dollar */
+                 afterstar = true;
+                 accum = 0;
+                 goto nextch1;
+             case '$':
+                 if (accum <= 0 || accum > NL_ARGMAX)
+                     return false;
+                 if (afterstar)
+                 {
+                     if (argtypes[accum] &&
+                         argtypes[accum] != ATYPE_INT)
+                         return false;
+                     argtypes[accum] = ATYPE_INT;
+                     last_dollar = Max(last_dollar, accum);
+                     afterstar = false;
+                 }
+                 else
+                     fmtpos = accum;
+                 accum = 0;
+                 goto nextch1;
+             case 'l':
+                 if (longflag)
+                     longlongflag = 1;
+                 else
+                     longflag = 1;
+                 goto nextch1;
+             case 'z':
+ #if SIZEOF_SIZE_T == 8
+ #ifdef HAVE_LONG_INT_64
+                 longflag = 1;
+ #elif defined(HAVE_LONG_LONG_INT_64)
+                 longlongflag = 1;
+ #else
+ #error "Don't know how to print 64bit integers"
+ #endif
+ #else
+                 /* assume size_t is same size as int */
+ #endif
+                 goto nextch1;
+             case 'h':
+             case '\'':
+                 /* ignore these */
+                 goto nextch1;
+             case 'd':
+             case 'i':
+             case 'o':
+             case 'u':
+             case 'x':
+             case 'X':
+                 if (fmtpos)
+                 {
+                     PrintfArgType atype;
+
+                     if (longlongflag)
+                         atype = ATYPE_LONGLONG;
+                     else if (longflag)
+                         atype = ATYPE_LONG;
+                     else
+                         atype = ATYPE_INT;
+                     if (argtypes[fmtpos] &&
+                         argtypes[fmtpos] != atype)
+                         return false;
+                     argtypes[fmtpos] = atype;
+                     last_dollar = Max(last_dollar, fmtpos);
+                 }
+                 else
+                     return false;    /* non-dollar conversion spec */
+                 break;
+             case 'c':
+                 if (fmtpos)
+                 {
+                     if (argtypes[fmtpos] &&
+                         argtypes[fmtpos] != ATYPE_INT)
+                         return false;
+                     argtypes[fmtpos] = ATYPE_INT;
+                     last_dollar = Max(last_dollar, fmtpos);
+                 }
+                 else
+                     return false;    /* non-dollar conversion spec */
+                 break;
+             case 's':
+             case 'p':
+                 if (fmtpos)
+                 {
+                     if (argtypes[fmtpos] &&
+                         argtypes[fmtpos] != ATYPE_CHARPTR)
+                         return false;
+                     argtypes[fmtpos] = ATYPE_CHARPTR;
+                     last_dollar = Max(last_dollar, fmtpos);
+                 }
+                 else
+                     return false;    /* non-dollar conversion spec */
+                 break;
+             case 'e':
+             case 'E':
+             case 'f':
+             case 'g':
+             case 'G':
+                 if (fmtpos)
+                 {
+                     if (argtypes[fmtpos] &&
+                         argtypes[fmtpos] != ATYPE_DOUBLE)
+                         return false;
+                     argtypes[fmtpos] = ATYPE_DOUBLE;
+                     last_dollar = Max(last_dollar, fmtpos);
+                 }
+                 else
+                     return false;    /* non-dollar conversion spec */
+                 break;
+             case '%':
+                 break;
+         }
+
+         /*
+          * If we finish the spec with afterstar still set, there's a
+          * non-dollar star in there.
+          */
+         if (afterstar)
+             return false;        /* non-dollar conversion spec */
+     }
+
+     /*
+      * Format appears valid so far, so collect the arguments in physical
+      * order.  (Since we rejected any non-dollar specs that would have
+      * collected arguments, we know that dopr() hasn't collected any yet.)
+      */
+     for (i = 1; i <= last_dollar; i++)
+     {
+         switch (argtypes[i])
+         {
+             case ATYPE_NONE:
+                 return false;
+             case ATYPE_INT:
+                 argvalues[i].i = va_arg(args, int);
+                 break;
+             case ATYPE_LONG:
+                 argvalues[i].l = va_arg(args, long);
+                 break;
+             case ATYPE_LONGLONG:
+                 argvalues[i].ll = va_arg(args, int64);
+                 break;
+             case ATYPE_DOUBLE:
+                 argvalues[i].d = va_arg(args, double);
+                 break;
+             case ATYPE_CHARPTR:
+                 argvalues[i].cptr = va_arg(args, char *);
+                 break;
+         }
+     }
+
+     return true;
+ }
+
  static void
! fmtstr(const char *value, int leftjust, int minlen, int maxwidth,
         int pointflag, PrintfTarget *target)
  {
      int            padlen,
*************** fmtstr(char *value, int leftjust, int mi
*** 831,847 ****
      else
          vallen = strlen(value);

!     adjust_padlen(minlen, vallen, leftjust, &padlen);

!     while (padlen > 0)
      {
!         dopr_outch(' ', target);
!         --padlen;
      }

      dostr(value, vallen, target);

!     trailing_pad(&padlen, target);
  }

  static void
--- 912,928 ----
      else
          vallen = strlen(value);

!     padlen = compute_padlen(minlen, vallen, leftjust);

!     if (padlen > 0)
      {
!         dopr_outchmulti(' ', padlen, target);
!         padlen = 0;
      }

      dostr(value, vallen, target);

!     trailing_pad(padlen, target);
  }

  static void
*************** fmtint(int64 value, char type, int force
*** 869,875 ****
      int            signvalue = 0;
      char        convert[64];
      int            vallen = 0;
!     int            padlen = 0;        /* amount to pad */
      int            zeropad;        /* extra leading zeroes */

      switch (type)
--- 950,956 ----
      int            signvalue = 0;
      char        convert[64];
      int            vallen = 0;
!     int            padlen;            /* amount to pad */
      int            zeropad;        /* extra leading zeroes */

      switch (type)
*************** fmtint(int64 value, char type, int force
*** 917,958 ****

          do
          {
!             convert[vallen++] = cvt[uvalue % base];
              uvalue = uvalue / base;
          } while (uvalue);
      }

      zeropad = Max(0, precision - vallen);

!     adjust_padlen(minlen, vallen + zeropad, leftjust, &padlen);

!     leading_pad(zpad, &signvalue, &padlen, target);

!     while (zeropad-- > 0)
!         dopr_outch('0', target);

!     while (vallen > 0)
!         dopr_outch(convert[--vallen], target);

!     trailing_pad(&padlen, target);
  }

  static void
  fmtchar(int value, int leftjust, int minlen, PrintfTarget *target)
  {
!     int            padlen = 0;        /* amount to pad */

!     adjust_padlen(minlen, 1, leftjust, &padlen);

!     while (padlen > 0)
      {
!         dopr_outch(' ', target);
!         --padlen;
      }

      dopr_outch(value, target);

!     trailing_pad(&padlen, target);
  }

  static void
--- 998,1038 ----

          do
          {
!             convert[sizeof(convert) - (++vallen)] = cvt[uvalue % base];
              uvalue = uvalue / base;
          } while (uvalue);
      }

      zeropad = Max(0, precision - vallen);

!     padlen = compute_padlen(minlen, vallen + zeropad, leftjust);

!     leading_pad(zpad, signvalue, &padlen, target);

!     if (zeropad > 0)
!         dopr_outchmulti('0', zeropad, target);

!     dostr(convert + sizeof(convert) - vallen, vallen, target);

!     trailing_pad(padlen, target);
  }

  static void
  fmtchar(int value, int leftjust, int minlen, PrintfTarget *target)
  {
!     int            padlen;            /* amount to pad */

!     padlen = compute_padlen(minlen, 1, leftjust);

!     if (padlen > 0)
      {
!         dopr_outchmulti(' ', padlen, target);
!         padlen = 0;
      }

      dopr_outch(value, target);

!     trailing_pad(padlen, target);
  }

  static void
*************** fmtfloat(double value, char type, int fo
*** 963,972 ****
      int            signvalue = 0;
      int            prec;
      int            vallen;
!     char        fmt[32];
      char        convert[1024];
      int            zeropadlen = 0; /* amount to pad with zeroes */
!     int            padlen = 0;        /* amount to pad with spaces */

      /*
       * We rely on the regular C library's sprintf to do the basic conversion,
--- 1043,1056 ----
      int            signvalue = 0;
      int            prec;
      int            vallen;
!     char        fmt[8];
      char        convert[1024];
      int            zeropadlen = 0; /* amount to pad with zeroes */
!     int            padlen;            /* amount to pad with spaces */
!
!     /* Handle sign (NaNs have no sign) */
!     if (!isnan(value) && adjust_sign((value < 0), forcesign, &signvalue))
!         value = -value;

      /*
       * We rely on the regular C library's sprintf to do the basic conversion,
*************** fmtfloat(double value, char type, int fo
*** 988,1004 ****

      if (pointflag)
      {
-         if (sprintf(fmt, "%%.%d%c", prec, type) < 0)
-             goto fail;
          zeropadlen = precision - prec;
      }
-     else if (sprintf(fmt, "%%%c", type) < 0)
-         goto fail;
-
-     if (!isnan(value) && adjust_sign((value < 0), forcesign, &signvalue))
-         value = -value;
-
-     vallen = sprintf(convert, fmt, value);
      if (vallen < 0)
          goto fail;

--- 1072,1092 ----

      if (pointflag)
      {
          zeropadlen = precision - prec;
+         fmt[0] = '%';
+         fmt[1] = '.';
+         fmt[2] = '*';
+         fmt[3] = type;
+         fmt[4] = '\0';
+         vallen = sprintf(convert, fmt, prec, value);
+     }
+     else
+     {
+         fmt[0] = '%';
+         fmt[1] = type;
+         fmt[2] = '\0';
+         vallen = sprintf(convert, fmt, value);
      }
      if (vallen < 0)
          goto fail;

*************** fmtfloat(double value, char type, int fo
*** 1006,1014 ****
      if (zeropadlen > 0 && !isdigit((unsigned char) convert[vallen - 1]))
          zeropadlen = 0;

!     adjust_padlen(minlen, vallen + zeropadlen, leftjust, &padlen);

!     leading_pad(zpad, &signvalue, &padlen, target);

      if (zeropadlen > 0)
      {
--- 1094,1102 ----
      if (zeropadlen > 0 && !isdigit((unsigned char) convert[vallen - 1]))
          zeropadlen = 0;

!     padlen = compute_padlen(minlen, vallen + zeropadlen, leftjust);

!     leading_pad(zpad, signvalue, &padlen, target);

      if (zeropadlen > 0)
      {
*************** fmtfloat(double value, char type, int fo
*** 1019,1036 ****
              epos = strrchr(convert, 'E');
          if (epos)
          {
!             /* pad after exponent */
              dostr(convert, epos - convert, target);
!             while (zeropadlen-- > 0)
!                 dopr_outch('0', target);
              dostr(epos, vallen - (epos - convert), target);
          }
          else
          {
              /* no exponent, pad after the digits */
              dostr(convert, vallen, target);
!             while (zeropadlen-- > 0)
!                 dopr_outch('0', target);
          }
      }
      else
--- 1107,1124 ----
              epos = strrchr(convert, 'E');
          if (epos)
          {
!             /* pad before exponent */
              dostr(convert, epos - convert, target);
!             if (zeropadlen > 0)
!                 dopr_outchmulti('0', zeropadlen, target);
              dostr(epos, vallen - (epos - convert), target);
          }
          else
          {
              /* no exponent, pad after the digits */
              dostr(convert, vallen, target);
!             if (zeropadlen > 0)
!                 dopr_outchmulti('0', zeropadlen, target);
          }
      }
      else
*************** fmtfloat(double value, char type, int fo
*** 1039,1045 ****
          dostr(convert, vallen, target);
      }

!     trailing_pad(&padlen, target);
      return;

  fail:
--- 1127,1133 ----
          dostr(convert, vallen, target);
      }

!     trailing_pad(padlen, target);
      return;

  fail:
*************** fail:
*** 1049,1054 ****
--- 1137,1149 ----
  static void
  dostr(const char *str, int slen, PrintfTarget *target)
  {
+     /* fast path for common case of slen == 1 */
+     if (slen == 1)
+     {
+         dopr_outch(*str, target);
+         return;
+     }
+
      while (slen > 0)
      {
          int            avail;
*************** dopr_outch(int c, PrintfTarget *target)
*** 1092,1097 ****
--- 1187,1228 ----
      *(target->bufptr++) = c;
  }

+ static void
+ dopr_outchmulti(int c, int slen, PrintfTarget *target)
+ {
+     /* fast path for common case of slen == 1 */
+     if (slen == 1)
+     {
+         dopr_outch(c, target);
+         return;
+     }
+
+     while (slen > 0)
+     {
+         int            avail;
+
+         if (target->bufend != NULL)
+             avail = target->bufend - target->bufptr;
+         else
+             avail = slen;
+         if (avail <= 0)
+         {
+             /* buffer full, can we dump to stream? */
+             if (target->stream == NULL)
+             {
+                 target->nchars += slen; /* no, lose the data */
+                 return;
+             }
+             flushbuffer(target);
+             continue;
+         }
+         avail = Min(avail, slen);
+         memset(target->bufptr, c, avail);
+         target->bufptr += avail;
+         slen -= avail;
+     }
+ }
+

  static int
  adjust_sign(int is_negative, int forcesign, int *signvalue)
*************** adjust_sign(int is_negative, int forcesi
*** 1107,1148 ****
  }


! static void
! adjust_padlen(int minlen, int vallen, int leftjust, int *padlen)
  {
!     *padlen = minlen - vallen;
!     if (*padlen < 0)
!         *padlen = 0;
      if (leftjust)
!         *padlen = -(*padlen);
  }


  static void
! leading_pad(int zpad, int *signvalue, int *padlen, PrintfTarget *target)
  {
      if (*padlen > 0 && zpad)
      {
!         if (*signvalue)
          {
!             dopr_outch(*signvalue, target);
              --(*padlen);
!             *signvalue = 0;
          }
!         while (*padlen > 0)
          {
!             dopr_outch(zpad, target);
!             --(*padlen);
          }
      }
!     while (*padlen > (*signvalue != 0))
      {
!         dopr_outch(' ', target);
!         --(*padlen);
      }
!     if (*signvalue)
      {
!         dopr_outch(*signvalue, target);
          if (*padlen > 0)
              --(*padlen);
          else if (*padlen < 0)
--- 1238,1285 ----
  }


! static int
! compute_padlen(int minlen, int vallen, int leftjust)
  {
!     int            padlen;
!
!     padlen = minlen - vallen;
!     if (padlen < 0)
!         padlen = 0;
      if (leftjust)
!         padlen = -padlen;
!     return padlen;
  }


  static void
! leading_pad(int zpad, int signvalue, int *padlen, PrintfTarget *target)
  {
+     int            maxpad;
+
      if (*padlen > 0 && zpad)
      {
!         if (signvalue)
          {
!             dopr_outch(signvalue, target);
              --(*padlen);
!             signvalue = 0;
          }
!         if (*padlen > 0)
          {
!             dopr_outchmulti(zpad, *padlen, target);
!             *padlen = 0;
          }
      }
!     maxpad = (signvalue != 0);
!     if (*padlen > maxpad)
      {
!         dopr_outchmulti(' ', *padlen - maxpad, target);
!         *padlen = maxpad;
      }
!     if (signvalue)
      {
!         dopr_outch(signvalue, target);
          if (*padlen > 0)
              --(*padlen);
          else if (*padlen < 0)
*************** leading_pad(int zpad, int *signvalue, in
*** 1152,1162 ****


  static void
! trailing_pad(int *padlen, PrintfTarget *target)
  {
!     while (*padlen < 0)
!     {
!         dopr_outch(' ', target);
!         ++(*padlen);
!     }
  }
--- 1289,1296 ----


  static void
! trailing_pad(int padlen, PrintfTarget *target)
  {
!     if (padlen < 0)
!         dopr_outchmulti(' ', -padlen, target);
  }

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Allowing printf("%m") only where it actually works
Next
From: David Rowley
Date:
Subject: Re: [sqlsmith] ERROR: partition missing from subplans