Bug 1191508 - Do not serve counts from the memory module in the performance tools; this should be handled on the client. r=fitzgen
authorJordan Santell <jsantell@mozilla.com>
Fri, 07 Aug 2015 10:38:26 -0700
changeset 288499 4438dc641c8e0d09c2d4c689ce8ab5066db60935
parent 288461 3ccebe8a8e612e96054f56d4b10e55a054fc1b65
child 288500 0c01ce7ee9feb3a29358e69656fd09cbbac84e7e
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1191508
milestone42.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 1191508 - Do not serve counts from the memory module in the performance tools; this should be handled on the client. r=fitzgen
browser/devtools/performance/modules/logic/actors.js
browser/devtools/performance/modules/logic/io.js
browser/devtools/performance/modules/logic/recording-model.js
browser/devtools/performance/modules/logic/recording-utils.js
browser/devtools/performance/test/browser_perf-allocations-to-samples.js
browser/devtools/performance/test/browser_perf-compatibility-02.js
browser/devtools/performance/test/browser_perf-compatibility-04.js
browser/devtools/performance/test/browser_perf-compatibility-08.js
browser/devtools/performance/test/browser_perf_recordings-io-04.js
toolkit/devtools/server/actors/utils/stack.js
toolkit/devtools/shared/memory.js
--- a/browser/devtools/performance/modules/logic/actors.js
+++ b/browser/devtools/performance/modules/logic/actors.js
@@ -366,22 +366,17 @@ MemoryFrontFacade.prototype = {
     if ((yield this.getState()) !== "attached") {
       deferred.resolve();
       return;
     }
 
     let memoryData = yield this.getAllocations();
     // Match the signature of the TimelineFront events, with "timeline-data"
     // being the event name, and the second argument describing the type.
-    this.emit("timeline-data", "allocations", {
-      sites: memoryData.allocations,
-      timestamps: memoryData.allocationsTimestamps,
-      frames: memoryData.frames,
-      counts: memoryData.counts
-    });
+    this.emit("timeline-data", "allocations", memoryData);
 
     deferred.resolve();
   }),
 
   toString: () => "[object MemoryFrontFacade]"
 };
 
 // Bind all the methods that directly proxy to the actor
--- a/browser/devtools/performance/modules/logic/io.js
+++ b/browser/devtools/performance/modules/logic/io.js
@@ -140,17 +140,17 @@ function convertLegacyData (legacyData) 
   // fields just are empty arrays or objects.
   let data = {
     label: profilerData.profilerLabel,
     duration: recordingDuration,
     markers: [],
     frames: [],
     memory: [],
     ticks: ticksData,
-    allocations: { sites: [], timestamps: [], frames: [], counts: [] },
+    allocations: { sites: [], timestamps: [], frames: [] },
     profile: profilerData.profile,
     // Fake a configuration object here if there's tick data,
     // so that it can be rendered
     configuration: {
       withTicks: !!ticksData.length,
       withMarkers: false,
       withMemory: false,
       withAllocations: false
--- a/browser/devtools/performance/modules/logic/recording-model.js
+++ b/browser/devtools/performance/modules/logic/recording-model.js
@@ -111,17 +111,17 @@ RecordingModel.prototype = {
     this._bufferPercent = info.position !== void 0 ? 0 : null;
 
     this._recording = true;
 
     this._markers = [];
     this._frames = [];
     this._memory = [];
     this._ticks = [];
-    this._allocations = { sites: [], timestamps: [], frames: [], counts: [] };
+    this._allocations = { sites: [], timestamps: [], frames: [] };
   },
 
   /**
    * Called when the signal was sent to the front to no longer record more
    * data, and begin fetching the data. There's some delay during fetching,
    * even though the recording is stopped, the model is not yet completed until
    * all the data is fetched.
    */
@@ -365,23 +365,27 @@ RecordingModel.prototype = {
         this._ticks = timestamps;
         break;
       }
       // Accumulate allocation sites into an array. Furthermore, the timestamps
       // do not have a zero epoch, and are microseconds instead of milliseconds,
       // so offset all of them by the start time, also converting from ┬Ás to ms.
       case "allocations": {
         if (!config.withAllocations) { break; }
-        let [{ sites, timestamps, frames, counts }] = data;
+        let [{
+          allocations: sites,
+          allocationsTimestamps: timestamps,
+          frames,
+        }] = data;
+
         let timeOffset = this._memoryStartTime;
         RecordingUtils.offsetAndScaleTimestamps(timestamps, timeOffset);
         pushAll(this._allocations.sites, sites);
         pushAll(this._allocations.timestamps, timestamps);
         pushAll(this._allocations.frames, frames);
-        pushAll(this._allocations.counts, counts);
         break;
       }
     }
   },
 
   toString: () => "[object RecordingModel]"
 };
 
--- a/browser/devtools/performance/modules/logic/recording-utils.js
+++ b/browser/devtools/performance/modules/logic/recording-utils.js
@@ -129,17 +129,17 @@ let gProfileThreadFromAllocationCache = 
  *         The "profile" describing the allocations log.
  */
 function getProfileThreadFromAllocations(allocations) {
   let cached = gProfileThreadFromAllocationCache.get(allocations);
   if (cached) {
     return cached;
   }
 
-  let { sites, timestamps, frames, counts } = allocations;
+  let { sites, timestamps, frames } = allocations;
   let uniqueStrings = new UniqueStrings();
 
   // Convert allocation frames to the the stack and frame tables expected by
   // the profiler format.
   //
   // Since the allocations log is already presented as a tree, we would be
   // wasting time if we jumped through the same hoops as deflateProfile below
   // and instead use the existing structure of the allocations log to build up
@@ -215,27 +215,37 @@ function getProfileThreadFromAllocations
     if (frames[stackIndex]) {
       samples[writePos++] = [stackIndex, timestamps[i]];
     }
   }
   samples.length = writePos;
 
   let thread = {
     name: "allocations",
-    samples: samplesWithSchema(samples),
+    samples: allocationsWithSchema(samples),
     stackTable: stackTableWithSchema(stackTable),
     frameTable: frameTableWithSchema(frameTable),
-    stringTable: uniqueStrings.stringTable,
-    allocationsTable: counts
+    stringTable: uniqueStrings.stringTable
   };
 
   gProfileThreadFromAllocationCache.set(allocations, thread);
   return thread;
 }
 
+function allocationsWithSchema (data) {
+  let slot = 0;
+  return {
+    schema: {
+      stack: slot++,
+      time: slot++,
+    },
+    data: data
+  };
+}
+
 /**
  * Deduplicates a profile by deduplicating stacks, frames, and strings.
  *
  * This is used to adapt version 2 profiles from the backend to version 3, for
  * use with older Geckos (like B2G).
  *
  * Note that the schemas used by this must be kept in sync with schemas used
  * by the C++ UniqueStacks class in tools/profiler/ProfileEntry.cpp.
--- a/browser/devtools/performance/test/browser_perf-allocations-to-samples.js
+++ b/browser/devtools/performance/test/browser_perf-allocations-to-samples.js
@@ -34,31 +34,25 @@ let TEST_DATA = {
       parent: 1
     }, {
       source: "C",
       line: 5,
       column: 6,
       functionDisplayName: null,
       parent: 2
     }
-  ],
-  counts: [11, 22, 33, 44]
+  ]
 };
 
 let EXPECTED_OUTPUT = {
   name: "allocations",
   samples: {
     "schema": {
       "stack": 0,
       "time": 1,
-      "responsiveness": 2,
-      "rss": 3,
-      "uss": 4,
-      "frameNumber": 5,
-      "power": 6
     },
     data: [
       [ 1, 150 ],
       [ 2, 200 ],
       [ 3, 250 ]
     ]
   },
   stackTable: {
@@ -88,15 +82,9 @@ let EXPECTED_OUTPUT = {
       [ 2 ]
     ]
   },
   "stringTable": [
     "x (A:1:2)",
     "y (B:3:4)",
     "C:5:6"
   ],
-  "allocationsTable": [
-    11,
-    22,
-    33,
-    44
-  ]
 };
--- a/browser/devtools/performance/test/browser_perf-compatibility-02.js
+++ b/browser/devtools/performance/test/browser_perf-compatibility-02.js
@@ -34,17 +34,16 @@ let test = Task.async(function*() {
 
   isEmptyArray(markers, "markers");
   isEmptyArray(frames, "frames");
   isEmptyArray(memory, "memory");
   isEmptyArray(ticks, "ticks");
   isEmptyArray(allocations.sites, "allocations.sites");
   isEmptyArray(allocations.timestamps, "allocations.timestamps");
   isEmptyArray(allocations.frames, "allocations.frames");
-  isEmptyArray(allocations.counts, "allocations.counts");
 
   let sampleCount = 0;
 
   for (let thread of profile.threads) {
     info("Checking thread: " + thread.name);
 
     for (let sample of thread.samples.data) {
       sampleCount++;
--- a/browser/devtools/performance/test/browser_perf-compatibility-04.js
+++ b/browser/devtools/performance/test/browser_perf-compatibility-04.js
@@ -32,17 +32,16 @@ let test = Task.async(function*() {
 
   is(label, "", "Empty label for mock.");
   is(typeof duration, "number", "duration is a number");
   ok(duration > 0, "duration is not 0");
 
   isEmptyArray(allocations.sites, "allocations.sites");
   isEmptyArray(allocations.timestamps, "allocations.timestamps");
   isEmptyArray(allocations.frames, "allocations.frames");
-  isEmptyArray(allocations.counts, "allocations.counts");
 
   let sampleCount = 0;
 
   for (let thread of profile.threads) {
     info("Checking thread: " + thread.name);
 
     for (let sample of thread.samples.data) {
       sampleCount++;
--- a/browser/devtools/performance/test/browser_perf-compatibility-08.js
+++ b/browser/devtools/performance/test/browser_perf-compatibility-08.js
@@ -49,17 +49,16 @@ function *testMockMemory () {
     "Recording configuration set by target's support, not by UI prefs [No Memory Actor: withTicks]");
 
   ok(markers.length > 0, "markers exist.");
   ok(ticks.length > 0, "ticks exist.");
   isEmptyArray(memory, "memory");
   isEmptyArray(allocations.sites, "allocations.sites");
   isEmptyArray(allocations.timestamps, "allocations.timestamps");
   isEmptyArray(allocations.frames, "allocations.frames");
-  isEmptyArray(allocations.counts, "allocations.counts");
 
   is($("#overview-pane").hidden, false,
     "overview pane not hidden when server not supporting memory actors, yet UI prefs request them.");
   is($("#select-waterfall-view").hidden, false,
     "waterfall view button not hidden when memory mocked, and UI prefs enable them");
   is($("#select-js-calltree-view").hidden, false,
     "jscalltree view button not hidden when memory mocked, and UI prefs enable them");
   is($("#select-js-flamegraph-view").hidden, false,
@@ -106,17 +105,16 @@ function *testMockMemoryAndTimeline() {
   is(config.withTicks, false,
     "Recording configuration set by target's support, not by UI prefs [No Memory/Timeline Actor: withTicks]");
   isEmptyArray(markers, "markers");
   isEmptyArray(ticks, "ticks");
   isEmptyArray(memory, "memory");
   isEmptyArray(allocations.sites, "allocations.sites");
   isEmptyArray(allocations.timestamps, "allocations.timestamps");
   isEmptyArray(allocations.frames, "allocations.frames");
-  isEmptyArray(allocations.counts, "allocations.counts");
 
   is($("#overview-pane").hidden, true,
     "overview pane hidden when server not supporting memory/timeline actors, yet UI prefs request them.");
   is($("#select-waterfall-view").hidden, true,
     "waterfall view button hidden when memory/timeline mocked, and UI prefs enable them");
   is($("#select-js-calltree-view").hidden, false,
     "jscalltree view button not hidden when memory/timeline mocked, and UI prefs enable them");
   is($("#select-js-flamegraph-view").hidden, false,
--- a/browser/devtools/performance/test/browser_perf_recordings-io-04.js
+++ b/browser/devtools/performance/test/browser_perf_recordings-io-04.js
@@ -125,17 +125,17 @@ let test = Task.async(function*() {
   let expected = Object.create({
     label: "",
     duration: 10000,
     markers: [].toSource(),
     frames: [].toSource(),
     memory: [].toSource(),
     ticks: TICKS_DATA.toSource(),
     profile: RecordingUtils.deflateProfile(JSON.parse(JSON.stringify(PROFILER_DATA))).toSource(),
-    allocations: ({sites:[], timestamps:[], frames:[], counts:[]}).toSource(),
+    allocations: ({sites:[], timestamps:[], frames:[]}).toSource(),
     withTicks: true,
     withMemory: false,
     sampleFrequency: void 0
   });
 
   for (let field in expected) {
     if (!!~["withTicks", "withMemory", "sampleFrequency"].indexOf(field)) {
       is(importedData.configuration[field], expected[field], `${field} successfully converted in legacy import.`);
--- a/toolkit/devtools/server/actors/utils/stack.js
+++ b/toolkit/devtools/server/actors/utils/stack.js
@@ -12,85 +12,77 @@ let {Class} = require("sdk/core/heritage
  * index is used.  Users of the class can get an array of all frames
  * that have been added.
  */
 let StackFrameCache = Class({
   /**
    * Initialize this object.
    */
   initialize: function() {
-    this._framesToCounts = null;
     this._framesToIndices = null;
     this._framesToForms = null;
     this._lastEventSize = 0;
   },
 
   /**
    * Prepare to accept frames.
    */
   initFrames: function() {
-    if (this._framesToCounts) {
+    if (this._framesToIndices) {
       // The maps are already initialized.
       return;
     }
 
-    this._framesToCounts = new Map();
     this._framesToIndices = new Map();
     this._framesToForms = new Map();
     this._lastEventSize = 0;
   },
 
   /**
    * Forget all stored frames and reset to the initialized state.
    */
   clearFrames: function() {
-    this._framesToCounts.clear();
-    this._framesToCounts = null;
     this._framesToIndices.clear();
     this._framesToIndices = null;
     this._framesToForms.clear();
     this._framesToForms = null;
     this._lastEventSize = 0;
   },
 
   /**
    * Add a frame to this stack frame cache, and return the index of
    * the frame.
    */
   addFrame: function(frame) {
     this._assignFrameIndices(frame);
     this._createFrameForms(frame);
-    this._countFrame(frame);
     return this._framesToIndices.get(frame);
   },
 
   /**
    * A helper method for the memory actor.  This populates the packet
-   * object with "frames" and "counts" properties.  Each of these
+   * object with "frames" property. Each of these
    * properties will be an array indexed by frame ID.  "frames" will
-   * contain frame objects (see makeEvent) and "counts" will hold
-   * allocation counts for each frame.
+   * contain frame objects (see makeEvent).
    *
    * @param packet
    *        The packet to update.
    *
    * @returns packet
    */
   updateFramePacket: function(packet) {
     // Now that we are guaranteed to have a form for every frame, we know the
     // size the "frames" property's array must be. We use that information to
     // create dense arrays even though we populate them out of order.
     const size = this._framesToForms.size;
     packet.frames = Array(size).fill(null);
-    packet.counts = Array(size).fill(0);
 
-    // Populate the "frames" and "counts" properties.
+    // Populate the "frames" properties.
     for (let [stack, index] of this._framesToIndices) {
       packet.frames[index] = this._framesToForms.get(stack);
-      packet.counts[index] = this._framesToCounts.get(stack) || 0;
     }
 
     return packet;
   },
 
   /**
    * If any new stack frames have been added to this cache since the
    * last call to makeEvent (clearing the cache also resets the "last
@@ -183,26 +175,11 @@ let StackFrameCache = Class({
         asyncCause: frame.asyncCause
       };
       this._createFrameForms(frame.parent);
       this._createFrameForms(frame.asyncParent);
     }
 
     this._framesToForms.set(frame, form);
   },
-
-  /**
-   * Increment the allocation count for the provided frame.
-   *
-   * @param SavedFrame frame
-   *        The frame whose allocation count should be incremented.
-   */
-  _countFrame: function(frame) {
-    if (!this._framesToCounts.has(frame)) {
-      this._framesToCounts.set(frame, 1);
-    } else {
-      let count = this._framesToCounts.get(frame);
-      this._framesToCounts.set(frame, count + 1);
-    }
-  }
 });
 
 exports.StackFrameCache = StackFrameCache;
--- a/toolkit/devtools/shared/memory.js
+++ b/toolkit/devtools/shared/memory.js
@@ -231,37 +231,28 @@ let Memory = exports.Memory = Class({
    *                  line: <line number for this frame>,
    *                  column: <column number for this frame>,
    *                  source: <filename string for this frame>,
    *                  functionDisplayName: <this frame's inferred function name function or null>,
    *                  parent: <index into "frames">
    *                },
    *                ...
    *              ],
-   *              counts: [
-   *                <number of allocations in frames[0]>,
-   *                <number of allocations in frames[1]>,
-   *                <number of allocations in frames[2]>,
-   *                ...
-   *              ]
    *            }
    *
    *          The timestamps' unit is microseconds since the epoch.
    *
    *          Subsequent `getAllocations` request within the same recording and
    *          tab navigation will always place the same stack frames at the same
    *          indices as previous `getAllocations` requests in the same
    *          recording. In other words, it is safe to use the index as a
    *          unique, persistent id for its frame.
    *
    *          Additionally, the root node (null) is always at index 0.
    *
-   *          Note that the allocation counts include "self" allocations only,
-   *          and don't account for allocations in child frames.
-   *
    *          We use the indices into the "frames" array to avoid repeating the
    *          description of duplicate stack frames both when listing
    *          allocations, and when many stacks share the same tail of older
    *          frames. There shouldn't be any duplicates in the "frames" array,
    *          as that would defeat the purpose of this compression trick.
    *
    *          In the future, we might want to split out a frame's "source" and
    *          "functionDisplayName" properties out the same way we have split
@@ -278,19 +269,18 @@ let Memory = exports.Memory = Class({
       // the browser console so we at least know that it occurred.
       reportException("MemoryBridge.prototype.getAllocations",
                       "Warning: allocations log overflowed and lost some data.");
     }
 
     const allocations = this.dbg.memory.drainAllocationsLog()
     const packet = {
       allocations: [],
-      allocationsTimestamps: []
+      allocationsTimestamps: [],
     };
-
     for (let { frame: stack, timestamp } of allocations) {
       if (stack && Cu.isDeadWrapper(stack)) {
         continue;
       }
 
       // Safe because SavedFrames are frozen/immutable.
       let waived = Cu.waiveXrays(stack);