Bug 1132755 - Allocations tree has a bunch of columns that don't make sense, r=jsantell

browser/devtools/performance/performance.xul

@@ -197,34 +197,14 @@
 
               <vbox id="memory-calltree-view" flex="1">
                 <hbox class="call-tree-headers-container">
-                  <label class="plain call-tree-header"
-                         type="duration"
-                         crop="end"
-                         value="&profilerUI.table.totalDuration2;"/>
-                  <label class="plain call-tree-header"
-                         type="percentage"
-                         crop="end"
-                         value="&profilerUI.table.totalPercentage;"/>
                   <label class="plain call-tree-header"
                          type="allocations"
                          crop="end"
                          value="&profilerUI.table.totalAlloc;"/>
-                  <label class="plain call-tree-header"
-                         type="self-duration"
-                         crop="end"
-                         value="&profilerUI.table.selfDuration2;"/>
-                  <label class="plain call-tree-header"
-                         type="self-percentage"
-                         crop="end"
-                         value="&profilerUI.table.selfPercentage;"/>
                   <label class="plain call-tree-header"
                          type="self-allocations"
                          crop="end"
                          value="&profilerUI.table.selfAlloc;"/>
-                  <label class="plain call-tree-header"
-                         type="samples"
-                         crop="end"
-                         value="&profilerUI.table.samples;"/>
                   <label class="plain call-tree-header"
                          type="function"
                          crop="end"

browser/devtools/performance/test/browser.ini

@@ -16,6 +16,8 @@ support-files =
 [browser_perf-compatibility-04.js]
 [browser_perf-clear-01.js]
 [browser_perf-clear-02.js]
+[browser_perf-columns-js-calltree.js]
+[browser_perf-columns-memory-calltree.js]
 [browser_perf-data-massaging-01.js]
 [browser_perf-data-samples.js]
 [browser_perf-details-calltree-render.js]

browser/devtools/performance/test/browser_perf-columns-js-calltree.js

@@ -0,0 +1,53 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests that the js call tree view renders the correct columns.
+ */
+function spawnTest () {
+  let { panel } = yield initPerformance(SIMPLE_URL);
+  let { EVENTS, $, $$, DetailsView, JsCallTreeView } = panel.panelWin;
+
+  // Enable platform data to show the `busyWait` function in the tree.
+  Services.prefs.setBoolPref(PLATFORM_DATA_PREF, true);
+
+  yield DetailsView.selectView("js-calltree");
+  ok(DetailsView.isViewSelected(JsCallTreeView), "The call tree is now selected.");
+
+  yield startRecording(panel);
+  yield busyWait(1000);
+
+  let rendered = once(JsCallTreeView, EVENTS.JS_CALL_TREE_RENDERED);
+  yield stopRecording(panel);
+  yield rendered;
+
+  testCells($, $$, {
+    "duration": true,
+    "percentage": true,
+    "allocations": false,
+    "self-duration": true,
+    "self-percentage": true,
+    "self-allocations": false,
+    "samples": true,
+    "function": true
+  });
+
+  yield teardown(panel);
+  finish();
+}
+
+function testCells($, $$, visibleCells) {
+  for (let cell in visibleCells) {
+    if (visibleCells[cell]) {
+      ok($(`.call-tree-cell[type=${cell}]`),
+        `At least one ${cell} column was visible in the tree.`);
+    } else {
+      ok(!$(`.call-tree-cell[type=${cell}]`),
+        `No ${cell} columns were visible in the tree.`);
+    }
+  }
+
+  is($$(".call-tree-cell", $(".call-tree-item")).length,
+    Object.keys(visibleCells).filter(e => visibleCells[e]).length,
+    "The correct number of cells were found in the tree.");
+}

browser/devtools/performance/test/browser_perf-columns-memory-calltree.js

@@ -0,0 +1,53 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests that the memory call tree view renders the correct columns.
+ */
+function spawnTest () {
+  let { panel } = yield initPerformance(SIMPLE_URL);
+  let { EVENTS, $, $$, DetailsView, MemoryCallTreeView } = panel.panelWin;
+
+  // Enable memory to test.
+  Services.prefs.setBoolPref(MEMORY_PREF, true);
+
+  yield DetailsView.selectView("memory-calltree");
+  ok(DetailsView.isViewSelected(MemoryCallTreeView), "The call tree is now selected.");
+
+  yield startRecording(panel);
+  yield busyWait(1000);
+
+  let rendered = once(MemoryCallTreeView, EVENTS.MEMORY_CALL_TREE_RENDERED);
+  yield stopRecording(panel);
+  yield rendered;
+
+  testCells($, $$, {
+    "duration": false,
+    "percentage": false,
+    "allocations": true,
+    "self-duration": false,
+    "self-percentage": false,
+    "self-allocations": true,
+    "samples": false,
+    "function": true
+  });
+
+  yield teardown(panel);
+  finish();
+}
+
+function testCells($, $$, visibleCells) {
+  for (let cell in visibleCells) {
+    if (visibleCells[cell]) {
+      ok($(`.call-tree-cell[type=${cell}]`),
+        `At least one ${cell} column was visible in the tree.`);
+    } else {
+      ok(!$(`.call-tree-cell[type=${cell}]`),
+        `No ${cell} columns were visible in the tree.`);
+    }
+  }
+
+  is($$(".call-tree-cell", $(".call-tree-item")).length,
+    Object.keys(visibleCells).filter(e => visibleCells[e]).length,
+    "The correct number of cells were found in the tree.");
+}

browser/devtools/performance/views/details-js-call-tree.js

@@ -103,9 +103,6 @@ let JsCallTreeView = Heritage.extend(DetailsSubview, {
     container.innerHTML = "";
     root.attachTo(container);
 
-    // Profiler data does not contain memory allocations information.
-    root.toggleAllocations(false);
-
     // When platform data isn't shown, hide the cateogry labels, since they're
     // only available for C++ frames.
     let contentOnly = !PerformanceController.getOption("show-platform-data");

browser/devtools/performance/views/details-memory-call-tree.js

@@ -89,6 +89,13 @@ let MemoryCallTreeView = Heritage.extend(DetailsSubview, {
       // Call trees should only auto-expand when not inverted. Passing undefined
       // will default to the CALL_TREE_AUTO_EXPAND depth.
       autoExpandDepth: options.inverted ? 0 : undefined,
+      // Some cells like the time duration and cost percentage don't make sense
+      // for a memory allocations call tree.
+      visibleCells: {
+        allocations: true,
+        selfAllocations: true,
+        function: true
+      }
     });
 
     // Bind events.

browser/devtools/profiler/ui-profile.js

@@ -509,7 +509,6 @@ let ProfileView = {
 
     let contentOnly = !Prefs.showPlatformData;
     callTreeRoot.toggleCategories(!contentOnly);
-    callTreeRoot.toggleAllocations(false);
 
     this._callTreeRootByPanel.set(panel, callTreeRoot);
   },

browser/devtools/shared/profiler/tree-view.js

@@ -17,9 +17,20 @@ const MILLISECOND_UNITS = L10N.getStr("table.ms");
 const PERCENTAGE_UNITS = L10N.getStr("table.percentage");
 const URL_LABEL_TOOLTIP = L10N.getStr("table.url.tooltiptext");
 const ZOOM_BUTTON_TOOLTIP = L10N.getStr("table.zoom.tooltiptext");
-const CALL_TREE_AUTO_EXPAND = 3; // depth
 const CALL_TREE_INDENTATION = 16; // px
+
 const DEFAULT_SORTING_PREDICATE = (a, b) => a.frame.samples < b.frame.samples ? 1 : -1;
+const DEFAULT_AUTO_EXPAND_DEPTH = 3; // depth
+const DEFAULT_VISIBLE_CELLS = {
+  duration: true,
+  percentage: true,
+  allocations: false,
+  selfDuration: true,
+  selfPercentage: true,
+  selfAllocations: false,
+  samples: true,
+  function: true
+};
 
 const clamp = (val, min, max) => Math.max(min, Math.min(max, val));
 const sum = vals => vals.reduce((a, b) => a + b, 0);
@@ -55,23 +66,25 @@ exports.CallView = CallView;
  *        top-down). Defaults to false.
  * @param function sortingPredicate [optional]
  *        The predicate used to sort the tree items when created. Defaults to
- *        the caller's sortingPredicate if a caller exists, otherwise defaults
+ *        the caller's `sortingPredicate` if a caller exists, otherwise defaults
  *        to DEFAULT_SORTING_PREDICATE. The two passed arguments are FrameNodes.
  * @param number autoExpandDepth [optional]
  *        The depth to which the tree should automatically expand. Defualts to
  *        the caller's `autoExpandDepth` if a caller exists, otherwise defaults
- *        to CALL_TREE_AUTO_EXPAND.
+ *        to DEFAULT_AUTO_EXPAND_DEPTH.
+ * @param object visibleCells
+ *        An object specifying which cells are visible in the tree. Defaults to
+ *        the caller's `visibleCells` if a caller exists, otherwise defaults
+ *        to DEFAULT_VISIBLE_CELLS.
  */
-function CallView({ caller, frame, level, hidden, inverted, sortingPredicate, autoExpandDepth }) {
-  // Assume no indentation if this tree item's level is not specified.
-  level = level || 0;
-
-  // Don't increase indentation if this tree item is hidden.
-  if (hidden) {
-    level--;
-  }
-
-  AbstractTreeItem.call(this, { parent: caller, level });
+function CallView({
+  caller, frame, level, hidden, inverted,
+  sortingPredicate, autoExpandDepth, visibleCells
+}) {
+  AbstractTreeItem.call(this, {
+    parent: caller,
+    level: level|0 - (hidden ? 1 : 0)
+  });
 
   this.sortingPredicate = sortingPredicate != null
     ? sortingPredicate
@@ -81,7 +94,12 @@ function CallView({ caller, frame, level, hidden, inverted, sortingPredicate, au
   this.autoExpandDepth = autoExpandDepth != null
     ? autoExpandDepth
     : caller ? caller.autoExpandDepth
-             : CALL_TREE_AUTO_EXPAND;
+             : DEFAULT_AUTO_EXPAND_DEPTH;
+
+  this.visibleCells = visibleCells != null
+    ? visibleCells
+    : caller ? caller.visibleCells
+             : Object.create(DEFAULT_VISIBLE_CELLS);
 
   this.caller = caller;
   this.frame = frame;
@@ -110,35 +128,60 @@ CallView.prototype = Heritage.extend(AbstractTreeItem.prototype, {
     let totalAllocations;
 
     if (!this._getChildCalls().length) {
-      selfPercentage = framePercentage;
-      selfDuration = this.frame.duration;
-      totalAllocations = this.frame.allocations;
+      if (this.visibleCells.selfPercentage) {
+        selfPercentage = framePercentage;
+      }
+      if (this.visibleCells.selfDuration) {
+        selfDuration = this.frame.duration;
+      }
+      if (this.visibleCells.allocations) {
+        totalAllocations = this.frame.allocations;
+      }
     } else {
-      let childrenPercentage = sum(
-        [this._getPercentage(c.samples) for (c of this._getChildCalls())]);
-      let childrenDuration = sum(
-        [c.duration for (c of this._getChildCalls())]);
-      let childrenAllocations = sum(
-        [c.allocations for (c of this._getChildCalls())]);
-
-      selfPercentage = clamp(framePercentage - childrenPercentage, 0, 100);
-      selfDuration = this.frame.duration - childrenDuration;
-      totalAllocations = this.frame.allocations + childrenAllocations;
-
+      // Avoid performing costly computations if the respective columns
+      // won't be shown anyway.
+      if (this.visibleCells.selfPercentage) {
+        let childrenPercentage = sum([this._getPercentage(c.samples) for (c of this._getChildCalls())]);
+        selfPercentage = clamp(framePercentage - childrenPercentage, 0, 100);
+      }
+      if (this.visibleCells.selfDuration) {
+        let childrenDuration = sum([c.duration for (c of this._getChildCalls())]);
+        selfDuration = this.frame.duration - childrenDuration;
+      }
+      if (this.visibleCells.allocations) {
+        let childrenAllocations = sum([c.allocations for (c of this._getChildCalls())]);
+        totalAllocations = this.frame.allocations + childrenAllocations;
+      }
       if (this.inverted) {
         selfPercentage = framePercentage - selfPercentage;
         selfDuration = this.frame.duration - selfDuration;
       }
     }
 
-    let durationCell = this._createTimeCell(this.frame.duration);
-    let selfDurationCell = this._createTimeCell(selfDuration, true);
-    let percentageCell = this._createExecutionCell(framePercentage);
-    let selfPercentageCell = this._createExecutionCell(selfPercentage, true);
-    let allocationsCell = this._createAllocationsCell(totalAllocations);
-    let selfAllocationsCell = this._createAllocationsCell(this.frame.allocations, true);
-    let samplesCell = this._createSamplesCell(this.frame.samples);
-    let functionCell = this._createFunctionCell(arrowNode, frameInfo, this.level);
+    if (this.visibleCells.duration) {
+      var durationCell = this._createTimeCell(this.frame.duration);
+    }
+    if (this.visibleCells.selfDuration) {
+      var selfDurationCell = this._createTimeCell(selfDuration, true);
+    }
+    if (this.visibleCells.percentage) {
+      var percentageCell = this._createExecutionCell(framePercentage);
+    }
+    if (this.visibleCells.selfPercentage) {
+      var selfPercentageCell = this._createExecutionCell(selfPercentage, true);
+    }
+    if (this.visibleCells.allocations) {
+      var allocationsCell = this._createAllocationsCell(totalAllocations);
+    }
+    if (this.visibleCells.selfAllocations) {
+      var selfAllocationsCell = this._createAllocationsCell(this.frame.allocations, true);
+    }
+    if (this.visibleCells.samples) {
+      var samplesCell = this._createSamplesCell(this.frame.samples);
+    }
+    if (this.visibleCells.function) {
+      var functionCell = this._createFunctionCell(arrowNode, frameInfo, this.level);
+    }
 
     let targetNode = document.createElement("hbox");
     targetNode.className = "call-tree-item";
@@ -155,14 +198,30 @@ CallView.prototype = Heritage.extend(AbstractTreeItem.prototype, {
       functionCell.querySelector(".call-tree-category").hidden = true;
     }
 
-    targetNode.appendChild(durationCell);
-    targetNode.appendChild(percentageCell);
-    targetNode.appendChild(allocationsCell);
-    targetNode.appendChild(selfDurationCell);
-    targetNode.appendChild(selfPercentageCell);
-    targetNode.appendChild(selfAllocationsCell);
-    targetNode.appendChild(samplesCell);
-    targetNode.appendChild(functionCell);
+    if (this.visibleCells.duration) {
+      targetNode.appendChild(durationCell);
+    }
+    if (this.visibleCells.percentage) {
+      targetNode.appendChild(percentageCell);
+    }
+    if (this.visibleCells.allocations) {
+      targetNode.appendChild(allocationsCell);
+    }
+    if (this.visibleCells.selfDuration) {
+      targetNode.appendChild(selfDurationCell);
+    }
+    if (this.visibleCells.selfPercentage) {
+      targetNode.appendChild(selfPercentageCell);
+    }
+    if (this.visibleCells.selfAllocations) {
+      targetNode.appendChild(selfAllocationsCell);
+    }
+    if (this.visibleCells.samples) {
+      targetNode.appendChild(samplesCell);
+    }
+    if (this.visibleCells.function) {
+      targetNode.appendChild(functionCell);
+    }
 
     return targetNode;
   },
@@ -301,18 +360,6 @@ CallView.prototype = Heritage.extend(AbstractTreeItem.prototype, {
     return cell;
   },
 
-  /**
-   * Toggles the allocations information hidden or visible.
-   * @param boolean visible
-   */
-  toggleAllocations: function(visible) {
-    if (!visible) {
-      this.container.setAttribute("allocations-hidden", "");
-    } else {
-      this.container.removeAttribute("allocations-hidden");
-    }
-  },
-
   /**
    * Toggles the category information hidden or visible.
    * @param boolean visible

browser/themes/shared/devtools/performance.inc.css

@@ -124,8 +124,6 @@
   overflow: auto;
 }
 
-.call-tree-cells-container[allocations-hidden] .call-tree-cell[type="allocations"],
-.call-tree-cells-container[allocations-hidden] .call-tree-cell[type="self-allocations"],
 .call-tree-cells-container[categories-hidden] .call-tree-category {
   display: none;
 }

browser/themes/shared/devtools/profiler.inc.css

@@ -220,8 +220,6 @@
   overflow: auto;
 }
 
-.call-tree-cells-container[allocations-hidden] .call-tree-cell[type="allocations"],
-.call-tree-cells-container[allocations-hidden] .call-tree-cell[type="self-allocations"],
 .call-tree-cells-container[categories-hidden] .call-tree-category {
   display: none;
 }