Bug 1539605 - add fast paths for integer formatting on nsTSubstring; r=erahm
authorNathan Froyd <froydnj@mozilla.com>
Fri, 29 Mar 2019 19:38:53 +0000
changeset 466869 a5c4f7e6069a1962f5364226f4746914c08ae6ef
parent 466868 88df194b6f92e1cb6623398323f2e593d58bfddc
child 466870 f7937d3264db00771b46cb1fcba71640d8df05cb
push id35784
push usernerli@mozilla.com
push dateSat, 30 Mar 2019 09:32:04 +0000
treeherdermozilla-central@d42c60ccf0d0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm
bugs1539605
milestone68.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 1539605 - add fast paths for integer formatting on nsTSubstring; r=erahm This way we don't have to go through a bunch of printf nonsense, and we ought to be able to arrive at optimized routines that take advantage of constant radices, etc. Differential Revision: https://phabricator.services.mozilla.com/D25141
mozglue/misc/Printf.cpp
mozglue/misc/Printf.h
xpcom/string/nsTSubstring.cpp
xpcom/string/nsTSubstring.h
--- a/mozglue/misc/Printf.cpp
+++ b/mozglue/misc/Printf.cpp
@@ -67,16 +67,19 @@ typedef mozilla::Vector<NumArgState, 20,
 #define TYPE_UNKNOWN 20
 
 #define FLAG_LEFT 0x1
 #define FLAG_SIGNED 0x2
 #define FLAG_SPACED 0x4
 #define FLAG_ZEROS 0x8
 #define FLAG_NEG 0x10
 
+static const char hex[] = "0123456789abcdef";
+static const char HEX[] = "0123456789ABCDEF";
+
 // Fill into the buffer using the data in src
 bool mozilla::PrintfTarget::fill2(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';
@@ -162,16 +165,65 @@ bool mozilla::PrintfTarget::fill_n(const
   }
   if (!emit(src, uint32_t(srclen))) return false;
   while (--rightspaces >= 0) {
     if (!emit(" ", 1)) return false;
   }
   return true;
 }
 
+// All that the cvt_* functions care about as far as the TYPE_* constants is
+// that the low bit is set to indicate unsigned, or unset to indicate signed.
+// So we don't try to hard to ensure that the passed TYPE_* constant lines
+// up with the actual size of the number being printed here.  The main printf
+// code, below, does have to care so that the correct bits are extracted from
+// the varargs list.
+bool mozilla::PrintfTarget::appendIntDec(int32_t num) {
+  int flags = 0;
+  long n = num;
+  if (n < 0) {
+    n = -n;
+    flags |= FLAG_NEG;
+  }
+  return cvt_l(n, -1, -1, 10, TYPE_INTN, flags, hex);
+}
+
+bool mozilla::PrintfTarget::appendIntDec(uint32_t num) {
+  return cvt_l(num, -1, -1, 10, TYPE_UINTN, 0, hex);
+}
+
+bool mozilla::PrintfTarget::appendIntOct(uint32_t num) {
+  return cvt_l(num, -1, -1, 8, TYPE_UINTN, 0, hex);
+}
+
+bool mozilla::PrintfTarget::appendIntHex(uint32_t num) {
+  return cvt_l(num, -1, -1, 16, TYPE_UINTN, 0, hex);
+}
+
+bool mozilla::PrintfTarget::appendIntDec(int64_t num) {
+  int flags = 0;
+  if (num < 0) {
+    num = -num;
+    flags |= FLAG_NEG;
+  }
+  return cvt_ll(num, -1, -1, 10, TYPE_INTN, flags, hex);
+}
+
+bool mozilla::PrintfTarget::appendIntDec(uint64_t num) {
+  return cvt_ll(num, -1, -1, 10, TYPE_UINTN, 0, hex);
+}
+
+bool mozilla::PrintfTarget::appendIntOct(uint64_t num) {
+  return cvt_ll(num, -1, -1, 8, TYPE_UINTN, 0, hex);
+}
+
+bool mozilla::PrintfTarget::appendIntHex(uint64_t num) {
+  return cvt_ll(num, -1, -1, 16, TYPE_UINTN, 0, hex);
+}
+
 /* Convert a long into its printable form. */
 bool mozilla::PrintfTarget::cvt_l(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
@@ -557,18 +609,16 @@ bool mozilla::PrintfTarget::vprint(const
     const char* s;
     int* ip;
     void* p;
 #if defined(XP_WIN)
     const wchar_t* ws;
 #endif
   } u;
   const char* fmt0;
-  static const char hex[] = "0123456789abcdef";
-  static const char HEX[] = "0123456789ABCDEF";
   const char* hexp;
   int i;
   char pattern[20];
   const char* dolPt = nullptr;  // in "%4$.2f", dolPt will point to '.'
 
   // Build an argument array, IF the fmt is numbered argument
   // list style, to contain the Numbered Argument list pointers.
 
--- a/mozglue/misc/Printf.h
+++ b/mozglue/misc/Printf.h
@@ -71,16 +71,28 @@ namespace mozilla {
 class PrintfTarget {
  public:
   /* The Printf-like interface.  */
   bool MFBT_API print(const char* format, ...) MOZ_FORMAT_PRINTF(2, 3);
 
   /* The Vprintf-like interface.  */
   bool MFBT_API vprint(const char* format, va_list) MOZ_FORMAT_PRINTF(2, 0);
 
+  /* Fast paths for formatting integers as though by %d, %o, %u, or %x.
+     Since octal and hex formatting always treat numbers as unsigned, there
+     are no signed overloads for AppendInt{Oct,Hex}.  */
+  bool MFBT_API appendIntDec(int32_t);
+  bool MFBT_API appendIntDec(uint32_t);
+  bool MFBT_API appendIntOct(uint32_t);
+  bool MFBT_API appendIntHex(uint32_t);
+  bool MFBT_API appendIntDec(int64_t);
+  bool MFBT_API appendIntDec(uint64_t);
+  bool MFBT_API appendIntOct(uint64_t);
+  bool MFBT_API appendIntHex(uint64_t);
+
  protected:
   MFBT_API PrintfTarget();
   virtual ~PrintfTarget() {}
 
   /* Subclasses override this.  It is called when more output is
      available.  It may be called with len==0.  This should return
      true on success, or false on failure.  */
   virtual bool append(const char* sp, size_t len) = 0;
--- a/xpcom/string/nsTSubstring.cpp
+++ b/xpcom/string/nsTSubstring.cpp
@@ -1216,16 +1216,88 @@ template <typename T>
 void nsTSubstring<T>::AppendPrintf(const char* aFormat, va_list aAp) {
   PrintfAppend<T> appender(this);
   bool r = appender.vprint(aFormat, aAp);
   if (!r) {
     MOZ_CRASH("Allocation or other failure in PrintfTarget::print");
   }
 }
 
+template <typename T>
+void nsTSubstring<T>::AppendIntDec(int32_t aInteger) {
+  PrintfAppend<T> appender(this);
+  bool r = appender.appendIntDec(aInteger);
+  if (MOZ_UNLIKELY(!r)) {
+    MOZ_CRASH("Allocation or other failure while appending integers");
+  }
+}
+
+template <typename T>
+void nsTSubstring<T>::AppendIntDec(uint32_t aInteger) {
+  PrintfAppend<T> appender(this);
+  bool r = appender.appendIntDec(aInteger);
+  if (MOZ_UNLIKELY(!r)) {
+    MOZ_CRASH("Allocation or other failure while appending integers");
+  }
+}
+
+template <typename T>
+void nsTSubstring<T>::AppendIntOct(uint32_t aInteger) {
+  PrintfAppend<T> appender(this);
+  bool r = appender.appendIntOct(aInteger);
+  if (MOZ_UNLIKELY(!r)) {
+    MOZ_CRASH("Allocation or other failure while appending integers");
+  }
+}
+
+template <typename T>
+void nsTSubstring<T>::AppendIntHex(uint32_t aInteger) {
+  PrintfAppend<T> appender(this);
+  bool r = appender.appendIntHex(aInteger);
+  if (MOZ_UNLIKELY(!r)) {
+    MOZ_CRASH("Allocation or other failure while appending integers");
+  }
+}
+
+template <typename T>
+void nsTSubstring<T>::AppendIntDec(int64_t aInteger) {
+  PrintfAppend<T> appender(this);
+  bool r = appender.appendIntDec(aInteger);
+  if (MOZ_UNLIKELY(!r)) {
+    MOZ_CRASH("Allocation or other failure while appending integers");
+  }
+}
+
+template <typename T>
+void nsTSubstring<T>::AppendIntDec(uint64_t aInteger) {
+  PrintfAppend<T> appender(this);
+  bool r = appender.appendIntDec(aInteger);
+  if (MOZ_UNLIKELY(!r)) {
+    MOZ_CRASH("Allocation or other failure while appending integers");
+  }
+}
+
+template <typename T>
+void nsTSubstring<T>::AppendIntOct(uint64_t aInteger) {
+  PrintfAppend<T> appender(this);
+  bool r = appender.appendIntOct(aInteger);
+  if (MOZ_UNLIKELY(!r)) {
+    MOZ_CRASH("Allocation or other failure while appending integers");
+  }
+}
+
+template <typename T>
+void nsTSubstring<T>::AppendIntHex(uint64_t aInteger) {
+  PrintfAppend<T> appender(this);
+  bool r = appender.appendIntHex(aInteger);
+  if (MOZ_UNLIKELY(!r)) {
+    MOZ_CRASH("Allocation or other failure while appending integers");
+  }
+}
+
 // Returns the length of the formatted aDouble in aBuf.
 static int FormatWithoutTrailingZeros(char (&aBuf)[40], double aDouble,
                                       int aPrecision) {
   static const DoubleToStringConverter converter(
       DoubleToStringConverter::UNIQUE_ZERO |
           DoubleToStringConverter::EMIT_POSITIVE_EXPONENT_SIGN,
       "Infinity", "NaN", 'e', -6, 21, 6, 1);
   double_conversion::StringBuilder builder(aBuf, sizeof(aBuf));
--- a/xpcom/string/nsTSubstring.h
+++ b/xpcom/string/nsTSubstring.h
@@ -651,47 +651,68 @@ class nsTSubstring : public mozilla::det
   /**
    * Append a formatted string to the current string. Uses the
    * standard printf format codes.  This uses NSPR formatting, which will be
    * locale-aware for floating-point values.  You probably don't want to use
    * this with floating-point values as a result.
    */
   void AppendPrintf(const char* aFormat, ...) MOZ_FORMAT_PRINTF(2, 3);
   void AppendPrintf(const char* aFormat, va_list aAp) MOZ_FORMAT_PRINTF(2, 0);
-  void AppendInt(int32_t aInteger) { AppendPrintf("%" PRId32, aInteger); }
+  void AppendInt(int32_t aInteger) { AppendIntDec(aInteger); }
   void AppendInt(int32_t aInteger, int aRadix) {
     if (aRadix == 10) {
-      AppendPrintf("%" PRId32, aInteger);
+      AppendIntDec(aInteger);
+    } else if (aRadix == 8) {
+      AppendIntOct(static_cast<uint32_t>(aInteger));
     } else {
-      AppendPrintf(aRadix == 8 ? "%" PRIo32 : "%" PRIx32,
-                   static_cast<uint32_t>(aInteger));
+      AppendIntHex(static_cast<uint32_t>(aInteger));
+    }
+  }
+  void AppendInt(uint32_t aInteger) { AppendIntDec(aInteger); }
+  void AppendInt(uint32_t aInteger, int aRadix) {
+    if (aRadix == 10) {
+      AppendIntDec(aInteger);
+    } else if (aRadix == 8) {
+      AppendIntOct(aInteger);
+    } else {
+      AppendIntHex(aInteger);
     }
   }
-  void AppendInt(uint32_t aInteger) { AppendPrintf("%" PRIu32, aInteger); }
-  void AppendInt(uint32_t aInteger, int aRadix) {
-    AppendPrintf(
-        aRadix == 10 ? "%" PRIu32 : aRadix == 8 ? "%" PRIo32 : "%" PRIx32,
-        aInteger);
-  }
-  void AppendInt(int64_t aInteger) { AppendPrintf("%" PRId64, aInteger); }
+  void AppendInt(int64_t aInteger) { AppendIntDec(aInteger); }
   void AppendInt(int64_t aInteger, int aRadix) {
     if (aRadix == 10) {
-      AppendPrintf("%" PRId64, aInteger);
+      AppendIntDec(aInteger);
+    } else if (aRadix == 8) {
+      AppendIntOct(static_cast<uint64_t>(aInteger));
     } else {
-      AppendPrintf(aRadix == 8 ? "%" PRIo64 : "%" PRIx64,
-                   static_cast<uint64_t>(aInteger));
+      AppendIntHex(static_cast<uint64_t>(aInteger));
     }
   }
-  void AppendInt(uint64_t aInteger) { AppendPrintf("%" PRIu64, aInteger); }
+  void AppendInt(uint64_t aInteger) { AppendIntDec(aInteger); }
   void AppendInt(uint64_t aInteger, int aRadix) {
-    AppendPrintf(
-        aRadix == 10 ? "%" PRIu64 : aRadix == 8 ? "%" PRIo64 : "%" PRIx64,
-        aInteger);
+    if (aRadix == 10) {
+      AppendIntDec(aInteger);
+    } else if (aRadix == 8) {
+      AppendIntOct(aInteger);
+    } else {
+      AppendIntHex(aInteger);
+    }
   }
 
+private:
+  void AppendIntDec(int32_t);
+  void AppendIntDec(uint32_t);
+  void AppendIntOct(uint32_t);
+  void AppendIntHex(uint32_t);
+  void AppendIntDec(int64_t);
+  void AppendIntDec(uint64_t);
+  void AppendIntOct(uint64_t);
+  void AppendIntHex(uint64_t);
+
+public:
   /**
    * Append the given float to this string
    */
   void NS_FASTCALL AppendFloat(float aFloat);
   void NS_FASTCALL AppendFloat(double aFloat);
 
   self_type& operator+=(char_type aChar) {
     Append(aChar);