Bug 1119932 - Flatten recursion in the flamegraph for the new performance tool, r=jsantell

--HG--
rename : browser/devtools/shared/test/browser_flame-graph-utils.js => browser/devtools/shared/test/browser_flame-graph-utils-01.js

browser/app/profile/firefox.js

@@ -1430,6 +1430,7 @@ pref("devtools.timeline.hiddenMarkers", "[]");
 pref("devtools.performance.ui.show-timeline-memory", false);
 
 // The default Profiler UI settings
+pref("devtools.profiler.ui.flatten-tree-recursion", true);
 pref("devtools.profiler.ui.show-platform-data", false);
 
 // The default cache UI setting

browser/devtools/performance/performance-controller.js

@@ -379,7 +379,8 @@ EventEmitter.decorate(PerformanceController);
  * Shortcuts for accessing various profiler preferences.
  */
 const Prefs = new ViewHelpers.Prefs("devtools.profiler", {
-  showPlatformData: ["Bool", "ui.show-platform-data"]
+  flattenTreeRecursion: ["Bool", "ui.flatten-tree-recursion"],
+  showPlatformData: ["Bool", "ui.show-platform-data"],
 });
 
 /**

browser/devtools/performance/views/details-flamegraph.js

@@ -42,7 +42,9 @@ let FlameGraphView = {
       return;
     }
     let samples = profilerData.profile.threads[0].samples;
-    let dataSrc = FlameGraphUtils.createFlameGraphDataFromSamples(samples);
+    let dataSrc = FlameGraphUtils.createFlameGraphDataFromSamples(samples, {
+      flattenRecursion: Prefs.flattenTreeRecursion
+    });
     this.graph.setData(dataSrc);
     this.emit(EVENTS.FLAMEGRAPH_RENDERED);
   },

browser/devtools/shared/test/browser.ini

@@ -20,7 +20,8 @@ support-files =
 [browser_flame-graph-03a.js]
 [browser_flame-graph-03b.js]
 [browser_flame-graph-04.js]
-[browser_flame-graph-utils.js]
+[browser_flame-graph-utils-01.js]
+[browser_flame-graph-utils-02.js]
 [browser_graphs-01.js]
 [browser_graphs-02.js]
 [browser_graphs-03.js]

browser/devtools/shared/test/browser_flame-graph-utils-01.js

@@ -1,7 +1,8 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
-// Tests that text metrics in the flame graph widget work properly.
+// Tests that text metrics and data conversion from profiler samples
+// widget work properly in the flame graph.
 
 let {FlameGraphUtils} = Cu.import("resource:///modules/devtools/FlameGraph.jsm", {});
 

browser/devtools/shared/test/browser_flame-graph-utils-02.js

@@ -0,0 +1,104 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests consecutive duplicate frames are removed from the flame graph data.
+
+let {FlameGraphUtils} = Cu.import("resource:///modules/devtools/FlameGraph.jsm", {});
+
+let test = Task.async(function*() {
+  yield promiseTab("about:blank");
+  yield performTest();
+  gBrowser.removeCurrentTab();
+  finish();
+});
+
+function* performTest() {
+  let out = FlameGraphUtils.createFlameGraphDataFromSamples(TEST_DATA, {
+    flattenRecursion: true
+  });
+
+  ok(out, "Some data was outputted properly");
+  is(out.length, 10, "The outputted length is correct.");
+
+  info("Got flame graph data:\n" + out.toSource() + "\n");
+
+  for (let i = 0; i < out.length; i++) {
+    let found = out[i];
+    let expected = EXPECTED_OUTPUT[i];
+
+    is(found.blocks.length, expected.blocks.length,
+      "The correct number of blocks were found in this bucket.");
+
+    for (let j = 0; j < found.blocks.length; j++) {
+      is(found.blocks[j].x, expected.blocks[j].x,
+        "The expected block X position is correct for this frame.");
+      is(found.blocks[j].y, expected.blocks[j].y,
+        "The expected block Y position is correct for this frame.");
+      is(found.blocks[j].width, expected.blocks[j].width,
+        "The expected block width is correct for this frame.");
+      is(found.blocks[j].height, expected.blocks[j].height,
+        "The expected block height is correct for this frame.");
+      is(found.blocks[j].text, expected.blocks[j].text,
+        "The expected block text is correct for this frame.");
+    }
+  }
+}
+
+let TEST_DATA = [{
+  frames: [{
+    location: "A"
+  }, {
+    location: "A"
+  }, {
+    location: "A"
+  }, {
+    location: "B",
+  }, {
+    location: "B",
+  }, {
+    location: "C"
+  }],
+  time: 50,
+}];
+
+let EXPECTED_OUTPUT = [{
+  blocks: []
+}, {
+  blocks: []
+}, {
+  blocks: [{
+    srcData: {
+      startTime: 0,
+      rawLocation: "A"
+    },
+    x: 0,
+    y: 0,
+    width: 50,
+    height: 11,
+    text: "A"
+  }]
+}, {
+  blocks: [{
+    srcData: {
+      startTime: 0,
+      rawLocation: "B"
+    },
+    x: 0,
+    y: 11,
+    width: 50,
+    height: 11,
+    text: "B"
+  }]
+}, {
+  blocks: []
+}, {
+  blocks: []
+}, {
+  blocks: []
+}, {
+  blocks: []
+}, {
+  blocks: []
+}, {
+  blocks: []
+}];

browser/devtools/shared/widgets/FlameGraph.jsm

@@ -827,12 +827,15 @@ let FlameGraphUtils = {
    *
    * @param array samples
    *        A list of { time, frames: [{ location }] } objects.
+   * @param object options [optional]
+   *        Additional options supported by this operation:
+   *          - flattenRecursion: specifies if consecutive frames are omitted
    * @param array out [optional]
    *        An output storage to reuse for storing the flame graph data.
    * @return array
    *         The flame graph data.
    */
-  createFlameGraphDataFromSamples: function(samples, out = []) {
+  createFlameGraphDataFromSamples: function(samples, options = {}, out = []) {
     // 1. Create a map of colors to arrays, representing buckets of
     // blocks inside the flame graph pyramid sharing the same style.
 
@@ -850,6 +853,12 @@ let FlameGraphUtils = {
     for (let { frames, time } of samples) {
       let frameIndex = 0;
 
+      // Flatten recursion if preferred, by removing consecutive frames
+      // sharing the same location.
+      if (options.flattenRecursion) {
+        frames = frames.filter(this._isConsecutiveDuplicate);
+      }
+
       for (let { location } of frames) {
         let prevFrame = prevFrames[frameIndex];
 
@@ -894,6 +903,22 @@ let FlameGraphUtils = {
     return out;
   },
 
+  /**
+   * Checks if the provided frame is the same as the next one in a sample.
+   *
+   * @param object e
+   *        An object containing a { location } property.
+   * @param number index
+   *        The index of the object in the parent array.
+   * @param array array
+   *        The parent array.
+   * @return boolean
+   *         True if the next frame shares the same location, false otherwise.
+   */
+  _isConsecutiveDuplicate: function(e, index, array) {
+    return index < array.length - 1 && e.location != array[index + 1].location;
+  },
+
   /**
    * Very dumb hashing of a string. Used to pick colors from a pallette.
    *