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 259728 a812ab9b77bb23fec84afbb303d927100cf7a448
parent 259727 86aaa9aafca2ad79e3989844eb1fc094c7cc55a3
child 259729 8b78514a72da184b5e481f7dfa5cf0fe9378b0a3
push id29289
push userryanvm@gmail.com
push dateFri, 28 Aug 2015 12:44:52 +0000
treeherdermozilla-central@c559c25f4954 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1198019
milestone43.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 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);
   },
 });