Flamegraph uses seconds instead of hours when appropriate. (#851)

The Javascript code now behave the same as the Go code. (It switches
to the next unit only if one unit is at least as big as the value
being displayed.) The main effect is that we no longer use "hours" as
the unit for time values in the range [36s,3600s). We only switch to
using "hours" when the measured time is at least one hour. This is
easier for users to understand: e.g., a time value of hundred seconds
now displays as "100s", not "0.03hrs".

Also added a unittest for the Javascript unit picking code.

internal/driver/browser_test.go

@@ -88,9 +88,10 @@ func TestFlameGraph(t *testing.T) {
 	ctx, cancel = chromedp.NewContext(ctx)
 	defer cancel()
 
+	var ignored []byte // Some chromedp.Evaluate() versions wants non-nil result argument
 	err := chromedp.Run(ctx,
 		chromedp.Navigate(server.URL+"/flamegraph"),
-		chromedp.Evaluate(jsTestFixture, nil),
+		chromedp.Evaluate(jsTestFixture, &ignored),
 		eval(t, jsCheckFlame),
 	)
 	if err != nil {

internal/driver/html/stacks.js

@@ -13,23 +13,6 @@ function stackViewer(stacks, nodes) {
   const FONT_SIZE = 12;
   const MIN_FONT_SIZE = 8;
 
-  // Mapping from unit to a list of display scales/labels.
-  // List should be ordered by increasing unit size.
-  const UNITS = new Map([
-    ['B', [
-      ['B', 1],
-      ['kB', Math.pow(2, 10)],
-      ['MB', Math.pow(2, 20)],
-      ['GB', Math.pow(2, 30)],
-      ['TB', Math.pow(2, 40)],
-      ['PB', Math.pow(2, 50)]]],
-    ['s', [
-      ['ns', 1e-9],
-      ['µs', 1e-6],
-      ['ms', 1e-3],
-      ['s', 1],
-      ['hrs', 60*60]]]]);
-
   // Fields
   let pivots = [];          // Indices of currently selected data.Sources entries.
   let matches = new Set();  // Indices of sources that match search
@@ -570,22 +553,7 @@ function stackViewer(stacks, nodes) {
 
   // unitText returns a formatted string to display for value.
   function unitText(value) {
-    const sign = (value < 0) ? "-" : "";
-    let v = Math.abs(value) * stacks.Scale;
-    // Rescale to appropriate display unit.
-    let unit = stacks.Unit;
-    const list = UNITS.get(unit);
-    if (list) {
-      // Find first entry in list that is not too small.
-      for (const [name, scale] of list) {
-        if (v <= 100*scale) {
-          v /= scale;
-          unit = name;
-          break;
-        }
-      }
-    }
-    return sign + Number(v.toFixed(2)) + unit;
+    return pprofUnitText(value*stacks.Scale, stacks.Unit);
   }
 
   function find(name) {
@@ -606,3 +574,40 @@ function stackViewer(stacks, nodes) {
     return hsl;
   }
 }
+
+// Mapping from unit to a list of display scales/labels.
+// List should be ordered by increasing unit size.
+const pprofUnits = new Map([
+  ['B', [
+    ['B', 1],
+    ['kB', Math.pow(2, 10)],
+    ['MB', Math.pow(2, 20)],
+    ['GB', Math.pow(2, 30)],
+    ['TB', Math.pow(2, 40)],
+    ['PB', Math.pow(2, 50)]]],
+  ['s', [
+    ['ns', 1e-9],
+    ['µs', 1e-6],
+    ['ms', 1e-3],
+    ['s', 1],
+    ['hrs', 60*60]]]]);
+
+// pprofUnitText returns a formatted string to display for value in the specified unit.
+function pprofUnitText(value, unit) {
+  const sign = (value < 0) ? "-" : "";
+  let v = Math.abs(value);
+  // Rescale to appropriate display unit.
+  const list = pprofUnits.get(unit);
+  if (list) {
+    // Stop just before entry that is too large.
+    for (let i = 0; i < list.length; i++) {
+      if (i == list.length-1 || list[i+1][1] > v) {
+        const [name, scale] = list[i];
+        v /= scale;
+        unit = name;
+        break;
+      }
+    }
+  }
+  return sign + Number(v.toFixed(2)) + unit;
+}

internal/driver/testdata/testflame.js

@@ -109,6 +109,30 @@ function TestFlame() {
     }
   });
 
+  test.run("Units", function() {
+    function checkUnitText(unit, v, expect) {
+      const result = pprofUnitText(v, unit);
+      if (result != expect) {
+        test.err("bad text for", v, unit, ":", result, "expecting:", expect);
+      }
+    }
+
+    // Time units, plus logic tests.
+    checkUnitText("s", 0.51e-9, "0.51ns");
+    checkUnitText("s", 3e-9, "3ns");
+    checkUnitText("s", 1.23e-6, "1.23µs");
+    checkUnitText("s", 0.04, "40ms");
+    checkUnitText("s", 1, "1s");
+    checkUnitText("s", 3599, "3599s");
+    checkUnitText("s", 3600, "1hrs");
+
+    // Sanity check for byte units.
+    checkUnitText("B", 2*1048576, "2MB");
+
+    // Unknown unit.
+    checkUnitText("cm", 100, "100cm");
+  });
+
   return test.result;
 }
 TestFlame();