Bug 1119932 - Flatten recursion in the flamegraph for the new performance tool, r=jsantell
authorVictor Porof <vporof@mozilla.com>
Thu, 15 Jan 2015 14:54:46 -0500
changeset 224319 b8b515f7197beb3a9b9afd043ba6d0d3dc5d3712
parent 224318 fb5bebd170baed4bcb5f88590bf316142febde4a
child 224320 aeffa71cbbb0e95c038bdc5f094c0ce710e89ade
push id54190
push userkwierso@gmail.com
push dateSat, 17 Jan 2015 02:06:29 +0000
treeherdermozilla-inbound@369a8f14ccf8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjsantell
bugs1119932
milestone38.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 1119932 - Flatten recursion in the flamegraph for the new performance tool, r=jsantell
browser/app/profile/firefox.js
browser/devtools/performance/performance-controller.js
browser/devtools/performance/views/details-flamegraph.js
browser/devtools/shared/test/browser.ini
browser/devtools/shared/test/browser_flame-graph-utils-01.js
browser/devtools/shared/test/browser_flame-graph-utils-02.js
browser/devtools/shared/test/browser_flame-graph-utils.js
browser/devtools/shared/widgets/FlameGraph.jsm
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1425,16 +1425,17 @@ pref("devtools.timeline.hiddenMarkers", 
   pref("devtools.performance_dev.enabled", true);
 #else
   pref("devtools.performance_dev.enabled", false);
 #endif
 
 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
 pref("devtools.cache.disabled", false);
 
 // Enable the Network Monitor
 pref("devtools.netmonitor.enabled", true);
 
--- a/browser/devtools/performance/performance-controller.js
+++ b/browser/devtools/performance/performance-controller.js
@@ -374,17 +374,18 @@ let PerformanceController = {
  * Convenient way of emitting events from the controller.
  */
 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"],
 });
 
 /**
  * DOM query helpers.
  */
 function $(selector, target = document) {
   return target.querySelector(selector);
 }
--- a/browser/devtools/performance/views/details-flamegraph.js
+++ b/browser/devtools/performance/views/details-flamegraph.js
@@ -37,17 +37,19 @@ let FlameGraphView = {
    * Method for handling all the set up for rendering a new flamegraph.
    */
   render: function (profilerData) {
     // Empty recordings might yield no profiler data.
     if (profilerData.profile == null) {
       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);
   },
 
   /**
    * Called when recording is stopped.
    */
   _onRecordingStopped: function () {
--- a/browser/devtools/shared/test/browser.ini
+++ b/browser/devtools/shared/test/browser.ini
@@ -15,17 +15,18 @@ support-files =
 [browser_cubic-bezier-01.js]
 [browser_cubic-bezier-02.js]
 [browser_cubic-bezier-03.js]
 [browser_flame-graph-01.js]
 [browser_flame-graph-02.js]
 [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_graphs-04.js]
 [browser_graphs-05.js]
 [browser_graphs-06.js]
 [browser_graphs-07a.js]
 [browser_graphs-07b.js]
rename from browser/devtools/shared/test/browser_flame-graph-utils.js
rename to browser/devtools/shared/test/browser_flame-graph-utils-01.js
--- a/browser/devtools/shared/test/browser_flame-graph-utils.js
+++ b/browser/devtools/shared/test/browser_flame-graph-utils-01.js
@@ -1,12 +1,13 @@
 /* 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", {});
 
 let test = Task.async(function*() {
   yield promiseTab("about:blank");
   yield performTest();
   gBrowser.removeCurrentTab();
   finish();
new file mode 100644
--- /dev/null
+++ b/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: []
+}];
--- a/browser/devtools/shared/widgets/FlameGraph.jsm
+++ b/browser/devtools/shared/widgets/FlameGraph.jsm
@@ -822,22 +822,25 @@ const COLOR_PALLETTE = Array.from(Array(
  */
 let FlameGraphUtils = {
   /**
    * Converts a list of samples from the profiler data to something that's
    * drawable by a FlameGraph widget.
    *
    * @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.
 
     let buckets = new Map();
 
     for (let color of COLOR_PALLETTE) {
       buckets.set(color, []);
     }
@@ -845,16 +848,22 @@ let FlameGraphUtils = {
     // 2. Populate the buckets by iterating over every frame in every sample.
 
     let prevTime = 0;
     let prevFrames = [];
 
     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];
 
         // Frames at the same location and the same depth will be reused.
         // If there is a block already created, change its width.
         if (prevFrame && prevFrame.srcData.rawLocation == location) {
           prevFrame.width = (time - prevFrame.srcData.startTime);
         }
@@ -890,16 +899,32 @@ let FlameGraphUtils = {
     for (let [color, blocks] of buckets) {
       out.push({ color, blocks });
     }
 
     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.
    *
    * @param string input
    * @return number
    */
   _getStringHash: function(input) {
     const STRING_HASH_PRIME1 = 7;
     const STRING_HASH_PRIME2 = 31;