Bug 1499906 - Use toLocaleString(). r=erahm
☠☠ backed out by be3ea7eb7ff4 ☠ ☠
authorNicholas Nethercote <nnethercote@mozilla.com>
Fri, 19 Oct 2018 11:19:32 +1100
changeset 500564 9819dbed2d88c0aa7f4832a0cee383e68481ebd8
parent 500563 97348d7aa09eace30cab2b769754bfc334f750f7
child 500565 eeea7ba794c30def2083f1a7555228b84d7b4626
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm
bugs1499906
milestone64.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 1499906 - Use toLocaleString(). r=erahm This replaces a bunch of code that inserted separators by hand. For now I've kept the output mostly the same by forcing the locale to en-US. But at least now we could consider localizing the output. The places where the output is different, it's more consistent with the new code. E.g. printing "-05.55%" (which matches "05.55%") instead of "-5.55%".
toolkit/components/aboutmemory/content/aboutMemory.js
toolkit/components/aboutmemory/tests/test_aboutmemory.xul
toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
--- a/toolkit/components/aboutmemory/content/aboutMemory.js
+++ b/toolkit/components/aboutmemory/content/aboutMemory.js
@@ -1122,17 +1122,17 @@ TreeNode.prototype = {
     this._maxAbsDescendant = max;
     return max;
   },
 
   toString() {
     switch (this._units) {
       case UNITS_BYTES: return formatBytes(this._amount);
       case UNITS_COUNT:
-      case UNITS_COUNT_CUMULATIVE: return formatInt(this._amount);
+      case UNITS_COUNT_CUMULATIVE: return formatNum(this._amount);
       case UNITS_PERCENTAGE: return formatPercentage(this._amount);
       default:
         throw "Invalid memory report(s): bad units in TreeNode.toString";
     }
   },
 };
 
 // Sort TreeNodes first by size, then by name.  The latter is important for the
@@ -1500,107 +1500,103 @@ function appendProcessAboutMemoryElement
     appendWarningElements(warningsDiv, hasKnownHeapAllocated,
                           aHasMozMallocUsableSize);
   }
 
   appendElementWithText(aP, "h3", "", "End of " + aProcess);
   appendLink("end", "start", "↑");
 }
 
-/**
- * Determines if a number has a negative sign when converted to a string.
- * Works even for -0.
- *
- * @param aN
- *        The number.
- * @return A boolean.
- */
-function hasNegativeSign(aN) {
-  if (aN === 0) { // this succeeds for 0 and -0
-    return 1 / aN === -Infinity; // this succeeds for -0
-  }
-  return aN < 0;
-}
+// Used for UNITS_BYTES values that are printed as MiB.
+const kMBStyle = {
+  minimumFractionDigits: 2,
+  maximumFractionDigits: 2
+};
+
+// Used for UNITS_PERCENTAGE values.
+const kPercStyle = {
+  style: "percent",
+  minimumFractionDigits: 2,
+  maximumFractionDigits: 2
+};
+
+// Used for fractions within the tree.
+const kFracStyle = {
+  style: "percent",
+  minimumIntegerDigits: 2,
+  minimumFractionDigits: 2,
+  maximumFractionDigits: 2
+};
+
+// Used for special-casing 100% fractions within the tree.
+const kFrac1Style = {
+  style: "percent",
+  minimumIntegerDigits: 3,
+  minimumFractionDigits: 1,
+  maximumFractionDigits: 1
+};
 
 /**
  * Formats an int as a human-readable string.
  *
  * @param aN
  *        The integer to format.
- * @param aExtra
- *        An extra string to tack onto the end.
+ * @param aOptions
+ *        Optional options object.
  * @return A human-readable string representing the int.
- *
- * Note: building an array of chars and converting that to a string with
- * Array.join at the end is more memory efficient than using string
- * concatenation.  See bug 722972 for details.
  */
-function formatInt(aN, aExtra) {
-  let neg = false;
-  if (hasNegativeSign(aN)) {
-    neg = true;
-    aN = -aN;
-  }
-  let s = [];
-  while (true) {
-    let k = aN % 1000;
-    aN = Math.floor(aN / 1000);
-    if (aN > 0) {
-      if (k < 10) {
-        s.unshift(",00", k);
-      } else if (k < 100) {
-        s.unshift(",0", k);
-      } else {
-        s.unshift(",", k);
-      }
-    } else {
-      s.unshift(k);
-      break;
-    }
-  }
-  if (neg) {
-    s.unshift("-");
-  }
-  if (aExtra) {
-    s.push(aExtra);
-  }
-  return s.join("");
+function formatNum(aN, aOptions) {
+  return aN.toLocaleString('en-US', aOptions);
 }
 
 /**
  * Converts a byte count to an appropriate string representation.
  *
  * @param aBytes
  *        The byte count.
  * @return The string representation.
  */
 function formatBytes(aBytes) {
-  let unit = gVerbose.checked ? " B" : " MB";
-
-  let s;
-  if (gVerbose.checked) {
-    s = formatInt(aBytes, unit);
-  } else {
-    let mbytes = (aBytes / (1024 * 1024)).toFixed(2);
-    let a = String(mbytes).split(".");
-    // If the argument to formatInt() is -0, it will print the negative sign.
-    s = formatInt(Number(a[0])) + "." + a[1] + unit;
-  }
-  return s;
+  return gVerbose.checked
+       ? `${formatNum(aBytes)} B`
+       : `${formatNum(aBytes / (1024 * 1024), kMBStyle)} MB`;
 }
 
 /**
- * Converts a percentage to an appropriate string representation.
+ * Converts a UNITS_PERCENTAGE value to an appropriate string representation.
  *
  * @param aPerc100x
  *        The percentage, multiplied by 100 (see nsIMemoryReporter).
  * @return The string representation
  */
 function formatPercentage(aPerc100x) {
-  return (aPerc100x / 100).toFixed(2) + "%";
+  // A percentage like 12.34% will have an aPerc100x value of 1234, and we need
+  // to divide that by 10,000 to get the 0.1234 that toLocaleString() wants.
+  return formatNum(aPerc100x / 10000, kPercStyle);
+}
+
+/*
+ * Converts a tree fraction to an appropriate string representation.
+ *
+ * @param aNum
+ *        The numerator.
+ * @param aDenom
+ *        The denominator.
+ * @return The string representation
+ */
+function formatTreeFrac(aNum, aDenom) {
+  // Two special behaviours here:
+  // - We treat 0 / 0 as 100%.
+  // - We want 4 digits, as much as possible, because it gives good vertical
+  //   alignment. For positive numbers, 00.00%--99.99% works straighforwardly,
+  //   but 100.0% needs special handling.
+  let num = aDenom === 0 ? 1 : (aNum / aDenom);
+  return (0.99995 <= num && num <= 1)
+         ? formatNum(1, kFrac1Style)
+         : formatNum(num, kFracStyle);
 }
 
 /**
  * Right-justifies a string in a field of a given width, padding as necessary.
  *
  * @param aS
  *        The string.
  * @param aN
@@ -1844,25 +1840,19 @@ function appendTreeElements(aP, aRoot, a
       d = aP;
     }
 
     // The value.
     appendElementWithText(d, "span", "mrValue" + (tIsInvalid ? " invalid" : ""),
                           valueText);
 
     // The percentage (omitted for single entries).
-    let percText;
     if (!aT._isDegenerate) {
-      // Treat 0 / 0 as 100%.
-      let num = aRoot._amount === 0 ? 100 : (100 * aT._amount / aRoot._amount);
-      let numText = num.toFixed(2);
-      percText = numText === "100.00"
-               ? " (100.0%)"
-               : (0 <= num && num < 10 ? " (0" : " (") + numText + "%)";
-      appendElementWithText(d, "span", "mrPerc", percText);
+      let percText = formatTreeFrac(aT._amount, aRoot._amount);
+      appendElementWithText(d, "span", "mrPerc", ` (${percText})`);
     }
 
     // The separator.
     appendElementWithText(d, "span", "mrSep", sep);
 
     // The entry's name.
     appendMrNameSpan(d, aT._description, aT._unsafeName,
                      tIsInvalid, aT._nMerged, aT._presence);
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
@@ -266,17 +266,17 @@ WARNING: the following values are negati
 \n\
 This indicates a defect in one or more memory reporters. The invalid values are highlighted.\n\
 Explicit Allocations\n\
 \n\
 98.00 MB (100.0%) -- explicit\n\
 ├──150.00 MB (153.06%) ── js/compartment(http://too-big.com/)/stuff [?!]\n\
 ├───5.00 MB (05.10%) ── ok\n\
 └──-57.00 MB (-58.16%) -- (2 tiny) [?!]\n\
-   ├───-2.00 MB (-2.04%) ── neg1 [?!]\n\
+   ├───-2.00 MB (-02.04%) ── neg1 [?!]\n\
    └──-55.00 MB (-56.12%) ── heap-unclassified [?!]\n\
 \n\
 Other Measurements\n\
 \n\
  100.00 MB ── heap-allocated\n\
   -0.00 MB ── other1 [?!]\n\
 -222.00 MB ── other2 [?!]\n\
       -333 ── other3 [?!]\n\
@@ -333,24 +333,24 @@ This indicates a defect in one or more m
 Explicit Allocations\n\
 \n\
 99.95 MB (100.0%) -- explicit\n\
 ├──99.00 MB (99.05%) ── big\n\
 └───0.95 MB (00.95%) -- (3 tiny)\n\
     ├──0.96 MB (00.96%) ── heap-unclassified\n\
     ├──0.01 MB (00.01%) -- a\n\
     │  ├──0.04 MB (00.04%) ── pos\n\
-    │  ├──-0.01 MB (-0.01%) ── neg2 [?!]\n\
-    │  └──-0.02 MB (-0.02%) ── neg1 [?!]\n\
-    └──-0.02 MB (-0.02%) -- b/c [?!]\n\
+    │  ├──-0.01 MB (-00.01%) ── neg2 [?!]\n\
+    │  └──-0.02 MB (-00.02%) ── neg1 [?!]\n\
+    └──-0.02 MB (-00.02%) -- b/c [?!]\n\
        ├───0.01 MB (00.01%) ── g/h\n\
        ├───0.00 MB (00.00%) ── i/j\n\
-       └──-0.04 MB (-0.04%) -- d [?!]\n\
+       └──-0.04 MB (-00.04%) -- d [?!]\n\
           ├───0.02 MB (00.02%) ── e\n\
-          └──-0.06 MB (-0.06%) ── f [?!]\n\
+          └──-0.06 MB (-00.06%) ── f [?!]\n\
 \n\
 Other Measurements\n\
 \n\
 100.00 MB ── heap-allocated\n\
 \n\
 End of 5th\n\
 ";
 
@@ -436,17 +436,17 @@ WARNING: the following values are negati
     other5 \n\
 \n\
 This indicates a defect in one or more memory reporters. The invalid values are highlighted.\n\
 Explicit Allocations\n\
 \n\
 102,760,448 B (100.0%) -- explicit\n\
 ├──157,286,400 B (153.06%) ── js/compartment(http://too-big.com/)/stuff [?!]\n\
 ├────5,242,880 B (05.10%) ── ok\n\
-├───-2,097,152 B (-2.04%) ── neg1 [?!]\n\
+├───-2,097,152 B (-02.04%) ── neg1 [?!]\n\
 └──-57,671,680 B (-56.12%) ── heap-unclassified [?!]\n\
 \n\
 Other Measurements\n\
 \n\
  104,857,600 B ── heap-allocated\n\
         -111 B ── other1 [?!]\n\
 -232,783,872 B ── other2 [?!]\n\
           -333 ── other3 [?!]\n\
@@ -502,24 +502,24 @@ WARNING: the following values are negati
 This indicates a defect in one or more memory reporters. The invalid values are highlighted.\n\
 Explicit Allocations\n\
 \n\
 104,801,280 B (100.0%) -- explicit\n\
 ├──103,809,024 B (99.05%) ── big\n\
 ├────1,007,616 B (00.96%) ── heap-unclassified\n\
 ├───────10,240 B (00.01%) -- a\n\
 │       ├──40,960 B (00.04%) ── pos\n\
-│       ├──-10,240 B (-0.01%) ── neg2 [?!]\n\
-│       └──-20,480 B (-0.02%) ── neg1 [?!]\n\
-└──────-25,600 B (-0.02%) -- b/c [?!]\n\
+│       ├──-10,240 B (-00.01%) ── neg2 [?!]\n\
+│       └──-20,480 B (-00.02%) ── neg1 [?!]\n\
+└──────-25,600 B (-00.02%) -- b/c [?!]\n\
        ├───10,240 B (00.01%) ── g/h\n\
        ├────5,120 B (00.00%) ── i/j\n\
-       └──-40,960 B (-0.04%) -- d [?!]\n\
+       └──-40,960 B (-00.04%) -- d [?!]\n\
           ├───20,480 B (00.02%) ── e\n\
-          └──-61,440 B (-0.06%) ── f [?!]\n\
+          └──-61,440 B (-00.06%) ── f [?!]\n\
 \n\
 Other Measurements\n\
 \n\
 104,857,600 B ── heap-allocated\n\
 \n\
 End of 5th\n\
 ";
 
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
@@ -358,37 +358,37 @@ Other Measurements\n\
 \n\
 -0.00 MB (100.0%) -- p8\n\
 └──-0.00 MB (100.0%) -- a\n\
    ├──-0.00 MB (50.00%) -- b\n\
    │  ├──-0.00 MB (31.82%) -- c\n\
    │  │  ├──-0.00 MB (18.18%) ── e [-]\n\
    │  │  └──-0.00 MB (13.64%) ── d [-]\n\
    │  ├──-0.00 MB (22.73%) ── f [-]\n\
-   │  └───0.00 MB (-4.55%) ── (fake child) [!]\n\
+   │  └───0.00 MB (-04.55%) ── (fake child) [!]\n\
    └──-0.00 MB (50.00%) -- g\n\
       ├──-0.00 MB (31.82%) ── i [-]\n\
       ├──-0.00 MB (27.27%) ── h [-]\n\
-      └───0.00 MB (-9.09%) ── (fake child) [!]\n\
+      └───0.00 MB (-09.09%) ── (fake child) [!]\n\
 \n\
 End of P8\n\
 ";
 
   // This is the output for a verbose diff.
   let expectedDiffVerbose =
 "\
 P\n\
 \n\
 WARNING: the 'heap-allocated' memory reporter does not work for this platform and/or configuration. This means that 'heap-unclassified' is not shown and the 'explicit' tree shows less memory than it should.\n\
 Explicit Allocations\n\
 \n\
 -10,005 B (100.0%) -- explicit\n\
 ├──-10,000 B (99.95%) ── storage/prefixset/goog-phish-shavar\n\
 ├───────-6 B (00.06%) ── spell-check [2]\n\
-└────────1 B (-0.01%) ── xpcom/category-manager\n\
+└────────1 B (-00.01%) ── xpcom/category-manager\n\
 \n\
 Other Measurements\n\
 \n\
 1,002,000 B (100.0%) -- a\n\
 ├──1,000,000 B (99.80%) ── b\n\
 ├──────1,000 B (00.10%) -- c\n\
 │      ├──-999,000 B (-99.70%) ── e\n\
 │      ├──998,000 B (99.60%) ── d\n\
@@ -456,21 +456,21 @@ Other Measurements\n\
 \n\
 -22 B (100.0%) -- p8\n\
 └──-22 B (100.0%) -- a\n\
    ├──-11 B (50.00%) -- b\n\
    │  ├───-7 B (31.82%) -- c\n\
    │  │   ├──-4 B (18.18%) ── e [-]\n\
    │  │   └──-3 B (13.64%) ── d [-]\n\
    │  ├───-5 B (22.73%) ── f [-]\n\
-   │  └────1 B (-4.55%) ── (fake child) [!]\n\
+   │  └────1 B (-04.55%) ── (fake child) [!]\n\
    └──-11 B (50.00%) -- g\n\
       ├───-7 B (31.82%) ── i [-]\n\
       ├───-6 B (27.27%) ── h [-]\n\
-      └────2 B (-9.09%) ── (fake child) [!]\n\
+      └────2 B (-09.09%) ── (fake child) [!]\n\
 \n\
 End of P8\n\
 ";
 
   // This is the output for the crash reports diff.
   let expectedDiff2 =
 "\
 Main Process (pid NNN)\n\