diff --git a/devtools/client/memory/actions/diffing.js b/devtools/client/memory/actions/diffing.js index 9ffb8dd65e27..e742952ed438 100644 --- a/devtools/client/memory/actions/diffing.js +++ b/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, parentMap; + let report; let inverted = getState().inverted; let breakdown = getState().breakdown; let filter = getState().filter; diff --git a/devtools/client/memory/actions/snapshot.js b/devtools/client/memory/actions/snapshot.js index b27f8581499e..5b39e3400730 100644 --- a/devtools/client/memory/actions/snapshot.js +++ b/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, parentMap; + let report; let inverted = getState().inverted; let breakdown = getState().breakdown; let filter = getState().filter; diff --git a/devtools/client/memory/components/census.js b/devtools/client/memory/components/census.js index fb78648a234a..5480b2145342 100644 --- a/devtools/client/memory/components/census.js +++ b/devtools/client/memory/components/census.js @@ -32,7 +32,7 @@ const Census = module.exports = createClass({ } = this.props; const report = census.report; - let parentMap = census.parentMap; + let parentMap = createParentMap(report); const { totalBytes, totalCount } = report; const getPercentBytes = totalBytes === 0 diff --git a/devtools/client/memory/components/dominator-tree.js b/devtools/client/memory/components/dominator-tree.js index 4609f73b0b4c..a7bfb34f1719 100644 --- a/devtools/client/memory/components/dominator-tree.js +++ b/devtools/client/memory/components/dominator-tree.js @@ -4,10 +4,9 @@ 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 { L10N } = require("../utils"); +const { createParentMap, L10N } = require("../utils"); const { TREE_ROW_HEIGHT, dominatorTreeState } = require("../constants"); const { dominatorTreeModel } = require("../models"); const DominatorTreeLazyChildren = require("../dominator-tree-lazy-children"); diff --git a/devtools/client/memory/models.js b/devtools/client/memory/models.js index 1600e392364c..c84902a07cb6 100644 --- a/devtools/client/memory/models.js +++ b/devtools/client/memory/models.js @@ -56,8 +56,6 @@ 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. diff --git a/devtools/client/memory/reducers/diffing.js b/devtools/client/memory/reducers/diffing.js index d9ed22974a49..5678f737e2c2 100644 --- a/devtools/client/memory/reducers/diffing.js +++ b/devtools/client/memory/reducers/diffing.js @@ -81,7 +81,6 @@ 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, diff --git a/devtools/client/memory/reducers/snapshots.js b/devtools/client/memory/reducers/snapshots.js index 202d0cc78173..b79bfbe78b6c 100644 --- a/devtools/client/memory/reducers/snapshots.js +++ b/devtools/client/memory/reducers/snapshots.js @@ -67,15 +67,9 @@ handlers[actions.TAKE_CENSUS_START] = function (snapshots, { id, breakdown, inve }); }; -handlers[actions.TAKE_CENSUS_END] = function (snapshots, { id, - report, - parentMap, - breakdown, - inverted, - filter }) { +handlers[actions.TAKE_CENSUS_END] = function (snapshots, { id, report, breakdown, inverted, filter }) { const census = { report, - parentMap, expanded: new Set(), breakdown, inverted, diff --git a/devtools/client/memory/store.js b/devtools/client/memory/store.js index 7d5ca8001245..85dde609c915 100644 --- a/devtools/client/memory/store.js +++ b/devtools/client/memory/store.js @@ -16,8 +16,7 @@ module.exports = function () { // we'll later attach to the store if (DevToolsUtils.testing) { history = []; - // Uncomment this for TONS of logging in tests. - // shouldLog = true; + shouldLog = true; } let store = createStore({ diff --git a/devtools/client/memory/utils.js b/devtools/client/memory/utils.js index b122e1f5c5b8..403c81fea9ab 100644 --- a/devtools/client/memory/utils.js +++ b/devtools/client/memory/utils.js @@ -561,3 +561,25 @@ 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} aggregator + * + * @return {Object} + */ +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; +}; diff --git a/devtools/shared/heapsnapshot/CensusUtils.js b/devtools/shared/heapsnapshot/CensusUtils.js index 200825c44fca..9d50971c7933 100644 --- a/devtools/shared/heapsnapshot/CensusUtils.js +++ b/devtools/shared/heapsnapshot/CensusUtils.js @@ -360,25 +360,3 @@ 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} aggregator - * - * @return {Object} - */ -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; -}; diff --git a/devtools/shared/heapsnapshot/HeapAnalysesClient.js b/devtools/shared/heapsnapshot/HeapAnalysesClient.js index 1a2ab23cc949..6dea624b15d1 100644 --- a/devtools/shared/heapsnapshot/HeapAnalysesClient.js +++ b/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.dumpv.wantLogging + verbose: DevToolsUtils.dumpn.wantLogging }); }; @@ -104,15 +104,10 @@ 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 - * An object with the following properties: - * - report: - * 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. + * @returns Promise + * The report generated by the given census breakdown, or + * a CensusTreeNode generated by the given census breakdown + * if `asTreeNode` is true. */ HeapAnalysesClient.prototype.takeCensus = function (snapshotFilePath, censusOptions, @@ -152,14 +147,10 @@ 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: - * 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. + * @returns Promise + * The delta report generated by diffing the two census reports, or a + * CensusTreeNode generated from the delta report if + * `requestOptions.asTreeNode` was true. */ HeapAnalysesClient.prototype.takeCensusDiff = function (firstSnapshotFilePath, secondSnapshotFilePath, diff --git a/devtools/shared/heapsnapshot/HeapAnalysesWorker.js b/devtools/shared/heapsnapshot/HeapAnalysesWorker.js index a01d9fc964e3..8c39d188aa51 100644 --- a/devtools/shared/heapsnapshot/HeapAnalysesWorker.js +++ b/devtools/shared/heapsnapshot/HeapAnalysesWorker.js @@ -58,19 +58,17 @@ workerHelper.createTask(self, "takeCensus", ({ snapshotFilePath, censusOptions, throw new Error(`No known heap snapshot for '${snapshotFilePath}'`); } - let report = snapshots[snapshotFilePath].takeCensus(censusOptions); - let parentMap; + const report = snapshots[snapshotFilePath].takeCensus(censusOptions); if (requestOptions.asTreeNode || requestOptions.asInvertedTreeNode) { const opts = { filter: requestOptions.filter || null }; if (requestOptions.asInvertedTreeNode) { opts.invert = true; } - report = censusReportToCensusTreeNode(censusOptions.breakdown, report, opts); - parentMap = CensusUtils.createParentMap(report); + return censusReportToCensusTreeNode(censusOptions.breakdown, report, opts); } - return { report, parentMap }; + return report; }); /** @@ -94,19 +92,17 @@ workerHelper.createTask(self, "takeCensusDiff", request => { const first = snapshots[firstSnapshotFilePath].takeCensus(censusOptions); const second = snapshots[secondSnapshotFilePath].takeCensus(censusOptions); - let delta = CensusUtils.diff(censusOptions.breakdown, first, second); - let parentMap; + const delta = CensusUtils.diff(censusOptions.breakdown, first, second); if (requestOptions.asTreeNode || requestOptions.asInvertedTreeNode) { const opts = { filter: requestOptions.filter || null }; if (requestOptions.asInvertedTreeNode) { opts.invert = true; } - delta = censusReportToCensusTreeNode(censusOptions.breakdown, delta, opts); - parentMap = CensusUtils.createParentMap(delta); + return censusReportToCensusTreeNode(censusOptions.breakdown, delta, opts); } - return { delta, parentMap }; + return delta; }); /** diff --git a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensusDiff_01.js b/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensusDiff_01.js index 6f22cbad3998..3f980a16d60a 100644 --- a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensusDiff_01.js +++ b/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensusDiff_01.js @@ -30,17 +30,17 @@ add_task(function* () { yield client.readHeapSnapshot(secondSnapshotFilePath); ok(true, "Should have read both heap snapshot files"); - const { delta } = yield client.takeCensusDiff(firstSnapshotFilePath, - secondSnapshotFilePath, - { breakdown: BREAKDOWN }); + 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 { delta: deltaTreeNode } = yield client.takeCensusDiff(firstSnapshotFilePath, - secondSnapshotFilePath, - { breakdown: BREAKDOWN }, - { asTreeNode: true }); + const deltaTreeNode = yield client.takeCensusDiff(firstSnapshotFilePath, + secondSnapshotFilePath, + { breakdown: BREAKDOWN }, + { asTreeNode: true }); // Have to manually set these because symbol properties aren't structured // cloned. diff --git a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensusDiff_02.js b/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensusDiff_02.js index f1ba9ce84922..a27be892c239 100644 --- a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensusDiff_02.js +++ b/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensusDiff_02.js @@ -39,14 +39,14 @@ add_task(function* () { ok(true, "Should have read both heap snapshot files"); - const { delta } = yield client.takeCensusDiff(firstSnapshotFilePath, - secondSnapshotFilePath, - { breakdown: BREAKDOWN }); + const delta = yield client.takeCensusDiff(firstSnapshotFilePath, + secondSnapshotFilePath, + { breakdown: BREAKDOWN }); - const { delta: deltaTreeNode } = yield client.takeCensusDiff(firstSnapshotFilePath, - secondSnapshotFilePath, - { breakdown: BREAKDOWN }, - { asInvertedTreeNode: true }); + const deltaTreeNode = yield client.takeCensusDiff(firstSnapshotFilePath, + secondSnapshotFilePath, + { breakdown: BREAKDOWN }, + { asInvertedTreeNode: true }); // Have to manually set these because symbol properties aren't structured // cloned. diff --git a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_01.js b/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_01.js index e26981db4c4f..bb41923dca7b 100644 --- a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_01.js +++ b/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"); diff --git a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_02.js b/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_02.js index 34494af704b3..1696acf781c1 100644 --- a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_02.js +++ b/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 } }); diff --git a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_04.js b/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_04.js index 4366a66ca1aa..c285f089b8be 100644 --- a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_04.js +++ b/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', diff --git a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_05.js b/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_05.js index 7e16d9f00f39..be7ee53498c9 100644 --- a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_05.js +++ b/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 { report: treeNode } = yield client.takeCensus(snapshotFilePath, { + const treeNode = yield client.takeCensus(snapshotFilePath, { breakdown: BREAKDOWN }, { asTreeNode: true diff --git a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_06.js b/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_06.js index 7795a97007da..9064f77fa697 100644 --- a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_06.js +++ b/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 { report: treeNode } = yield client.takeCensus(snapshotFilePath, { + const treeNode = yield client.takeCensus(snapshotFilePath, { breakdown: BREAKDOWN }, { asTreeNode: true diff --git a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_07.js b/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_07.js index 986b3aaa808a..6f5c3d184f0d 100644 --- a/devtools/shared/heapsnapshot/tests/unit/test_HeapAnalyses_takeCensus_07.js +++ b/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 { report: treeNode } = yield client.takeCensus(snapshotFilePath, { + const treeNode = yield client.takeCensus(snapshotFilePath, { breakdown: BREAKDOWN }, { asInvertedTreeNode: true