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

This commit is contained in:
Nick Fitzgerald 2016-02-05 15:11:48 -08:00
parent db9bc3593c
commit b8817f5424
20 changed files with 100 additions and 72 deletions

View File

@ -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;
@ -87,10 +87,10 @@ const takeCensusDiff = exports.takeCensusDiff = function (heapWorker, first, sec
opts.filter = filter || null;
try {
report = yield heapWorker.takeCensusDiff(first.path,
second.path,
{ breakdown },
opts);
({ delta: report, parentMap } = yield heapWorker.takeCensusDiff(first.path,
second.path,
{ breakdown },
opts));
} catch (error) {
reportException("actions/diffing/takeCensusDiff", error);
dispatch({ type: actions.DIFFING_ERROR, error });
@ -106,6 +106,7 @@ const takeCensusDiff = exports.takeCensusDiff = function (heapWorker, first, sec
first,
second,
report,
parentMap,
inverted,
filter,
breakdown,

View File

@ -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;
@ -166,7 +166,9 @@ const takeCensus = exports.takeCensus = function (heapWorker, id) {
opts.filter = filter || null;
try {
report = yield heapWorker.takeCensus(snapshot.path, { breakdown }, opts);
({ report, parentMap } = yield heapWorker.takeCensus(snapshot.path,
{ breakdown },
opts));
} catch (error) {
reportException("takeCensus", error);
dispatch({ type: actions.SNAPSHOT_ERROR, id, error });
@ -183,7 +185,8 @@ const takeCensus = exports.takeCensus = function (heapWorker, id) {
breakdown,
inverted,
filter,
report
report,
parentMap
});
telemetry.countCensus({ inverted, filter, breakdown });

View File

@ -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

View File

@ -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");

View File

@ -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.

View File

@ -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,

View File

@ -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,

View File

@ -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({

View File

@ -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;
};

View File

@ -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;
};

View File

@ -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>
* The report generated by the given census breakdown, or
* a CensusTreeNode generated by the given census breakdown
* if `asTreeNode` is true.
* @returns Promise<Object>
* 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.
*/
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>
* The delta report generated by diffing the two census reports, or a
* CensusTreeNode generated from the delta report if
* `requestOptions.asTreeNode` was true.
* @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,

View File

@ -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 };
});
/**

View File

@ -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 deltaTreeNode = yield client.takeCensusDiff(firstSnapshotFilePath,
secondSnapshotFilePath,
{ breakdown: BREAKDOWN },
{ asTreeNode: true });
const { delta: deltaTreeNode } = yield client.takeCensusDiff(firstSnapshotFilePath,
secondSnapshotFilePath,
{ breakdown: BREAKDOWN },
{ asTreeNode: true });
// Have to manually set these because symbol properties aren't structured
// cloned.

View File

@ -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 deltaTreeNode = yield client.takeCensusDiff(firstSnapshotFilePath,
secondSnapshotFilePath,
{ breakdown: BREAKDOWN },
{ asInvertedTreeNode: true });
const { delta: deltaTreeNode } = yield client.takeCensusDiff(firstSnapshotFilePath,
secondSnapshotFilePath,
{ breakdown: BREAKDOWN },
{ asInvertedTreeNode: true });
// Have to manually set these because symbol properties aren't structured
// cloned.

View File

@ -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");

View File

@ -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 }
});

View File

@ -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',

View File

@ -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

View File

@ -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

View File

@ -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