Skip to content

Commit

Permalink
Bug 1650151 - Merge webIsolated processes when diffing with non-Fissi…
Browse files Browse the repository at this point in the history
…on memory reports. r=njn

Fission processes show up in about:memory as webIsolated=<URL>. This is
great when comparing two reports from a single session, to precisely
see how the memory usage of a particular process has changed, but it
makes it harder to compare to a non-Fission memory report. For instance,
when loading a news page with many ad iframes, what was a single process
without Fission enabled will be split across more than a dozen processes.

This patch fixes this by making it so that if exactly one memory report
being diffed contains a webIsolated process, then webIsolated processes
will be treated like regular web processes.

Differential Revision: https://phabricator.services.mozilla.com/D85608
  • Loading branch information
amccreight committed Aug 14, 2020
1 parent 1971ae6 commit 3c8ea64
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 4 deletions.
36 changes: 33 additions & 3 deletions toolkit/components/aboutmemory/content/aboutMemory.js
Original file line number Diff line number Diff line change
Expand Up @@ -935,15 +935,31 @@ DReport.PRESENT_IN_FIRST_ONLY = 1;
DReport.PRESENT_IN_SECOND_ONLY = 2;
DReport.ADDED_FOR_BALANCE = 3;

/**
* Return true if the report contains a webIsolated process,
* which is a good indication that Fission is enabled.
*/
function hasWebIsolatedProcess(aJSONReports) {
for (let jr of aJSONReports) {
assert(jr.process !== undefined, "Missing process");
if (jr.process.startsWith("webIsolated")) {
return true;
}
}
return false;
}

/**
* Make a report map, which has combined path+process strings for keys, and
* DReport objects for values.
*
* @param aJSONReports
* The |reports| field of a JSON object.
* @param aForgetIsolation
If this is true, treat webIsolated processes like web processes.
* @return The constructed report map.
*/
function makeDReportMap(aJSONReports) {
function makeDReportMap(aJSONReports, aForgetIsolation) {
let dreportMap = {};
for (let jr of aJSONReports) {
assert(jr.process !== undefined, "Missing process");
Expand All @@ -968,6 +984,10 @@ function makeDReportMap(aJSONReports) {
let process = jr.process.replace(pidRegex, pidSubst);
let path = jr.path.replace(pidRegex, pidSubst);

if (aForgetIsolation && process.startsWith("webIsolated")) {
process = "web (pid NNN)";
}

// Strip TIDs and threadpool IDs.
path = path.replace(/\(tid=(\d+)\)/, "(tid=NNN)");
path = path.replace(/#\d+ \(tid=NNN\)/, "#N (tid=NNN)");
Expand Down Expand Up @@ -1103,15 +1123,25 @@ function diffJSONObjects(aJson1, aJson2) {
return aJson1[aProp];
}

// If one report we're diffing contains webIsolated processes, but the other
// does not, then we're probably comparing a report with Fission enabled with
// one where it is not enabled. In this case, we want to make all of the
// webIsolated processes look like plain old web processes to get a better
// diff.
let hasIsolated1 = hasWebIsolatedProcess(aJson1.reports);
let hasIsolated2 = hasWebIsolatedProcess(aJson2.reports);
let eitherIsolated = hasIsolated1 || hasIsolated2;
let forgetIsolation = hasIsolated1 != hasIsolated2 && eitherIsolated;

return {
version: simpleProp("version"),

hasMozMallocUsableSize: simpleProp("hasMozMallocUsableSize"),

reports: makeJSONReports(
diffDReportMaps(
makeDReportMap(aJson1.reports),
makeDReportMap(aJson2.reports)
makeDReportMap(aJson1.reports, forgetIsolation),
makeDReportMap(aJson2.reports, forgetIsolation)
)
),
};
Expand Down
2 changes: 2 additions & 0 deletions toolkit/components/aboutmemory/tests/chrome.ini
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ support-files =
crash-dump-diff1.json
crash-dump-diff2.json
crash-dump-good.json
fiss-diff1.json
fiss-diff2.json
memory-reports-bad.json
memory-reports-diff1.json
memory-reports-diff2.json
Expand Down
18 changes: 18 additions & 0 deletions toolkit/components/aboutmemory/tests/fiss-diff1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"foo": 1,
"blah": 2,
"memory_report": {
"version": 1,
"hasMozMallocUsableSize": true,
"reports": [
{"process": "P (pid 12345)", "path": "explicit/foobar", "kind": 1, "units": 0, "amount": 100, "description": "Desc."},
{"process": "P (pid 12345)", "path": "explicit/zero1", "kind": 1, "units": 0, "amount": 0, "description": "Desc."},
{"process": "P (pid 12345)", "path": "heap-allocated", "kind": 2, "units": 0, "amount": 10000000, "description": "Heap allocated."},

{"process": "web (pid 12345)", "path": "explicit/a/b", "kind": 1, "units": 0, "amount": 2000000, "description": "Desc."},
{"process": "web (pid 12345)", "path": "explicit/a/c/d", "kind": 1, "units": 0, "amount": 2000000, "description": "Desc."},
{"process": "web (pid 12345)", "path": "heap-allocated", "kind": 2, "units": 0, "amount": 10000000, "description": "Heap allocated."}
]
}
}

18 changes: 18 additions & 0 deletions toolkit/components/aboutmemory/tests/fiss-diff2.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"foo": 1,
"blah": 2,
"memory_report": {
"version": 1,
"hasMozMallocUsableSize": true,
"reports": [
{"process": "P (pid 12345)", "path": "explicit/foobar", "kind": 1, "units": 0, "amount": 400, "description": "Desc."},
{"process": "P (pid 12345)", "path": "explicit/zero1", "kind": 1, "units": 0, "amount": 0, "description": "Desc."},
{"process": "P (pid 12345)", "path": "heap-allocated", "kind": 2, "units": 0, "amount": 13000000, "description": "Heap allocated."},

{"process": "webIsolated=https://example.com (pid 12345)", "path": "explicit/a/b", "kind": 1, "units": 0, "amount": 2000000, "description": "Desc."},
{"process": "webIsolated=https://example.com (pid 12345)", "path": "explicit/a/c/d", "kind": 1, "units": 0, "amount": 3000000, "description": "Desc."},
{"process": "webIsolated=https://example.com (pid 12345)", "path": "heap-allocated", "kind": 2, "units": 0, "amount": 12000000, "description": "Heap allocated."}
]
}
}

33 changes: 32 additions & 1 deletion toolkit/components/aboutmemory/tests/test_aboutmemory3.xhtml
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,34 @@ Other Measurements\n\
1 B ── heap-allocated\n\
\n\
End of Main Process (pid NNN)\n\
";

let expectedDiffFiss =
"\
P (pid NNN)\n\
Explicit Allocations\n\
\n\
3,000,000 B (100.0%) -- explicit\n\
├──2,999,700 B (99.99%) ── heap-unclassified\n\
└────────300 B (00.01%) ── foobar\n\
\n\
Other Measurements\n\
\n\
3,000,000 B ── heap-allocated\n\
\n\
End of P (pid NNN)\n\
web (pid NNN)\n\
Explicit Allocations\n\
\n\
2,000,000 B (100.0%) -- explicit\n\
├──1,000,000 B (50.00%) ── a/c/d\n\
└──1,000,000 B (50.00%) ── heap-unclassified\n\
\n\
Other Measurements\n\
\n\
2,000,000 B ── heap-allocated\n\
\n\
End of web (pid NNN)\n\
";

let frames = [
Expand All @@ -518,7 +546,10 @@ End of Main Process (pid NNN)\n\
{ filename: "memory-reports-diff1.json", filename2: "memory-reports-diff2.json", expected: expectedDiffVerbose, dumpFirst: false, verbose: true },

// This diffs two pre-existing crash report files.
{ filename: "crash-dump-diff1.json", filename2: "crash-dump-diff2.json", expected: expectedDiff2, dumpFirst: false, verbose: true }
{ filename: "crash-dump-diff1.json", filename2: "crash-dump-diff2.json", expected: expectedDiff2, dumpFirst: false, verbose: true },

// This diffs a non-Fission and a Fission memory report.
{ filename: "fiss-diff1.json", filename2: "fiss-diff2.json", expected: expectedDiffFiss, dumpFirst: false, verbose: true }
];

SimpleTest.waitForFocus(chain(frames));
Expand Down

0 comments on commit 3c8ea64

Please sign in to comment.