Bug 1251303 - Refactor internal printf implementation a little r=terrence
☠☠ backed out by 346a7684fde5 ☠ ☠
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 26 Feb 2016 11:04:15 +0000
changeset 322133 94593f55cd01bd9d2f227f211eec535a61bc7670
parent 322033 f19129c21eb309e5ce4142c7941c7ebd0aa3995b
child 322134 cb43c67f89030fb0ee8e702e48bc78bcbc4c8d51
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1251303
milestone47.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1251303 - Refactor internal printf implementation a little r=terrence
js/src/jsprf.cpp
--- a/js/src/jsprf.cpp
+++ b/js/src/jsprf.cpp
@@ -34,26 +34,23 @@ using namespace js;
 #ifdef HAVE_VA_COPY
 #define VARARGS_ASSIGN(foo, bar)        VA_COPY(foo, bar)
 #elif defined(HAVE_VA_LIST_AS_ARRAY)
 #define VARARGS_ASSIGN(foo, bar)        foo[0] = bar[0]
 #else
 #define VARARGS_ASSIGN(foo, bar)        (foo) = (bar)
 #endif
 
-struct SprintfState
+struct PrintBuffer
 {
-    bool (*stuff)(SprintfState* ss, const char* sp, size_t len);
+    virtual bool append(const char* sp, size_t len) = 0;
 
     char* base;
     char* cur;
     size_t maxlen;
-
-    int (*func)(void* arg, const char* sp, uint32_t len);
-    void* arg;
 };
 
 /*
  * Numbered Argument State
  */
 struct NumArgState
 {
     int type;       // type of the current ap
@@ -79,77 +76,77 @@ typedef mozilla::Vector<NumArgState, 20,
 
 #define FLAG_LEFT       0x1
 #define FLAG_SIGNED     0x2
 #define FLAG_SPACED     0x4
 #define FLAG_ZEROS      0x8
 #define FLAG_NEG        0x10
 
 inline bool
-generic_write(SprintfState* ss, const char* src, size_t srclen)
+generic_write(PrintBuffer* buf, const char* src, size_t srclen)
 {
-    return (*ss->stuff)(ss, src, srclen);
+    return buf->append(src, srclen);
 }
 
 inline bool
-generic_write(SprintfState* ss, const char16_t* src, size_t srclen)
+generic_write(PrintBuffer* buf, const char16_t* src, size_t srclen)
 {
     const size_t CHUNK_SIZE = 64;
     char chunk[CHUNK_SIZE];
 
     size_t j = 0;
     size_t i = 0;
     while (i < srclen) {
         // FIXME: truncates characters to 8 bits
         chunk[j++] = char(src[i++]);
 
         if (j == CHUNK_SIZE || i == srclen) {
-            if (!(*ss->stuff)(ss, chunk, j))
+            if (!buf->append(chunk, j))
                 return false;
             j = 0;
         }
     }
     return true;
 }
 
 // Fill into the buffer using the data in src
 template <typename Char>
 static bool
-fill2(SprintfState* ss, const Char* src, int srclen, int width, int flags)
+fill2(PrintBuffer* buf, const Char* src, int srclen, int width, int flags)
 {
     char space = ' ';
 
     width -= srclen;
     if (width > 0 && (flags & FLAG_LEFT) == 0) {    // Right adjusting
         if (flags & FLAG_ZEROS)
             space = '0';
         while (--width >= 0) {
-            if (!(*ss->stuff)(ss, &space, 1))
+            if (!buf->append(&space, 1))
                 return false;
         }
     }
 
     // Copy out the source data
-    if (!generic_write(ss, src, srclen))
+    if (!generic_write(buf, src, srclen))
         return false;
 
     if (width > 0 && (flags & FLAG_LEFT) != 0) {    // Left adjusting
         while (--width >= 0) {
-            if (!(*ss->stuff)(ss, &space, 1))
+            if (!buf->append(&space, 1))
                 return false;
         }
     }
     return true;
 }
 
 /*
  * Fill a number. The order is: optional-sign zero-filling conversion-digits
  */
 static bool
-fill_n(SprintfState* ss, const char* src, int srclen, int width, int prec, int type, int flags)
+fill_n(PrintBuffer* buf, const char* src, int srclen, int width, int prec, int type, int flags)
 {
     int zerowidth = 0;
     int precwidth = 0;
     int signwidth = 0;
     int leftspaces = 0;
     int rightspaces = 0;
     int cvtwidth;
     char sign;
@@ -189,42 +186,42 @@ fill_n(SprintfState* ss, const char* src
         }
     } else {
         if (width > cvtwidth) {
             // Space filling on the left (i.e. right adjusting)
             leftspaces = width - cvtwidth;
         }
     }
     while (--leftspaces >= 0) {
-        if (!(*ss->stuff)(ss, " ", 1))
+        if (!buf->append(" ", 1))
             return false;
     }
     if (signwidth) {
-        if (!(*ss->stuff)(ss, &sign, 1))
+        if (!buf->append(&sign, 1))
             return false;
     }
     while (--precwidth >= 0) {
-        if (!(*ss->stuff)(ss, "0", 1))
+        if (!buf->append("0", 1))
             return false;
     }
     while (--zerowidth >= 0) {
-        if (!(*ss->stuff)(ss, "0", 1))
+        if (!buf->append("0", 1))
             return false;
     }
-    if (!(*ss->stuff)(ss, src, uint32_t(srclen)))
+    if (!buf->append(src, uint32_t(srclen)))
         return false;
     while (--rightspaces >= 0) {
-        if (!(*ss->stuff)(ss, " ", 1))
+        if (!buf->append(" ", 1))
             return false;
     }
     return true;
 }
 
 /* Convert a long into its printable form. */
-static bool cvt_l(SprintfState* ss, long num, int width, int prec, int radix,
+static bool cvt_l(PrintBuffer* buf, long num, int width, int prec, int radix,
                   int type, int flags, const char* hexp)
 {
     char cvtbuf[100];
     char* cvt;
     int digits;
 
     // according to the man page this needs to happen
     if ((prec == 0) && (num == 0))
@@ -243,21 +240,21 @@ static bool cvt_l(SprintfState* ss, long
     }
     if (digits == 0) {
         *--cvt = '0';
         digits++;
     }
 
     // Now that we have the number converted without its sign, deal with
     // the sign and zero padding.
-    return fill_n(ss, cvt, digits, width, prec, type, flags);
+    return fill_n(buf, cvt, digits, width, prec, type, flags);
 }
 
 /* Convert a 64-bit integer into its printable form. */
-static bool cvt_ll(SprintfState* ss, int64_t num, int width, int prec, int radix,
+static bool cvt_ll(PrintBuffer* buf, int64_t num, int width, int prec, int radix,
                    int type, int flags, const char* hexp)
 {
     // According to the man page, this needs to happen.
     if (prec == 0 && num == 0)
         return true;
 
     // Converting decimal is a little tricky. In the unsigned case we
     // need to stop when we hit 10 digits. In the signed case, we can
@@ -276,24 +273,24 @@ static bool cvt_ll(SprintfState* ss, int
     }
     if (digits == 0) {
         *--cvt = '0';
         digits++;
     }
 
     // Now that we have the number converted without its sign, deal with
     // the sign and zero padding.
-    return fill_n(ss, cvt, digits, width, prec, type, flags);
+    return fill_n(buf, cvt, digits, width, prec, type, flags);
 }
 
 /*
  * Convert a double precision floating point number into its printable
  * form.
  */
-static bool cvt_f(SprintfState* ss, double d, const char* fmt0, const char* fmt1)
+static bool cvt_f(PrintBuffer* buf, double d, const char* fmt0, const char* fmt1)
 {
     char fin[20];
     char fout[300];
     int amount = fmt1 - fmt0;
 
     MOZ_ASSERT((amount > 0) && (amount < (int)sizeof(fin)));
     if (amount >= (int)sizeof(fin)) {
         // Totally bogus % command to sprintf. Just ignore it
@@ -309,46 +306,46 @@ static bool cvt_f(SprintfState* ss, doub
         while (*p) {
             MOZ_ASSERT(*p != 'L');
             p++;
         }
     }
 #endif
     snprintf_literal(fout, fin, d);
 
-    return (*ss->stuff)(ss, fout, strlen(fout));
+    return buf->append(fout, strlen(fout));
 }
 
 static inline const char* generic_null_str(const char*) { return "(null)"; }
 static inline const char16_t* generic_null_str(const char16_t*) { return MOZ_UTF16("(null)"); }
 
 static inline size_t generic_strlen(const char* s) { return strlen(s); }
 static inline size_t generic_strlen(const char16_t* s) { return js_strlen(s); }
 
 /*
  * Convert a string into its printable form.  "width" is the output
  * width. "prec" is the maximum number of characters of "s" to output,
  * where -1 means until NUL.
  */
 template <typename Char>
 static bool
-cvt_s(SprintfState* ss, const Char* s, int width, int prec, int flags)
+cvt_s(PrintBuffer* buf, const Char* s, int width, int prec, int flags)
 {
     if (prec == 0)
         return true;
     if (!s)
         s = generic_null_str(s);
 
     // Limit string length by precision value
     int slen = int(generic_strlen(s));
     if (0 < prec && prec < slen)
         slen = prec;
 
     // and away we go
-    return fill2(ss, s, slen, width, flags);
+    return fill2(buf, s, slen, width, flags);
 }
 
 /*
  * BuildArgArray stands for Numbered Argument list Sprintf
  * for example,
  *      fmp = "%4$i, %2$d, %3s, %1d";
  * the number must start from 1, and no gap among them
  */
@@ -565,17 +562,17 @@ BuildArgArray(const char* fmt, va_list a
 
     return true;
 }
 
 /*
  * The workhorse sprintf code.
  */
 static bool
-dosprintf(SprintfState* ss, const char* fmt, va_list ap)
+dosprintf(PrintBuffer* buf, const char* fmt, va_list ap)
 {
     char c;
     int flags, width, prec, radix, type;
     union {
         char ch;
         char16_t wch;
         int i;
         long l;
@@ -599,30 +596,30 @@ dosprintf(SprintfState* ss, const char* 
     NumArgStateVector nas;
     if (!BuildArgArray(fmt, ap, nas)) {
         // the fmt contains error Numbered Argument format, jliu@netscape.com
         MOZ_CRASH("Bad format string");
     }
 
     while ((c = *fmt++) != 0) {
         if (c != '%') {
-            if (!(*ss->stuff)(ss, fmt - 1, 1))
+            if (!buf->append(fmt - 1, 1))
                 return false;
 
             continue;
         }
         fmt0 = fmt - 1;
 
         // Gobble up the % format string. Hopefully we have handled all
         // of the strange cases!
         flags = 0;
         c = *fmt++;
         if (c == '%') {
             // quoting a % with %%
-            if (!(*ss->stuff)(ss, fmt - 1, 1))
+            if (!buf->append(fmt - 1, 1))
                 return false;
 
             continue;
         }
 
         if (!nas.empty()) {
             // the fmt contains the Numbered Arguments feature
             i = 0;
@@ -761,76 +758,76 @@ dosprintf(SprintfState* ss, const char* 
                 if (u.l < 0) {
                     u.l = -u.l;
                     flags |= FLAG_NEG;
                 }
                 goto do_long;
               case TYPE_UINT32:
                 u.l = (long)va_arg(ap, uint32_t);
               do_long:
-                if (!cvt_l(ss, u.l, width, prec, radix, type, flags, hexp))
+                if (!cvt_l(buf, u.l, width, prec, radix, type, flags, hexp))
                     return false;
 
                 break;
 
               case TYPE_INT64:
                 u.ll = va_arg(ap, int64_t);
                 if (u.ll < 0) {
                     u.ll = -u.ll;
                     flags |= FLAG_NEG;
                 }
                 goto do_longlong;
               case TYPE_UINT64:
                 u.ll = va_arg(ap, uint64_t);
               do_longlong:
-                if (!cvt_ll(ss, u.ll, width, prec, radix, type, flags, hexp))
+                if (!cvt_ll(buf, u.ll, width, prec, radix, type, flags, hexp))
                     return false;
 
                 break;
             }
             break;
 
           case 'e':
           case 'E':
           case 'f':
           case 'g':
             u.d = va_arg(ap, double);
             if (!nas.empty()) {
                 i = fmt - dolPt;
                 if (i < int(sizeof(pattern))) {
                     pattern[0] = '%';
                     js_memcpy(&pattern[1], dolPt, size_t(i));
-                    if (!cvt_f(ss, u.d, pattern, &pattern[i + 1]))
+                    if (!cvt_f(buf, u.d, pattern, &pattern[i + 1]))
                         return false;
                 }
             } else {
-                if (!cvt_f(ss, u.d, fmt0, fmt))
+                if (!cvt_f(buf, u.d, fmt0, fmt))
                     return false;
             }
 
             break;
 
           case 'c':
             if ((flags & FLAG_LEFT) == 0) {
                 while (width-- > 1) {
-                    if (!(*ss->stuff)(ss, " ", 1))
+                    if (!buf->append(" ", 1))
                         return false;
                 }
             }
             switch (type) {
               case TYPE_INT16:
               case TYPE_INTN:
                 u.ch = va_arg(ap, int);
-                if (!(*ss->stuff)(ss, &u.ch, 1))
+                if (!buf->append(&u.ch, 1))
                     return false;
                 break;
             }
             if (flags & FLAG_LEFT) {
                 while (width-- > 1) {
-                    if (!(*ss->stuff)(ss, " ", 1))
+                    if (!buf->append(" ", 1))
                         return false;
                 }
             }
             break;
 
           case 'p':
             if (sizeof(void*) == sizeof(int32_t)) {
                 type = TYPE_UINT32;
@@ -853,84 +850,89 @@ dosprintf(SprintfState* ss, const char* 
             // XXX not supported I suppose
             MOZ_ASSERT(0);
             break;
 #endif
 
           case 's':
             if(type == TYPE_INT16) {
                 u.ws = va_arg(ap, const char16_t*);
-                if (!cvt_s(ss, u.ws, width, prec, flags))
+                if (!cvt_s(buf, u.ws, width, prec, flags))
                     return false;
             } else {
                 u.s = va_arg(ap, const char*);
-                if (!cvt_s(ss, u.s, width, prec, flags))
+                if (!cvt_s(buf, u.s, width, prec, flags))
                     return false;
             }
             break;
 
           case 'n':
             u.ip = va_arg(ap, int*);
             if (u.ip) {
-                *u.ip = ss->cur - ss->base;
+                *u.ip = buf->cur - buf->base;
             }
             break;
 
           default:
             // Not a % token after all... skip it
 #if 0
             MOZ_ASSERT(0);
 #endif
-            if (!(*ss->stuff)(ss, "%", 1))
+            if (!buf->append("%", 1))
                 return false;
-            if (!(*ss->stuff)(ss, fmt - 1, 1))
+            if (!buf->append(fmt - 1, 1))
                 return false;
         }
     }
 
     // Stuff trailing NUL
-    if (!(*ss->stuff)(ss, "\0", 1))
+    if (!buf->append("\0", 1))
         return false;
 
     return true;
 }
 
 /************************************************************************/
 
 /*
- * Stuff routine that automatically grows the js_malloc'd output buffer
+ * Print buffer that automatically grows the js_malloc'd output buffer
  * before it overflows.
  */
-static bool
-GrowStuff(SprintfState* ss, const char* sp, size_t len)
+struct DynamicBuffer : public PrintBuffer
+{
+    bool append(const char* sp, size_t len) override;
+};
+
+bool
+DynamicBuffer::append(const char* sp, size_t len)
 {
     ptrdiff_t off;
     char* newbase;
     size_t newlen;
 
-    off = ss->cur - ss->base;
-    if (off + len >= ss->maxlen) {
+    off = cur - base;
+    if (off + len >= maxlen) {
         /* Grow the buffer */
-        newlen = ss->maxlen + ((len > 32) ? len : 32);
-        newbase = static_cast<char*>(js_realloc(ss->base, newlen));
+        newlen = maxlen + ((len > 32) ? len : 32);
+        newbase = static_cast<char*>(js_realloc(base, newlen));
         if (!newbase) {
             /* Ran out of memory */
             return false;
         }
-        ss->base = newbase;
-        ss->maxlen = newlen;
-        ss->cur = ss->base + off;
+        base = newbase;
+        maxlen = newlen;
+        cur = base + off;
     }
 
     /* Copy data */
     while (len) {
         --len;
-        *ss->cur++ = *sp++;
+        *cur++ = *sp++;
     }
-    MOZ_ASSERT(size_t(ss->cur - ss->base) <= ss->maxlen);
+    MOZ_ASSERT(size_t(cur - base) <= maxlen);
     return true;
 }
 
 /*
  * sprintf into a js_malloc'd buffer
  */
 JS_PUBLIC_API(char*)
 JS_smprintf(const char* fmt, ...)
@@ -951,42 +953,46 @@ JS_PUBLIC_API(void)
 JS_smprintf_free(char* mem)
 {
     js_free(mem);
 }
 
 JS_PUBLIC_API(char*)
 JS_vsmprintf(const char* fmt, va_list ap)
 {
-    SprintfState ss;
+    DynamicBuffer buf;
 
-    ss.stuff = GrowStuff;
-    ss.base = 0;
-    ss.cur = 0;
-    ss.maxlen = 0;
-    if (!dosprintf(&ss, fmt, ap)) {
-        js_free(ss.base);
+    buf.base = 0;
+    buf.cur = 0;
+    buf.maxlen = 0;
+    if (!dosprintf(&buf, fmt, ap)) {
+        js_free(buf.base);
         return 0;
     }
-    return ss.base;
+    return buf.base;
 }
 
 /*
- * Stuff routine that discards overflow data
+ * Print buffer that discards overflow data
  */
-static bool
-LimitStuff(SprintfState* ss, const char* sp, size_t len)
+struct FixedBuffer : public PrintBuffer
 {
-    size_t limit = ss->maxlen - (ss->cur - ss->base);
+    bool append(const char* sp, size_t len) override;
+};
+
+bool
+FixedBuffer::append(const char* sp, size_t len)
+{
+    size_t limit = maxlen - (cur - base);
 
     if (len > limit)
         len = limit;
     while (len) {
         --len;
-        *ss->cur++ = *sp++;
+        *cur++ = *sp++;
     }
     return true;
 }
 
 /*
  * sprintf into a fixed size buffer. Make sure there is a NUL at the end
  * when finished.
  */
@@ -1004,34 +1010,34 @@ JS_snprintf(char* out, uint32_t outlen, 
     rv = JS_vsnprintf(out, outlen, fmt, ap);
     va_end(ap);
     return rv;
 }
 
 JS_PUBLIC_API(uint32_t)
 JS_vsnprintf(char* out, uint32_t outlen, const char* fmt, va_list ap)
 {
-    SprintfState ss;
+    FixedBuffer buf;
 
     if (outlen == 0)
         return 0;
 
-    ss.stuff = LimitStuff;
-    ss.base = out;
-    ss.cur = out;
-    ss.maxlen = outlen;
-    (void) dosprintf(&ss, fmt, ap);
+    buf.base = out;
+    buf.cur = out;
+    buf.maxlen = outlen;
 
-    uint32_t charsWritten = ss.cur - ss.base;
+    (void) dosprintf(&buf, fmt, ap);
+
+    uint32_t charsWritten = buf.cur - buf.base;
     MOZ_RELEASE_ASSERT(charsWritten > 0);
 
     // If we didn't append a null then we must have hit the buffer limit. Write
     // a null terminator now and return a value indicating that we failed.
-    if (ss.cur[-1] != '\0') {
-        ss.cur[-1] = '\0';
+    if (buf.cur[-1] != '\0') {
+        buf.cur[-1] = '\0';
         return outlen;
     }
 
     // Success: return the number of character written excluding the null
     // terminator.
     return charsWritten - 1;
 }
 
@@ -1045,34 +1051,33 @@ JS_sprintf_append(char* last, const char
     rv = JS_vsprintf_append(last, fmt, ap);
     va_end(ap);
     return rv;
 }
 
 JS_PUBLIC_API(char*)
 JS_vsprintf_append(char* last, const char* fmt, va_list ap)
 {
-    SprintfState ss;
+    DynamicBuffer buf;
 
-    ss.stuff = GrowStuff;
     if (last) {
         size_t lastlen = strlen(last);
-        ss.base = last;
-        ss.cur = last + lastlen;
-        ss.maxlen = lastlen;
+        buf.base = last;
+        buf.cur = last + lastlen;
+        buf.maxlen = lastlen;
     } else {
-        ss.base = 0;
-        ss.cur = 0;
-        ss.maxlen = 0;
+        buf.base = 0;
+        buf.cur = 0;
+        buf.maxlen = 0;
     }
-    if (!dosprintf(&ss, fmt, ap)) {
-        js_free(ss.base);
+    if (!dosprintf(&buf, fmt, ap)) {
+        js_free(buf.base);
         return 0;
     }
-    return ss.base;
+    return buf.base;
 }
 
 #undef TYPE_INT16
 #undef TYPE_UINT16
 #undef TYPE_INTN
 #undef TYPE_UINTN
 #undef TYPE_INT32
 #undef TYPE_UINT32