Bug 1565176 - Part 1: Prefer UFormattedValue API when available. r=jwalden
authorAndré Bargull <andre.bargull@gmail.com>
Fri, 19 Jul 2019 11:45:02 +0000
changeset 483476 abeee4a000ccc049a7404f2524f0faeec762ecbe
parent 483474 eb7f4d56f54b3283fc15983ee859b5e62fcb9f3b
child 483477 0f7682e2f00109c49c39c02a5ac886e6358c2676
push id36321
push usermalexandru@mozilla.com
push dateFri, 19 Jul 2019 21:56:14 +0000
treeherdermozilla-central@23d4ebd5f8e7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden
bugs1565176
milestone70.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 1565176 - Part 1: Prefer UFormattedValue API when available. r=jwalden Instead of first collecting all formatted-number field positions into UFieldPositionIterator and then iterating over the position iterator, the new UFormattedValue API allows direct iteration, resulting in a slight performance improvement. And using `ufmtval_getString` will additionally allow us to access the underlying formatted string directly, which saves us a copy via `unumf_resultToString`, leading to another minor performance improvement. Differential Revision: https://phabricator.services.mozilla.com/D37698
js/src/builtin/intl/ICUStubs.h
js/src/builtin/intl/NumberFormat.cpp
js/src/builtin/intl/NumberFormat.h
js/src/builtin/intl/RelativeTimeFormat.cpp
--- a/js/src/builtin/intl/ICUStubs.h
+++ b/js/src/builtin/intl/ICUStubs.h
@@ -223,16 +223,23 @@ inline void unumf_closeResult(UFormatted
   MOZ_CRASH("unumf_closeResult: Intl API disabled");
 }
 
 inline void unumf_formatDouble(const UNumberFormatter* uformatter, double value,
                                UFormattedNumber* uresult, UErrorCode* status) {
   MOZ_CRASH("unumf_formatDouble: Intl API disabled");
 }
 
+struct UFormattedValue;
+
+inline const UFormattedValue* unumf_resultAsValue(
+    const UFormattedNumber* uresult, UErrorCode* status) {
+  MOZ_CRASH("unumf_resultAsValue: Intl API disabled");
+}
+
 inline void unumf_resultGetAllFieldPositions(const UFormattedNumber* uresult,
                                              UFieldPositionIterator* ufpositer,
                                              UErrorCode* status) {
   MOZ_CRASH("unumf_resultGetAllFieldPositions: Intl API disabled");
 }
 
 inline int32_t unumf_resultToString(const UFormattedNumber* uresult,
                                     UChar* buffer, int32_t bufferCapacity,
--- a/js/src/builtin/intl/NumberFormat.cpp
+++ b/js/src/builtin/intl/NumberFormat.cpp
@@ -4,48 +4,52 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* Intl.NumberFormat implementation. */
 
 #include "builtin/intl/NumberFormat.h"
 
 #include "mozilla/Assertions.h"
+#include "mozilla/Casting.h"
 #include "mozilla/FloatingPoint.h"
 
 #include <algorithm>
 #include <stddef.h>
 #include <stdint.h>
+#include <type_traits>
 
 #include "builtin/intl/CommonFunctions.h"
 #include "builtin/intl/ICUStubs.h"
 #include "builtin/intl/ScopedICUObject.h"
 #include "ds/Sort.h"
 #include "gc/FreeOp.h"
 #include "js/CharacterEncoding.h"
 #include "js/PropertySpec.h"
 #include "js/RootingAPI.h"
 #include "js/StableStringChars.h"
 #include "js/TypeDecls.h"
+#include "js/Vector.h"
 #include "vm/JSContext.h"
 #include "vm/SelfHosting.h"
 #include "vm/Stack.h"
 
 #include "vm/JSObject-inl.h"
 
 using namespace js;
 
 using mozilla::AssertedCast;
 using mozilla::IsFinite;
 using mozilla::IsNaN;
 using mozilla::IsNegative;
 using mozilla::SpecificNaN;
 
 using js::intl::CallICU;
 using js::intl::DateTimeFormatOptions;
+using js::intl::FieldType;
 using js::intl::GetAvailableLocales;
 using js::intl::IcuLocale;
 
 using JS::AutoStableStringChars;
 
 const ClassOps NumberFormatObject::classOps_ = {nullptr, /* addProperty */
                                                 nullptr, /* delProperty */
                                                 nullptr, /* enumerate */
@@ -465,53 +469,103 @@ static UFormattedNumber* NewUFormattedNu
   UFormattedNumber* formatted = unumf_openResult(&status);
   if (U_FAILURE(status)) {
     intl::ReportInternalError(cx);
     return nullptr;
   }
   return formatted;
 }
 
-static JSString* PartitionNumberPattern(JSContext* cx, UNumberFormatter* nf,
-                                        UFormattedNumber* formatted,
-                                        double* x) {
+// We also support UFormattedNumber in addition to UFormattedValue, in case
+// we're compiling against a system ICU which doesn't expose draft APIs.
+
+#ifndef U_HIDE_DRAFT_API
+using PartitionNumberPatternResult = const UFormattedValue*;
+#else
+using PartitionNumberPatternResult = const UFormattedNumber*;
+#endif
+
+static PartitionNumberPatternResult PartitionNumberPattern(
+    JSContext* cx, const UNumberFormatter* nf, UFormattedNumber* formatted,
+    double* x) {
   // ICU incorrectly formats NaN values with the sign bit set, as if they
   // were negative.  Replace all NaNs with a single pattern with sign bit
   // unset ("positive", that is) until ICU is fixed.
   if (MOZ_UNLIKELY(IsNaN(*x))) {
     *x = SpecificNaN<double>(0, 1);
   }
 
   UErrorCode status = U_ZERO_ERROR;
   unumf_formatDouble(nf, *x, formatted, &status);
   if (U_FAILURE(status)) {
     intl::ReportInternalError(cx);
     return nullptr;
   }
 
+#ifndef U_HIDE_DRAFT_API
+  const UFormattedValue* formattedValue =
+      unumf_resultAsValue(formatted, &status);
+  if (U_FAILURE(status)) {
+    intl::ReportInternalError(cx);
+    return nullptr;
+  }
+  return formattedValue;
+#else
+  return formatted;
+#endif
+}
+
+static JSString* FormattedNumberToString(
+    JSContext* cx, PartitionNumberPatternResult formattedValue) {
+#ifndef U_HIDE_DRAFT_API
+  static_assert(
+      std::is_same<PartitionNumberPatternResult, const UFormattedValue*>::value,
+      "UFormattedValue arm");
+
+  UErrorCode status = U_ZERO_ERROR;
+  int32_t strLength;
+  const char16_t* str = ufmtval_getString(formattedValue, &strLength, &status);
+  if (U_FAILURE(status)) {
+    intl::ReportInternalError(cx);
+    return nullptr;
+  }
+
+  return NewStringCopyN<CanGC>(cx, str, AssertedCast<uint32_t>(strLength));
+#else
+  static_assert(std::is_same<PartitionNumberPatternResult,
+                             const UFormattedNumber*>::value,
+                "UFormattedNumber arm");
+
   return CallICU(cx,
                  [formatted](UChar* chars, int32_t size, UErrorCode* status) {
                    return unumf_resultToString(formatted, chars, size, status);
                  });
+#endif
 }
 
-static bool intl_FormatNumber(JSContext* cx, UNumberFormatter* nf,
+static bool intl_FormatNumber(JSContext* cx, const UNumberFormatter* nf,
                               UFormattedNumber* formatted, double x,
                               MutableHandleValue result) {
-  JSString* str = PartitionNumberPattern(cx, nf, formatted, &x);
+  PartitionNumberPatternResult formattedValue =
+      PartitionNumberPattern(cx, nf, formatted, &x);
+  if (!formattedValue) {
+    return false;
+  }
+
+  JSString* str = FormattedNumberToString(cx, formattedValue);
   if (!str) {
     return false;
   }
 
   result.setString(str);
   return true;
 }
 
-static intl::FieldType GetFieldTypeForNumberField(UNumberFormatFields fieldName,
-                                                  double d) {
+static FieldType GetFieldTypeForNumberField(UNumberFormatFields fieldName,
+                                            double d) {
   // See intl/icu/source/i18n/unicode/unum.h for a detailed field list.  This
   // list is deliberately exhaustive: cases might have to be added/removed if
   // this code is compiled with a different ICU with more UNumberFormatFields
   // enum initializers.  Please guard such cases with appropriate ICU
   // version-testing #ifdefs, should cross-version divergence occur.
   switch (fieldName) {
     case UNUM_INTEGER_FIELD:
       if (IsNaN(d)) {
@@ -591,30 +645,58 @@ static intl::FieldType GetFieldTypeForNu
   }
 
   MOZ_ASSERT_UNREACHABLE(
       "unenumerated, undocumented format field returned "
       "by iterator");
   return nullptr;
 }
 
-bool js::intl::NumberFormatFields::append(int32_t field, int32_t begin,
-                                          int32_t end) {
+struct Field {
+  uint32_t begin;
+  uint32_t end;
+  FieldType type;
+
+  // Needed for vector-resizing scratch space.
+  Field() = default;
+
+  Field(uint32_t begin, uint32_t end, FieldType type)
+      : begin(begin), end(end), type(type) {}
+};
+
+class NumberFormatFields {
+  using FieldsVector = Vector<Field, 16>;
+
+  FieldsVector fields_;
+  double number_;
+
+ public:
+  NumberFormatFields(JSContext* cx, double number)
+      : fields_(cx), number_(number) {}
+
+  MOZ_MUST_USE bool append(int32_t field, int32_t begin, int32_t end);
+
+  MOZ_MUST_USE ArrayObject* toArray(JSContext* cx,
+                                    JS::HandleString overallResult,
+                                    FieldType unitType);
+};
+
+bool NumberFormatFields::append(int32_t field, int32_t begin, int32_t end) {
   MOZ_ASSERT(begin >= 0);
   MOZ_ASSERT(end >= 0);
   MOZ_ASSERT(begin < end, "erm, aren't fields always non-empty?");
 
   FieldType type =
       GetFieldTypeForNumberField(UNumberFormatFields(field), number_);
   return fields_.emplaceBack(uint32_t(begin), uint32_t(end), type);
 }
 
-ArrayObject* js::intl::NumberFormatFields::toArray(JSContext* cx,
-                                                   HandleString overallResult,
-                                                   FieldType unitType) {
+ArrayObject* NumberFormatFields::toArray(JSContext* cx,
+                                         HandleString overallResult,
+                                         FieldType unitType) {
   // Merge sort the fields vector.  Expand the vector to have scratch space for
   // performing the sort.
   size_t fieldsLen = fields_.length();
   if (!fields_.resizeUninitialized(fieldsLen * 2)) {
     return nullptr;
   }
 
   MOZ_ALWAYS_TRUE(MergeSort(
@@ -899,20 +981,86 @@ ArrayObject* js::intl::NumberFormatField
   } while (true);
 
   MOZ_ASSERT(lastEndIndex == overallResult->length(),
              "result array must partition the entire string");
 
   return partsArray;
 }
 
-static bool intl_FormatNumberToParts(JSContext* cx, UNumberFormatter* nf,
-                                     UFormattedNumber* formatted, double x,
-                                     MutableHandleValue result) {
-  RootedString overallResult(cx, PartitionNumberPattern(cx, nf, formatted, &x));
+#ifndef U_HIDE_DRAFT_API
+bool js::intl::FormattedNumberToParts(JSContext* cx,
+                                      const UFormattedValue* formattedValue,
+                                      double number, FieldType unitType,
+                                      MutableHandleValue result) {
+  RootedString overallResult(cx, FormattedNumberToString(cx, formattedValue));
+  if (!overallResult) {
+    return false;
+  }
+
+  UErrorCode status = U_ZERO_ERROR;
+  UConstrainedFieldPosition* fpos = ucfpos_open(&status);
+  if (U_FAILURE(status)) {
+    intl::ReportInternalError(cx);
+    return false;
+  }
+  ScopedICUObject<UConstrainedFieldPosition, ucfpos_close> toCloseFpos(fpos);
+
+  // We're only interested in UFIELD_CATEGORY_NUMBER fields.
+  ucfpos_constrainCategory(fpos, UFIELD_CATEGORY_NUMBER, &status);
+  if (U_FAILURE(status)) {
+    intl::ReportInternalError(cx);
+    return false;
+  }
+
+  // Vacuum up fields in the overall formatted string.
+
+  NumberFormatFields fields(cx, number);
+
+  while (true) {
+    bool hasMore = ufmtval_nextPosition(formattedValue, fpos, &status);
+    if (U_FAILURE(status)) {
+      intl::ReportInternalError(cx);
+      return false;
+    }
+    if (!hasMore) {
+      break;
+    }
+
+    int32_t field = ucfpos_getField(fpos, &status);
+    if (U_FAILURE(status)) {
+      intl::ReportInternalError(cx);
+      return false;
+    }
+
+    int32_t beginIndex, endIndex;
+    ucfpos_getIndexes(fpos, &beginIndex, &endIndex, &status);
+    if (U_FAILURE(status)) {
+      intl::ReportInternalError(cx);
+      return false;
+    }
+
+    if (!fields.append(field, beginIndex, endIndex)) {
+      return false;
+    }
+  }
+
+  ArrayObject* array = fields.toArray(cx, overallResult, unitType);
+  if (!array) {
+    return false;
+  }
+
+  result.setObject(*array);
+  return true;
+}
+#else
+static ArrayObject* LegacyFormattedNumberToParts(
+    JSContext* cx, const UFormattedNumber* formatted, double x,
+    MutableHandleValue result) {
+  RootedString overallResult(cx, FormattedNumberToString(cx, formatted));
   if (!overallResult) {
     return false;
   }
 
   UErrorCode status = U_ZERO_ERROR;
   UFieldPositionIterator* fpositer = ufieldpositer_open(&status);
   if (U_FAILURE(status)) {
     intl::ReportInternalError(cx);
@@ -926,33 +1074,50 @@ static bool intl_FormatNumberToParts(JSC
   unumf_resultGetAllFieldPositions(formatted, fpositer, &status);
   if (U_FAILURE(status)) {
     intl::ReportInternalError(cx);
     return false;
   }
 
   // Vacuum up fields in the overall formatted string.
 
-  intl::NumberFormatFields fields(cx, x);
+  NumberFormatFields fields(cx, x);
 
   int32_t field, beginIndex, endIndex;
   while ((field = ufieldpositer_next(fpositer, &beginIndex, &endIndex)) >= 0) {
     if (!fields.append(field, beginIndex, endIndex)) {
       return false;
     }
   }
 
   ArrayObject* array = fields.toArray(cx, overallResult, nullptr);
   if (!array) {
     return false;
   }
 
   result.setObject(*array);
   return true;
 }
+#endif
+
+static bool intl_FormatNumberToParts(JSContext* cx, const UNumberFormatter* nf,
+                                     UFormattedNumber* formatted, double x,
+                                     MutableHandleValue result) {
+  PartitionNumberPatternResult formattedValue =
+      PartitionNumberPattern(cx, nf, formatted, &x);
+  if (!formattedValue) {
+    return false;
+  }
+
+#ifndef U_HIDE_DRAFT_API
+  return intl::FormattedNumberToParts(cx, formattedValue, x, nullptr, result);
+#else
+  return LegacyFormattedNumberToParts(cx, formattedValue, x, result);
+#endif
+}
 
 bool js::intl_FormatNumber(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   MOZ_ASSERT(args.length() == 3);
   MOZ_ASSERT(args[0].isObject());
   MOZ_ASSERT(args[1].isNumber());
   MOZ_ASSERT(args[2].isBoolean());
 
--- a/js/src/builtin/intl/NumberFormat.h
+++ b/js/src/builtin/intl/NumberFormat.h
@@ -9,21 +9,21 @@
 
 #include "mozilla/Attributes.h"
 
 #include <stdint.h>
 
 #include "builtin/SelfHostingDefines.h"
 #include "gc/Barrier.h"
 #include "js/Class.h"
-#include "js/Vector.h"
 #include "vm/NativeObject.h"
 #include "vm/Runtime.h"
 
 struct UFormattedNumber;
+struct UFormattedValue;
 struct UNumberFormatter;
 
 namespace js {
 
 class ArrayObject;
 class FreeOp;
 
 class NumberFormatObject : public NativeObject {
@@ -212,41 +212,19 @@ class MOZ_STACK_CLASS NumberFormatterSke
    *
    * https://github.com/unicode-org/icu/blob/master/docs/userguide/format_parse/numbers/skeletons.md#rounding-mode
    */
   MOZ_MUST_USE bool roundingModeHalfUp();
 };
 
 using FieldType = js::ImmutablePropertyNamePtr JSAtomState::*;
 
-struct Field {
-  uint32_t begin;
-  uint32_t end;
-  FieldType type;
-
-  // Needed for vector-resizing scratch space.
-  Field() = default;
-
-  Field(uint32_t begin, uint32_t end, FieldType type)
-      : begin(begin), end(end), type(type) {}
-};
-
-class NumberFormatFields {
-  using FieldsVector = Vector<Field, 16>;
-
-  FieldsVector fields_;
-  double number_;
-
- public:
-  NumberFormatFields(JSContext* cx, double number)
-      : fields_(cx), number_(number) {}
-
-  MOZ_MUST_USE bool append(int32_t field, int32_t begin, int32_t end);
-
-  MOZ_MUST_USE ArrayObject* toArray(JSContext* cx,
-                                    JS::HandleString overallResult,
-                                    FieldType unitType);
-};
+#ifndef U_HIDE_DRAFT_API
+MOZ_MUST_USE bool FormattedNumberToParts(JSContext* cx,
+                                         const UFormattedValue* formattedValue,
+                                         double number, FieldType unitType,
+                                         MutableHandleValue result);
+#endif
 
 }  // namespace intl
 }  // namespace js
 
 #endif /* builtin_intl_NumberFormat_h */
--- a/js/src/builtin/intl/RelativeTimeFormat.cpp
+++ b/js/src/builtin/intl/RelativeTimeFormat.cpp
@@ -309,30 +309,16 @@ static bool intl_FormatToPartsRelativeTi
 
   const UFormattedValue* formattedValue =
       ureldatefmt_resultAsValue(formatted, &status);
   if (U_FAILURE(status)) {
     intl::ReportInternalError(cx);
     return false;
   }
 
-  int32_t strLength;
-  const char16_t* str = ufmtval_getString(formattedValue, &strLength, &status);
-  if (U_FAILURE(status)) {
-    intl::ReportInternalError(cx);
-    return false;
-  }
-  MOZ_ASSERT(strLength >= 0);
-
-  RootedString overallResult(cx,
-                             NewStringCopyN<CanGC>(cx, str, size_t(strLength)));
-  if (!overallResult) {
-    return false;
-  }
-
   intl::FieldType unitType;
   switch (unit) {
     case UDAT_REL_UNIT_SECOND:
       unitType = &JSAtomState::second;
       break;
     case UDAT_REL_UNIT_MINUTE:
       unitType = &JSAtomState::minute;
       break;
@@ -353,69 +339,17 @@ static bool intl_FormatToPartsRelativeTi
       break;
     case UDAT_REL_UNIT_YEAR:
       unitType = &JSAtomState::year;
       break;
     default:
       MOZ_CRASH("unexpected relative time unit");
   }
 
-  UConstrainedFieldPosition* fpos = ucfpos_open(&status);
-  if (U_FAILURE(status)) {
-    intl::ReportInternalError(cx);
-    return false;
-  }
-  ScopedICUObject<UConstrainedFieldPosition, ucfpos_close> toCloseFpos(fpos);
-
-  // The possible field position categories are UFIELD_CATEGORY_NUMBER and
-  // UFIELD_CATEGORY_RELATIVE_DATETIME. For the parts array we only need to
-  // iterate over the number formatted fields.
-  ucfpos_constrainCategory(fpos, UFIELD_CATEGORY_NUMBER, &status);
-  if (U_FAILURE(status)) {
-    intl::ReportInternalError(cx);
-    return false;
-  }
-
-  intl::NumberFormatFields fields(cx, t);
-
-  while (true) {
-    bool hasMore = ufmtval_nextPosition(formattedValue, fpos, &status);
-    if (U_FAILURE(status)) {
-      intl::ReportInternalError(cx);
-      return false;
-    }
-    if (!hasMore) {
-      break;
-    }
-
-    int32_t field = ucfpos_getField(fpos, &status);
-    if (U_FAILURE(status)) {
-      intl::ReportInternalError(cx);
-      return false;
-    }
-
-    int32_t beginIndex, endIndex;
-    ucfpos_getIndexes(fpos, &beginIndex, &endIndex, &status);
-    if (U_FAILURE(status)) {
-      intl::ReportInternalError(cx);
-      return false;
-    }
-
-    if (!fields.append(field, beginIndex, endIndex)) {
-      return false;
-    }
-  }
-
-  ArrayObject* array = fields.toArray(cx, overallResult, unitType);
-  if (!array) {
-    return false;
-  }
-
-  result.setObject(*array);
-  return true;
+  return intl::FormattedNumberToParts(cx, formattedValue, t, unitType, result);
 }
 #endif
 
 bool js::intl_FormatRelativeTime(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   MOZ_ASSERT(args.length() == 5);
 
   Rooted<RelativeTimeFormatObject*> relativeTimeFormat(cx);