Bug 1158645 - Calculate FPS in performance tool by counting frames over duration, rather than averaging values of reported framerate timestamps. r=vp
authorJordan Santell <jsantell@gmail.com>
Wed, 29 Apr 2015 13:53:19 -0700
changeset 273071 1b3bee7ca060658b090051d44a3112db940011a0
parent 273070 80cb5f706116b7b7abf72cc64ff73bf0b49409db
child 273072 bd128b529a76471a43a8dee4bd2f1234f9c97b27
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvp
bugs1158645
milestone40.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 1158645 - Calculate FPS in performance tool by counting frames over duration, rather than averaging values of reported framerate timestamps. r=vp
browser/devtools/performance/modules/graphs.js
browser/devtools/shared/test/browser.ini
browser/devtools/shared/test/browser_graphs-15.js
browser/devtools/shared/widgets/Graphs.jsm
browser/devtools/shared/widgets/GraphsWorker.js
--- a/browser/devtools/performance/modules/graphs.js
+++ b/browser/devtools/performance/modules/graphs.js
@@ -105,17 +105,17 @@ PerformanceGraph.prototype = Heritage.ex
 function FramerateGraph(parent) {
   PerformanceGraph.call(this, parent, ProfilerGlobal.L10N.getStr("graphs.fps"));
 }
 
 FramerateGraph.prototype = Heritage.extend(PerformanceGraph.prototype, {
   mainColor: FRAMERATE_GRAPH_COLOR_NAME,
   setPerformanceData: function ({ duration, ticks }, resolution) {
     this.dataDuration = duration;
-    return this.setDataFromTimestamps(ticks, resolution);
+    return this.setDataFromTimestamps(ticks, resolution, duration);
   }
 });
 
 /**
  * Constructor for the memory graph. Inherits from PerformanceGraph.
  *
  * @param nsIDOMNode parent
  *        The parent node holding the overview.
--- a/browser/devtools/shared/test/browser.ini
+++ b/browser/devtools/shared/test/browser.ini
@@ -67,16 +67,17 @@ support-files =
 [browser_graphs-10b.js]
 [browser_graphs-11a.js]
 [browser_graphs-11b.js]
 [browser_graphs-12.js]
 [browser_graphs-13.js]
 [browser_graphs-14.js]
 [browser_inplace-editor-01.js]
 [browser_inplace-editor-02.js]
+[browser_graphs-15.js]
 [browser_layoutHelpers.js]
 skip-if = e10s # Layouthelpers test should not run in a content page.
 [browser_layoutHelpers-getBoxQuads.js]
 skip-if = e10s # Layouthelpers test should not run in a content page.
 [browser_mdn-docs-01.js]
 [browser_mdn-docs-02.js]
 [browser_num-l10n.js]
 [browser_observableobject.js]
new file mode 100644
--- /dev/null
+++ b/browser/devtools/shared/test/browser_graphs-15.js
@@ -0,0 +1,47 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests that graph widgets correctly emit mouse input events.
+
+const FAST_FPS = 60;
+const SLOW_FPS = 10;
+// Each element represents a second
+const FRAMES= [FAST_FPS, FAST_FPS, FAST_FPS, SLOW_FPS, FAST_FPS];
+const TEST_DATA = [];
+const INTERVAL = 100;
+const DURATION = 5000; // 5s
+let t = 0;
+for (let frameRate of FRAMES) {
+  for (let i = 0; i < frameRate; i++) {
+    let delta = Math.floor(1000 / frameRate); // Duration between frames at this rate
+    t += delta;
+    TEST_DATA.push(t);
+  }
+}
+
+let {LineGraphWidget} = Cu.import("resource:///modules/devtools/Graphs.jsm", {});
+let {Promise} = devtools.require("resource://gre/modules/Promise.jsm");
+
+add_task(function*() {
+  yield promiseTab("about:blank");
+  yield performTest();
+  gBrowser.removeCurrentTab();
+});
+
+function* performTest() {
+  let [host, win, doc] = yield createHost();
+  let graph = new LineGraphWidget(doc.body, "fps");
+
+  yield testGraph(graph);
+
+  yield graph.destroy();
+  host.destroy();
+}
+
+function* testGraph(graph) {
+
+  console.log("test data", TEST_DATA);
+  yield graph.setDataFromTimestamps(TEST_DATA, INTERVAL, DURATION);
+  is(graph._avgTooltip.querySelector("[text=value]").textContent, "50",
+    "The average tooltip displays the correct value.");
+}
--- a/browser/devtools/shared/widgets/Graphs.jsm
+++ b/browser/devtools/shared/widgets/Graphs.jsm
@@ -1297,24 +1297,25 @@ LineGraphWidget.prototype = Heritage.ext
    * framerate, for example, from a sequence of timestamps.
    *
    * @param array timestamps
    *        A list of numbers representing time, ordered ascending. For example,
    *        this can be the raw data received from the framerate actor, which
    *        represents the elapsed time on each refresh driver tick.
    * @param number interval
    *        The maximum amount of time to wait between calculations.
+   * @param number duration
+   *        The duration of the recording in milliseconds.
    */
-  setDataFromTimestamps: Task.async(function*(timestamps, interval) {
+  setDataFromTimestamps: Task.async(function*(timestamps, interval, duration) {
     let {
       plottedData,
       plottedMinMaxSum
     } = yield CanvasGraphUtils._performTaskInWorker("plotTimestampsGraph", {
-      timestamps: timestamps,
-      interval: interval
+      timestamps, interval, duration
     });
 
     this._tempMinMaxSum = plottedMinMaxSum;
     this.setData(plottedData);
   }),
 
   /**
    * Renders the graph's data source.
--- a/browser/devtools/shared/widgets/GraphsWorker.js
+++ b/browser/devtools/shared/widgets/GraphsWorker.js
@@ -16,43 +16,48 @@ self.onmessage = e => {
   }
 };
 
 /**
  * @see LineGraphWidget.prototype.setDataFromTimestamps in Graphs.jsm
  * @param number id
  * @param array timestamps
  * @param number interval
+ * @param number duration
  */
 function plotTimestampsGraph(id, args) {
   let plottedData = plotTimestamps(args.timestamps, args.interval);
-  let plottedMinMaxSum = getMinMaxSum(plottedData);
+  let plottedMinMaxSum = getMinMaxAvg(plottedData, args.timestamps, args.duration);
 
   let response = { id, plottedData, plottedMinMaxSum };
   self.postMessage(response);
 }
 
 /**
  * Gets the min, max and average of the values in an array.
  * @param array source
+ * @param array timestamps
+ * @param number duration
  * @return object
  */
-function getMinMaxSum(source) {
+function getMinMaxAvg(source, timestamps, duration) {
   let totalTicks = source.length;
+  let totalFrames = timestamps.length;
   let maxValue = Number.MIN_SAFE_INTEGER;
   let minValue = Number.MAX_SAFE_INTEGER;
-  let avgValue = 0;
-  let sumValues = 0;
+  // Calculate the average by counting how many frames occurred
+  // in the duration of the recording, rather than average the frame points
+  // we have, as that weights higher FPS, as there'll be more timestamps for those
+  // values
+  let avgValue = totalFrames / (duration / 1000);
 
   for (let { value } of source) {
     maxValue = Math.max(value, maxValue);
     minValue = Math.min(value, minValue);
-    sumValues += value;
   }
-  avgValue = sumValues / totalTicks;
 
   return { minValue, maxValue, avgValue };
 }
 
 /**
  * Takes a list of numbers and plots them on a line graph representing
  * the rate of occurences in a specified interval.
  *