Bug 1130274 - JS and Memory flame graphs get rerendered but don't actually change when prefs are modified, r=jsantell
authorVictor Porof <vporof@mozilla.com>
Fri, 06 Feb 2015 13:23:02 -0500
changeset 227861 e4ed2f7b72c5ee5a6392cea5115fbab24a465b19
parent 227860 8d3c748186062dfe4696bd5a682a7915f0be1a90
child 227862 44685f89442cd073dbb2ab575fe3dce7ad2de768
push id11205
push uservporof@mozilla.com
push dateFri, 06 Feb 2015 19:16:34 +0000
treeherderfx-team@dee2c8135242 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjsantell
bugs1130274
milestone38.0a1
Bug 1130274 - JS and Memory flame graphs get rerendered but don't actually change when prefs are modified, r=jsantell
browser/devtools/performance/test/browser_perf-options-flatten-tree-recursion-01.js
browser/devtools/performance/test/browser_perf-options-flatten-tree-recursion-02.js
browser/devtools/performance/views/details-abstract-subview.js
browser/devtools/performance/views/details-js-flamegraph.js
browser/devtools/performance/views/details-memory-call-tree.js
browser/devtools/performance/views/details-memory-flamegraph.js
--- a/browser/devtools/performance/test/browser_perf-options-flatten-tree-recursion-01.js
+++ b/browser/devtools/performance/test/browser_perf-options-flatten-tree-recursion-01.js
@@ -1,36 +1,60 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
  * Tests that the js flamegraphs get rerendered when toggling `flatten-tree-recursion`
  */
 function spawnTest () {
   let { panel } = yield initPerformance(SIMPLE_URL);
-  let { EVENTS, DetailsView, JsFlameGraphView } = panel.panelWin;
+  let { EVENTS, PerformanceController, DetailsView, JsFlameGraphView } = panel.panelWin;
 
   Services.prefs.setBoolPref(FLATTEN_PREF, true);
 
   yield DetailsView.selectView("js-flamegraph");
 
   yield startRecording(panel);
   yield busyWait(100);
 
   let rendered = once(JsFlameGraphView, EVENTS.JS_FLAMEGRAPH_RENDERED);
   yield stopRecording(panel);
   yield rendered;
 
+  let samples1 = PerformanceController.getCurrentRecording().getProfile().threads[0].samples;
+  let rendering1 = FlameGraphUtils._cache.get(samples1);
+
+  ok(samples1,
+    "The samples were retrieved from the controller.");
+  ok(rendering1,
+    "The rendering data was cached.");
+
   rendered = once(JsFlameGraphView, EVENTS.JS_FLAMEGRAPH_RENDERED);
   Services.prefs.setBoolPref(FLATTEN_PREF, false);
   yield rendered;
 
   ok(true, "JsFlameGraphView rerendered when toggling flatten-tree-recursion.");
 
+  let samples2 = PerformanceController.getCurrentRecording().getProfile().threads[0].samples;
+  let rendering2 = FlameGraphUtils._cache.get(samples2);
+
+  is(samples1, samples2,
+    "The same samples data should be retrieved from the controller (1).");
+  isnot(rendering1, rendering2,
+    "The rendering data should be different because other options were used (1).");
+
   rendered = once(JsFlameGraphView, EVENTS.JS_FLAMEGRAPH_RENDERED);
   Services.prefs.setBoolPref(FLATTEN_PREF, true);
   yield rendered;
 
   ok(true, "JsFlameGraphView rerendered when toggling back flatten-tree-recursion.");
 
+  let samples3 = PerformanceController.getCurrentRecording().getProfile().threads[0].samples;
+  let rendering3 = FlameGraphUtils._cache.get(samples3);
+
+  is(samples2, samples3,
+    "The same samples data should be retrieved from the controller (2).");
+  isnot(rendering2, rendering3,
+    "The rendering data should be different because other options were used (2).");
+
   yield teardown(panel);
   finish();
 }
--- a/browser/devtools/performance/test/browser_perf-options-flatten-tree-recursion-02.js
+++ b/browser/devtools/performance/test/browser_perf-options-flatten-tree-recursion-02.js
@@ -1,38 +1,71 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
  * Tests that the memory flamegraphs get rerendered when toggling `flatten-tree-recursion`
  */
 function spawnTest () {
   let { panel } = yield initPerformance(SIMPLE_URL);
-  let { EVENTS, DetailsView, MemoryFlameGraphView } = panel.panelWin;
+  let { EVENTS, PerformanceController, DetailsView, MemoryFlameGraphView } = panel.panelWin;
 
   // Enable memory to test
   Services.prefs.setBoolPref(MEMORY_PREF, true);
   Services.prefs.setBoolPref(FLATTEN_PREF, true);
 
   yield DetailsView.selectView("memory-flamegraph");
 
   yield startRecording(panel);
   yield busyWait(100);
 
   let rendered = once(MemoryFlameGraphView, EVENTS.MEMORY_FLAMEGRAPH_RENDERED);
   yield stopRecording(panel);
   yield rendered;
 
+  let allocations1 = PerformanceController.getCurrentRecording().getAllocations();
+  let samples1 = RecordingUtils.getSamplesFromAllocations(allocations1);
+  let rendering1 = FlameGraphUtils._cache.get(samples1);
+
+  ok(allocations1,
+    "The allocations were retrieved from the controller.");
+  ok(samples1,
+    "The samples were retrieved from the utility funcs.");
+  ok(rendering1,
+    "The rendering data was cached.");
+
   rendered = once(MemoryFlameGraphView, EVENTS.MEMORY_FLAMEGRAPH_RENDERED);
   Services.prefs.setBoolPref(FLATTEN_PREF, false);
   yield rendered;
 
   ok(true, "MemoryFlameGraphView rerendered when toggling flatten-tree-recursion.");
 
+  let allocations2 = PerformanceController.getCurrentRecording().getAllocations();
+  let samples2 = RecordingUtils.getSamplesFromAllocations(allocations2);
+  let rendering2 = FlameGraphUtils._cache.get(samples2);
+
+  is(allocations1, allocations2,
+    "The same allocations data should be retrieved from the controller (1).");
+  is(samples1, samples2,
+    "The same samples data should be retrieved from the utility funcs. (1).");
+  isnot(rendering1, rendering2,
+    "The rendering data should be different because other options were used (1).");
+
   rendered = once(MemoryFlameGraphView, EVENTS.MEMORY_FLAMEGRAPH_RENDERED);
   Services.prefs.setBoolPref(FLATTEN_PREF, true);
   yield rendered;
 
   ok(true, "MemoryFlameGraphView rerendered when toggling back flatten-tree-recursion.");
 
+  let allocations3 = PerformanceController.getCurrentRecording().getAllocations();
+  let samples3 = RecordingUtils.getSamplesFromAllocations(allocations3);
+  let rendering3 = FlameGraphUtils._cache.get(samples3);
+
+  is(allocations2, allocations3,
+    "The same allocations data should be retrieved from the controller (2).");
+  is(samples2, samples3,
+    "The same samples data should be retrieved from the utility funcs. (2).");
+  isnot(rendering2, rendering3,
+    "The rendering data should be different because other options were used (2).");
+
   yield teardown(panel);
   finish();
 }
--- a/browser/devtools/performance/views/details-abstract-subview.js
+++ b/browser/devtools/performance/views/details-abstract-subview.js
@@ -109,16 +109,20 @@ let DetailsSubview = {
     if (!recording || recording.isRecording()) {
       return;
     }
 
     if (!~this.rerenderPrefs.indexOf(prefName)) {
       return;
     }
 
+    if (this._onRerenderPrefChanged) {
+      this._onRerenderPrefChanged();
+    }
+
     if (DetailsView.isViewSelected(this) || this.canUpdateWhileHidden) {
       this.render(OverviewView.getTimeInterval());
     } else {
       this.shouldUpdateWhenShown = true;
     }
   }
 };
 
--- a/browser/devtools/performance/views/details-js-flamegraph.js
+++ b/browser/devtools/performance/views/details-js-flamegraph.js
@@ -68,10 +68,20 @@ let JsFlameGraphView = Heritage.extend(D
   },
 
   /**
    * Fired when a range is selected or cleared in the FlameGraph.
    */
   _onRangeChangeInGraph: function () {
     let interval = this.graph.getViewRange();
     OverviewView.setTimeInterval(interval, { stopPropagation: true });
+  },
+
+  /**
+   * Called whenever a pref is changed and this view needs to be rerendered.
+   */
+  _onRerenderPrefChanged: function() {
+    let recording = PerformanceController.getCurrentRecording();
+    let profile = recording.getProfile();
+    let samples = profile.threads[0].samples;
+    FlameGraphUtils.removeFromCache(samples);
   }
 });
--- a/browser/devtools/performance/views/details-memory-call-tree.js
+++ b/browser/devtools/performance/views/details-memory-call-tree.js
@@ -96,10 +96,9 @@ let MemoryCallTreeView = Heritage.extend
     // Clear out other call trees.
     let container = $("#memory-calltree-view > .call-tree-cells-container");
     container.innerHTML = "";
     root.attachTo(container);
 
     // Memory allocation samples don't contain cateogry labels.
     root.toggleCategories(false);
   }
-
 });
--- a/browser/devtools/performance/views/details-memory-flamegraph.js
+++ b/browser/devtools/performance/views/details-memory-flamegraph.js
@@ -67,10 +67,20 @@ let MemoryFlameGraphView = Heritage.exte
   },
 
   /**
    * Fired when a range is selected or cleared in the FlameGraph.
    */
   _onRangeChangeInGraph: function () {
     let interval = this.graph.getViewRange();
     OverviewView.setTimeInterval(interval, { stopPropagation: true });
+  },
+
+  /**
+   * Called whenever a pref is changed and this view needs to be rerendered.
+   */
+  _onRerenderPrefChanged: function() {
+    let recording = PerformanceController.getCurrentRecording();
+    let allocations = recording.getAllocations();
+    let samples = RecordingUtils.getSamplesFromAllocations(allocations);
+    FlameGraphUtils.removeFromCache(samples);
   }
 });