Bug 1198019 - Snapping to allocations from GC markers should snap to the previous cycle, not the previous marker. r=fitzgen
authorJordan Santell <jsantell@mozilla.com>
Thu, 27 Aug 2015 15:01:20 -0700
changeset 259644 a812ab9b77bb23fec84afbb303d927100cf7a448
parent 259643 86aaa9aafca2ad79e3989844eb1fc094c7cc55a3
child 259718 8b78514a72da184b5e481f7dfa5cf0fe9378b0a3
push id14901
push userjsantell@mozilla.com
push dateFri, 28 Aug 2015 01:06:57 +0000
treeherderfx-team@a812ab9b77bb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1198019
milestone43.0a1
Bug 1198019 - Snapping to allocations from GC markers should snap to the previous cycle, not the previous marker. r=fitzgen
browser/devtools/performance/test/browser_perf-details-waterfall-gc-snap.js
browser/devtools/performance/views/details-waterfall.js
toolkit/devtools/server/tests/browser/browser_markers-gc.js
toolkit/devtools/shared/timeline.js
--- a/browser/devtools/performance/test/browser_perf-details-waterfall-gc-snap.js
+++ b/browser/devtools/performance/test/browser_perf-details-waterfall-gc-snap.js
@@ -1,138 +1,144 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
- * Tests that the waterfall view renders content after recording.
+ * Tests that the marker details on GC markers displays allocation
+ * buttons and snaps to the correct range
  */
 function* spawnTest() {
-  let { panel } = yield initPerformance(SIMPLE_URL);
+  let { panel } = yield initPerformance(ALLOCS_URL);
   let { $, $$, EVENTS, PerformanceController, OverviewView, DetailsView, WaterfallView, MemoryCallTreeView } = panel.panelWin;
   let EPSILON = 0.00001;
 
-  // Add the Cu.forceGC option to display allocation types for tests
-  let triggerTypes = Services.prefs.getCharPref("devtools.performance.ui.show-triggers-for-gc-types");
-  Services.prefs.setCharPref("devtools.performance.ui.show-triggers-for-gc-types", `${triggerTypes} COMPONENT_UTILS`);
-
-  // Disable non GC markers so we don't have nested markers and marker index
-  // matches DOM index.
-  PerformanceController.setPref("hidden-markers", Array.reduce($$("menuitem"), (disabled, item) => {
-    let type = item.getAttribute("marker-type");
-    if (type && type !== "GarbageCollection") {
-      disabled.push(type);
-    }
-    return disabled;
-  }, []));
-
   Services.prefs.setBoolPref(ALLOCATIONS_PREF, true);
 
   yield startRecording(panel);
-  yield waitUntil(hasGCMarkers(PerformanceController));
-  let rendered = once(WaterfallView, EVENTS.WATERFALL_RENDERED);
+  yield idleWait(1000);
   yield stopRecording(panel);
-  ok(DetailsView.isViewSelected(WaterfallView),
-    "The waterfall view is selected by default in the details view.");
+
+  injectGCMarkers(PerformanceController, WaterfallView);
+
+  // Select everything
+  let rendered = WaterfallView.once(EVENTS.WATERFALL_RENDERED);
+  OverviewView.setTimeInterval({ startTime: 0, endTime: Number.MAX_VALUE });
   yield rendered;
 
   let bars = $$(".waterfall-marker-bar");
-  let markers = PerformanceController.getCurrentRecording().getMarkers();
-  let gcMarkers = markers.filter(isForceGCMarker);
-
-  ok(gcMarkers.length >= 2, "should have atleast 2 GC markers");
-  ok(bars.length >= 2, "should have atleast 2 GC markers rendered");
-
-  info("Received markers:");
-  for (let marker of gcMarkers) {
-    info(`${marker.causeName} ${marker.start}:${marker.end}`);
-  }
+  let gcMarkers = PerformanceController.getCurrentRecording().getMarkers();
+  ok(gcMarkers.length === 9, "should have 9 GC markers");
+  ok(bars.length === 9, "should have 9 GC markers rendered");
 
   /**
-   * Check when it's the first GC marker
+   * Check when it's the second marker of the first GC cycle.
    */
 
-  info(`Will show allocation trigger button for ${Services.prefs.getCharPref("devtools.performance.ui.show-triggers-for-gc-types")}`);
-  info(`Clicking GC Marker of type ${gcMarkers[0].causeName} ${gcMarkers[0].start}:${gcMarkers[0].end}`);
-  EventUtils.sendMouseEvent({ type: "mousedown" }, bars[0]);
+  let targetMarker = gcMarkers[1];
+  let targetBar = bars[1];
+  info(`Clicking GC Marker of type ${targetMarker.causeName} ${targetMarker.start}:${targetMarker.end}`);
+  EventUtils.sendMouseEvent({ type: "mousedown" }, targetBar);
   let showAllocsButton;
   // On slower machines this can not be found immediately?
   yield waitUntil(() => showAllocsButton = $("#waterfall-details .custom-button[type='show-allocations']"));
   ok(showAllocsButton, "GC buttons when allocations are enabled");
 
   rendered = once(MemoryCallTreeView, EVENTS.MEMORY_CALL_TREE_RENDERED);
   EventUtils.sendMouseEvent({ type: "click" }, showAllocsButton);
   yield rendered;
 
   is(OverviewView.getTimeInterval().startTime, 0, "When clicking first GC, should use 0 as start time");
-  within(OverviewView.getTimeInterval().endTime, gcMarkers[0].start, EPSILON, "Correct end time range");
+  within(OverviewView.getTimeInterval().endTime, targetMarker.start, EPSILON, "Correct end time range");
 
   let duration = PerformanceController.getCurrentRecording().getDuration();
   rendered = once(WaterfallView, EVENTS.WATERFALL_RENDERED);
   OverviewView.setTimeInterval({ startTime: 0, endTime: duration });
   yield DetailsView.selectView("waterfall");
   yield rendered;
 
   /**
-   * Check when there is a previous GC marker
+   * Check when there is a previous GC cycle
    */
 
   bars = $$(".waterfall-marker-bar");
-  info(`Clicking GC Marker of type ${gcMarkers[1].causeName} ${gcMarkers[1].start}:${gcMarkers[1].end}`);
-  EventUtils.sendMouseEvent({ type: "mousedown" }, bars[1]);
+  targetMarker = gcMarkers[4];
+  targetBar = bars[4];
+
+  info(`Clicking GC Marker of type ${targetMarker.causeName} ${targetMarker.start}:${targetMarker.end}`);
+  EventUtils.sendMouseEvent({ type: "mousedown" }, targetBar);
   // On slower machines this can not be found immediately?
   yield waitUntil(() => showAllocsButton = $("#waterfall-details .custom-button[type='show-allocations']"));
   ok(showAllocsButton, "GC buttons when allocations are enabled");
 
   rendered = once(MemoryCallTreeView, EVENTS.MEMORY_CALL_TREE_RENDERED);
   EventUtils.sendMouseEvent({ type: "click" }, showAllocsButton);
   yield rendered;
 
-  within(OverviewView.getTimeInterval().startTime, gcMarkers[0].end, EPSILON,
-    "selection start range is previous GC marker's end time");
-  within(OverviewView.getTimeInterval().endTime, gcMarkers[1].start, EPSILON,
+  within(OverviewView.getTimeInterval().startTime, gcMarkers[2].end, EPSILON,
+    "selection start range is last marker from previous GC cycle.");
+  within(OverviewView.getTimeInterval().endTime, targetMarker.start, EPSILON,
     "selection end range is current GC marker's start time");
 
   /**
    * Now with allocations disabled
    */
 
   // Reselect the entire recording -- due to bug 1196945, the new recording
   // won't reset the selection
   duration = PerformanceController.getCurrentRecording().getDuration();
   rendered = once(WaterfallView, EVENTS.WATERFALL_RENDERED);
   OverviewView.setTimeInterval({ startTime: 0, endTime: duration });
   yield rendered;
 
   Services.prefs.setBoolPref(ALLOCATIONS_PREF, false);
   yield startRecording(panel);
-  yield waitUntil(hasGCMarkers(PerformanceController));
-
   rendered = once(WaterfallView, EVENTS.WATERFALL_RENDERED);
   yield stopRecording(panel);
-  ok(DetailsView.isViewSelected(WaterfallView),
-    "The waterfall view is selected by default in the details view.");
+  yield rendered;
+
+  injectGCMarkers(PerformanceController, WaterfallView);
+
+  // Select everything
+  rendered = WaterfallView.once(EVENTS.WATERFALL_RENDERED);
+  OverviewView.setTimeInterval({ startTime: 0, endTime: Number.MAX_VALUE });
   yield rendered;
 
   ok(true, "WaterfallView rendered after recording is stopped.");
 
   bars = $$(".waterfall-marker-bar");
-  markers = PerformanceController.getCurrentRecording().getMarkers();
-  gcMarkers = markers.filter(isForceGCMarker);
+  gcMarkers = PerformanceController.getCurrentRecording().getMarkers();
 
   EventUtils.sendMouseEvent({ type: "mousedown" }, bars[0]);
   showAllocsButton = $("#waterfall-details .custom-button[type='show-allocations']");
   ok(!showAllocsButton, "No GC buttons when allocations are disabled");
 
 
   yield teardown(panel);
   finish();
 }
 
-function isForceGCMarker (m) {
-  return m.name === "GarbageCollection" && m.causeName === "COMPONENT_UTILS" && m.start >= 0;
+function injectGCMarkers (controller, waterfall) {
+  // Push some fake GC markers into the recording
+  let realMarkers = controller.getCurrentRecording().getMarkers();
+  // Invalidate marker cache
+  waterfall._cache.delete(realMarkers);
+  realMarkers.length = 0;
+  for (let gcMarker of GC_MARKERS) {
+    realMarkers.push(gcMarker);
+  }
 }
 
-function hasGCMarkers (controller) {
-  return function () {
-    Cu.forceGC();
-    return controller.getCurrentRecording().getMarkers().filter(isForceGCMarker).length >= 2;
-  };
-}
+let GC_MARKERS = [
+  { causeName: "TOO_MUCH_MALLOC", cycle: 1 },
+  { causeName: "TOO_MUCH_MALLOC", cycle: 1 },
+  { causeName: "TOO_MUCH_MALLOC", cycle: 1 },
+  { causeName: "ALLOC_TRIGGER", cycle: 2 },
+  { causeName: "ALLOC_TRIGGER", cycle: 2 },
+  { causeName: "ALLOC_TRIGGER", cycle: 2 },
+  { causeName: "SET_NEW_DOCUMENT", cycle: 3 },
+  { causeName: "SET_NEW_DOCUMENT", cycle: 3 },
+  { causeName: "SET_NEW_DOCUMENT", cycle: 3 },
+].map((marker, i) => {
+  marker.name = "GarbageCollection";
+  marker.start = 50 + (i * 10);
+  marker.end = marker.start + 9;
+  return marker;
+});
--- a/browser/devtools/performance/views/details-waterfall.js
+++ b/browser/devtools/performance/views/details-waterfall.js
@@ -137,32 +137,36 @@ let WaterfallView = Heritage.extend(Deta
    * Called when MarkerDetails view emits an event to snap to allocations.
    */
   _onShowAllocations: function (_, data) {
     let { endTime } = data;
     let startTime = 0;
     let recording = PerformanceController.getCurrentRecording();
     let markers = recording.getMarkers();
 
-    let mostRecentGC = null;
-
+    let lastGCMarkerFromPreviousCycle = null;
+    let lastGCMarker = null;
     // Iterate over markers looking for the most recent GC marker
-    // before the one who's start time is `endTime`.
+    // from the cycle before the marker's whose allocations we're interested in.
     for (let marker of markers) {
       // We found the marker whose allocations we're tracking; abort
       if (marker.start === endTime) {
         break;
       }
+
       if (marker.name === "GarbageCollection") {
-        mostRecentGC = marker;
+        if (lastGCMarker && lastGCMarker.cycle !== marker.cycle) {
+          lastGCMarkerFromPreviousCycle = lastGCMarker;
+        }
+        lastGCMarker = marker;
       }
     }
 
-    if (mostRecentGC) {
-      startTime = mostRecentGC.end;
+    if (lastGCMarkerFromPreviousCycle) {
+      startTime = lastGCMarkerFromPreviousCycle.end;
     }
 
     // Adjust times so we don't include the range of these markers themselves.
     endTime -= this.MARKER_EPSILON;
     startTime += startTime !== 0 ? this.MARKER_EPSILON : 0;
 
     OverviewView.setTimeInterval({ startTime, endTime });
     DetailsView.selectView("memory-calltree");
--- a/toolkit/devtools/server/tests/browser/browser_markers-gc.js
+++ b/toolkit/devtools/server/tests/browser/browser_markers-gc.js
@@ -19,16 +19,18 @@ add_task(function*() {
   let rec = yield front.startRecording({ withMarkers: true });
 
   let markers = yield waitForMarkerType(front, MARKER_NAME);
   yield front.stopRecording(rec);
 
   ok(markers.some(m => m.name === MARKER_NAME), `got some ${MARKER_NAME} markers`);
   ok(markers.every(({causeName}) => typeof causeName === "string"),
     "All markers have a causeName.");
+  ok(markers.every(({cycle}) => typeof cycle === "number"),
+    "All markers have a `cycle` ID.");
 
   markers = rec.getMarkers();
 
   // Bug 1197646
   let ordered = true;
   markers.reduce((previousStart, current, i) => {
     if (i === 0) {
       return current.start;
--- a/toolkit/devtools/shared/timeline.js
+++ b/toolkit/devtools/shared/timeline.js
@@ -253,27 +253,27 @@ let Timeline = exports.Timeline = Class(
    * Fired when the Memory component emits a `garbage-collection` event. Used to
    * take the data and make it look like the rest of our markers.
    *
    * A GC "marker" here represents a full GC cycle, which may contain several incremental
    * events within its `collection` array. The marker contains a `reason` field, indicating
    * why there was a GC, and may contain a `nonincrementalReason` when SpiderMonkey could
    * not incrementally collect garbage.
    */
-  _onGarbageCollection: function ({ collections, reason, nonincrementalReason }) {
+  _onGarbageCollection: function ({ collections, gcCycleNumber, reason, nonincrementalReason }) {
     if (!this._isRecording || !this.docShells.length) {
       return;
     }
 
     let endTime = this.docShells[0].now();
 
     events.emit(this, "markers", collections.map(({ startTimestamp: start, endTimestamp: end }) => {
       return {
         name: "GarbageCollection",
         causeName: reason,
         nonincrementalReason: nonincrementalReason,
-        // Both timestamps are in microseconds -- convert to milliseconds to match other markers
-        start: start,
-        end: end
+        cycle: gcCycleNumber,
+        start,
+        end,
       };
     }), endTime);
   },
 });