Bug 1246017 - Create the parent map in the HeapAnalysesWorker; r=jimb

devtools/client/memory/actions/diffing.js

@@ -50,7 +50,7 @@ const takeCensusDiff = exports.takeCensusDiff = function (heapWorker, first, sec
     assert(snapshotIsDiffable(second),
            `Second snapshot must be in a diffable state, found ${second.state}`);
 
-    let report;
+    let report, parentMap;
     let inverted = getState().inverted;
     let breakdown = getState().breakdown;
     let filter = getState().filter;

devtools/client/memory/actions/snapshot.js

@@ -136,7 +136,7 @@ const takeCensus = exports.takeCensus = function (heapWorker, id) {
     assert([states.READ, states.SAVED_CENSUS].includes(snapshot.state),
       `Can only take census of snapshots in READ or SAVED_CENSUS state, found ${snapshot.state}`);
 
-    let report;
+    let report, parentMap;
     let inverted = getState().inverted;
     let breakdown = getState().breakdown;
     let filter = getState().filter;

devtools/client/memory/components/census.js

@@ -32,7 +32,7 @@ const Census = module.exports = createClass({
     } = this.props;
 
     const report = census.report;
-    let parentMap = createParentMap(report);
+    let parentMap = census.parentMap;
     const { totalBytes, totalCount } = report;
 
     const getPercentBytes = totalBytes === 0

devtools/client/memory/components/dominator-tree.js

@@ -4,9 +4,10 @@
 
 const { DOM: dom, createClass, PropTypes, createFactory } = require("devtools/client/shared/vendor/react");
 const { assert, safeErrorString } = require("devtools/shared/DevToolsUtils");
+const { createParentMap } = require("devtools/shared/heapsnapshot/CensusUtils");
 const Tree = createFactory(require("devtools/client/shared/components/tree"));
 const DominatorTreeItem = createFactory(require("./dominator-tree-item"));
-const { createParentMap, L10N } = require("../utils");
+const { L10N } = require("../utils");
 const { TREE_ROW_HEIGHT, dominatorTreeState } = require("../constants");
 const { dominatorTreeModel } = require("../models");
 const DominatorTreeLazyChildren = require("../dominator-tree-lazy-children");

devtools/client/memory/models.js

@@ -56,6 +56,8 @@ let breakdownModel = exports.breakdown = PropTypes.shape({
 let censusModel = exports.censusModel = PropTypes.shape({
   // The current census report data.
   report: PropTypes.object,
+  // The parent map for the report.
+  parentMap: PropTypes.object,
   // The breakdown used to generate the current census
   breakdown: breakdownModel,
   // Whether the currently cached report tree is inverted or not.

devtools/client/memory/reducers/diffing.js

@@ -81,6 +81,7 @@ handlers[actions.TAKE_CENSUS_DIFF_END] = function (diffing, action) {
     state: diffingState.TOOK_DIFF,
     census: {
       report: action.report,
+      parentMap: action.parentMap,
       expanded: new Set(),
       inverted: action.inverted,
       filter: action.filter,

devtools/client/memory/reducers/snapshots.js

@@ -67,9 +67,15 @@ handlers[actions.TAKE_CENSUS_START] = function (snapshots, { id, breakdown, inve
   });
 };
 
-handlers[actions.TAKE_CENSUS_END] = function (snapshots, { id, report, breakdown, inverted, filter }) {
+handlers[actions.TAKE_CENSUS_END] = function (snapshots, { id,
+                                                           report,
+                                                           parentMap,
+                                                           breakdown,
+                                                           inverted,
+                                                           filter }) {
   const census = {
     report,
+    parentMap,
     expanded: new Set(),
     breakdown,
     inverted,

devtools/client/memory/store.js

@@ -16,7 +16,8 @@ module.exports = function () {
   // we'll later attach to the store
   if (DevToolsUtils.testing) {
     history = [];
-    shouldLog = true;
+    // Uncomment this for TONS of logging in tests.
+    // shouldLog = true;
   }
 
   let store = createStore({

devtools/client/memory/utils.js

@@ -561,25 +561,3 @@ exports.formatPercent = function(percent, showSign = false) {
   return exports.L10N.getFormatStr("tree-item.percent",
                            exports.formatNumber(percent, showSign));
 };
-
-/**
- * Creates a hash map mapping node IDs to its parent node.
- *
- * @param {CensusTreeNode} node
- * @param {Object<number, TreeNode>} aggregator
- *
- * @return {Object<number, TreeNode>}
- */
-const createParentMap = exports.createParentMap = function (node,
-                                                            getId = node => node.id,
-                                                            aggregator = Object.create(null)) {
-  if (node.children) {
-    for (let i = 0, length = node.children.length; i < length; i++) {
-      const child = node.children[i];
-      aggregator[getId(child)] = node;
-      createParentMap(child, getId, aggregator);
-    }
-  }
-
-  return aggregator;
-};

devtools/shared/heapsnapshot/CensusUtils.js

@@ -360,3 +360,25 @@ function diff(breakdown, startCensus, endCensus) {
   return visitor.results();
 };
 exports.diff = diff
+
+/**
+ * Creates a hash map mapping node IDs to its parent node.
+ *
+ * @param {CensusTreeNode} node
+ * @param {Object<number, TreeNode>} aggregator
+ *
+ * @return {Object<number, TreeNode>}
+ */
+const createParentMap = exports.createParentMap = function (node,
+                                                            getId = node => node.id,
+                                                            aggregator = Object.create(null)) {
+  if (node.children) {
+    for (let i = 0, length = node.children.length; i < length; i++) {
+      const child = node.children[i];
+      aggregator[getId(child)] = getId(node);
+      createParentMap(child, getId, aggregator);
+    }
+  }
+
+  return aggregator;
+};

devtools/shared/heapsnapshot/HeapAnalysesClient.js

@@ -21,7 +21,7 @@ var workerCounter = 0;
 const HeapAnalysesClient = module.exports = function () {
   this._worker = new DevToolsWorker(WORKER_URL, {
     name: `HeapAnalyses-${workerCounter++}`,
-    verbose: DevToolsUtils.dumpn.wantLogging
+    verbose: DevToolsUtils.dumpv.wantLogging
   });
 };
 
@@ -104,10 +104,15 @@ HeapAnalysesClient.prototype.getCreationTime = function (snapshotFilePath) {
  *          A filter string to prune the resulting tree with. Only applies if
  *          either asTreeNode or asInvertedTreeNode is true.
  *
- * @returns Promise<census report|CensusTreeNode>
+ * @returns Promise<Object>
- *          The report generated by the given census breakdown, or
+ *          An object with the following properties:
- *          a CensusTreeNode generated by the given census breakdown
+ *          - report:
- *          if `asTreeNode` is true.
+ *            The report generated by the given census breakdown, or a
+ *            CensusTreeNode generated by the given census breakdown if
+ *            `asTreeNode` is true.
+ *          - parentMap:
+ *            The result of calling CensusUtils.createParentMap on the generated
+ *            report. Only exists if asTreeNode or asInvertedTreeNode are set.
  */
 HeapAnalysesClient.prototype.takeCensus = function (snapshotFilePath,
                                                     censusOptions,
@@ -147,10 +152,14 @@ HeapAnalysesClient.prototype.takeCensus = function (snapshotFilePath,
  *          A filter string to prune the resulting tree with. Only applies if
  *          either asTreeNode or asInvertedTreeNode is true.
  *
- * @returns Promise<delta report|CensusTreeNode>
+ * @returns Promise<Object>
+ *          - delta:
  *            The delta report generated by diffing the two census reports, or a
  *            CensusTreeNode generated from the delta report if
  *            `requestOptions.asTreeNode` was true.
+ *          - parentMap:
+ *            The result of calling CensusUtils.createParentMap on the generated
+ *            delta. Only exists if asTreeNode or asInvertedTreeNode are set.
  */
 HeapAnalysesClient.prototype.takeCensusDiff = function (firstSnapshotFilePath,
                                                         secondSnapshotFilePath,

devtools/shared/heapsnapshot/HeapAnalysesWorker.js

@@ -58,17 +58,19 @@ workerHelper.createTask(self, "takeCensus", ({ snapshotFilePath, censusOptions,
     throw new Error(`No known heap snapshot for '${snapshotFilePath}'`);
   }
 
-  const report = snapshots[snapshotFilePath].takeCensus(censusOptions);
+  let report = snapshots[snapshotFilePath].takeCensus(censusOptions);
+  let parentMap;
 
   if (requestOptions.asTreeNode || requestOptions.asInvertedTreeNode) {
     const opts = { filter: requestOptions.filter || null };
     if (requestOptions.asInvertedTreeNode) {
       opts.invert = true;
     }
-    return censusReportToCensusTreeNode(censusOptions.breakdown, report, opts);
+    report = censusReportToCensusTreeNode(censusOptions.breakdown, report, opts);
+    parentMap = CensusUtils.createParentMap(report);
   }
 
-  return report;
+  return { report, parentMap };
 });
 
 /**
@@ -92,17 +94,19 @@ workerHelper.createTask(self, "takeCensusDiff", request => {
 
   const first = snapshots[firstSnapshotFilePath].takeCensus(censusOptions);
   const second = snapshots[secondSnapshotFilePath].takeCensus(censusOptions);
-  const delta = CensusUtils.diff(censusOptions.breakdown, first, second);
+  let delta = CensusUtils.diff(censusOptions.breakdown, first, second);
+  let parentMap;
 
   if (requestOptions.asTreeNode || requestOptions.asInvertedTreeNode) {
     const opts = { filter: requestOptions.filter || null };
     if (requestOptions.asInvertedTreeNode) {
       opts.invert = true;
     }
-    return censusReportToCensusTreeNode(censusOptions.breakdown, delta, opts);
+    delta = censusReportToCensusTreeNode(censusOptions.breakdown, delta, opts);
+    parentMap = CensusUtils.createParentMap(delta);
   }
 
-  return delta;
+  return { delta, parentMap };
 });
 
 /**

devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensusDiff_01.js

@@ -30,14 +30,14 @@ add_task(function* () {
   yield client.readHeapSnapshot(secondSnapshotFilePath);
   ok(true, "Should have read both heap snapshot files");
 
-  const delta = yield client.takeCensusDiff(firstSnapshotFilePath,
+  const { delta } = yield client.takeCensusDiff(firstSnapshotFilePath,
                                                 secondSnapshotFilePath,
                                                 { breakdown: BREAKDOWN });
 
   equal(delta.AllocationMarker.count, 1,
     "There exists one new AllocationMarker in the second heap snapshot");
 
-  const deltaTreeNode = yield client.takeCensusDiff(firstSnapshotFilePath,
+  const { delta: deltaTreeNode } = yield client.takeCensusDiff(firstSnapshotFilePath,
                                                                secondSnapshotFilePath,
                                                                { breakdown: BREAKDOWN },
                                                                { asTreeNode: true });

devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensusDiff_02.js

@@ -39,11 +39,11 @@ add_task(function* () {
 
   ok(true, "Should have read both heap snapshot files");
 
-  const delta = yield client.takeCensusDiff(firstSnapshotFilePath,
+  const { delta } = yield client.takeCensusDiff(firstSnapshotFilePath,
                                                 secondSnapshotFilePath,
                                                 { breakdown: BREAKDOWN });
 
-  const deltaTreeNode = yield client.takeCensusDiff(firstSnapshotFilePath,
+  const { delta: deltaTreeNode } = yield client.takeCensusDiff(firstSnapshotFilePath,
                                                                secondSnapshotFilePath,
                                                                { breakdown: BREAKDOWN },
                                                                { asInvertedTreeNode: true });

devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_01.js

@@ -14,7 +14,7 @@ add_task(function* () {
   yield client.readHeapSnapshot(snapshotFilePath);
   ok(true, "Should have read the heap snapshot");
 
-  const report = yield client.takeCensus(snapshotFilePath);
+  const { report } = yield client.takeCensus(snapshotFilePath);
   ok(report, "Should get a report");
   equal(typeof report, "object", "report should be an object");
 

devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_02.js

@@ -15,7 +15,7 @@ add_task(function* () {
   yield client.readHeapSnapshot(snapshotFilePath);
   ok(true, "Should have read the heap snapshot");
 
-  const report = yield client.takeCensus(snapshotFilePath, {
+  const { report } = yield client.takeCensus(snapshotFilePath, {
     breakdown: { by: "count", count: true, bytes: true }
   });
 

devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_04.js

@@ -49,7 +49,7 @@ add_task(function* test() {
   // Run a census broken down by class name -> allocation stack so we can grab
   // only the AllocationMarker objects we have complete control over.
 
-  const report = yield client.takeCensus(snapshotFilePath, {
+  const { report } = yield client.takeCensus(snapshotFilePath, {
     breakdown: { by: 'objectClass',
                  then: { by: 'allocationStack',
                          then: { by: 'count',

devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_05.js

@@ -20,11 +20,11 @@ add_task(function* () {
   yield client.readHeapSnapshot(snapshotFilePath);
   ok(true, "Should have read the heap snapshot");
 
-  const report = yield client.takeCensus(snapshotFilePath, {
+  const { report } = yield client.takeCensus(snapshotFilePath, {
     breakdown: BREAKDOWN
   });
 
-  const treeNode = yield client.takeCensus(snapshotFilePath, {
+  const { report: treeNode } = yield client.takeCensus(snapshotFilePath, {
     breakdown: BREAKDOWN
   }, {
     asTreeNode: true

devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_06.js

@@ -59,11 +59,11 @@ add_task(function* () {
   yield client.readHeapSnapshot(snapshotFilePath);
   ok(true, "Should have read the heap snapshot");
 
-  const report = yield client.takeCensus(snapshotFilePath, {
+  const { report } = yield client.takeCensus(snapshotFilePath, {
     breakdown: BREAKDOWN
   });
 
-  const treeNode = yield client.takeCensus(snapshotFilePath, {
+  const { report: treeNode } = yield client.takeCensus(snapshotFilePath, {
     breakdown: BREAKDOWN
   }, {
     asTreeNode: true

devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_07.js

@@ -36,11 +36,11 @@ add_task(function* () {
   yield client.readHeapSnapshot(snapshotFilePath);
   ok(true, "Should have read the heap snapshot");
 
-  const report = yield client.takeCensus(snapshotFilePath, {
+  const { report } = yield client.takeCensus(snapshotFilePath, {
     breakdown: BREAKDOWN
   });
 
-  const treeNode = yield client.takeCensus(snapshotFilePath, {
+  const { report: treeNode } = yield client.takeCensus(snapshotFilePath, {
     breakdown: BREAKDOWN
   }, {
     asInvertedTreeNode: true