Bug 857382 (part 1) - Correctly handle memory report files that have no measurements in the "explicit" or "other" sections. r=kats.
authorNicholas Nethercote <nnethercote@mozilla.com>
Sun, 21 Apr 2013 13:48:01 -0700
changeset 140974 8f6969de65be8e339eafb7f0ed818156f1548e33
parent 140973 0c9257e3c7a7d45c081756736a8ca34d462c5ec6
child 140975 0640cd38f16cf0cfcae54d108781ceba7b4cab45
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs857382
milestone23.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 857382 (part 1) - Correctly handle memory report files that have no measurements in the "explicit" or "other" sections. r=kats.
toolkit/components/aboutmemory/content/aboutMemory.js
toolkit/components/aboutmemory/tests/memory-reports-good.json
toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
toolkit/components/aboutmemory/tests/test_aboutmemory4.xul
--- a/toolkit/components/aboutmemory/content/aboutMemory.js
+++ b/toolkit/components/aboutmemory/content/aboutMemory.js
@@ -506,16 +506,18 @@ function onLoadAboutMemory()
         let filename = searchSplit[i].substring('file='.length);
         updateAboutMemoryFromFile(decodeURIComponent(filename));
         return;
       }
     }
   }
 }
 
+//---------------------------------------------------------------------------
+
 function doGC()
 {
   Cu.forceGC();
   let os = Cc["@mozilla.org/observer-service;1"]
              .getService(Ci.nsIObserverService);
   os.notifyObservers(null, "child-gc-request", null);
   updateMainAndFooter("Garbage collection completed", HIDE_FOOTER);
 }
@@ -537,18 +539,16 @@ function doMMU()
     () => updateMainAndFooter("Memory minimization completed", HIDE_FOOTER));
 }
 
 function doMeasure()
 {
   addChildObserversAndUpdate(updateAboutMemoryFromReporters);
 }
 
-//---------------------------------------------------------------------------
-
 /**
  * Top-level function that does the work of generating the page from the memory
  * reporters.
  */
 function updateAboutMemoryFromReporters()
 {
   // First, clear the contents of main.  Necessary because
   // updateAboutMemoryFromReporters() might be called more than once due to the
@@ -685,54 +685,62 @@ function updateAboutMemoryFromClipboard(
     // Success!  Now use the string to generate about:memory.
     updateAboutMemoryFromJSONString(cbString);
 
   } catch (ex) {
     handleException(ex);
   }
 }
 
+//---------------------------------------------------------------------------
+
+// |PColl| is short for "process collection".
+function PColl()
+{
+  this._trees = {};
+  this._degenerates = {};
+  this._heapTotal = 0;
+}
+
 /**
  * Processes reports (whether from reporters or from a file) and append the
  * main part of the page.
  *
  * @param aProcess
  *        Function that extracts the memory reports from the reporters or from
  *        file.
  * @param aHasMozMallocUsableSize
  *        Boolean indicating if moz_malloc_usable_size works.
  * @param aForceShowSmaps
  *        True if we should show the smaps memory reporters even if we're not
  *        in verbose mode.
  */
 function appendAboutMemoryMain(aProcess, aHasMozMallocUsableSize,
                                aForceShowSmaps)
 {
-  let treesByProcess = {}, degeneratesByProcess = {}, heapTotalByProcess = {};
-  getTreesByProcess(aProcess, treesByProcess, degeneratesByProcess,
-                    heapTotalByProcess, aForceShowSmaps);
+  let pcollsByProcess = getPCollsByProcess(aProcess, aForceShowSmaps);
 
-  // Sort our list of processes.
-  let processes = Object.keys(treesByProcess);
+  // Sort the processes.
+  let processes = Object.keys(pcollsByProcess);
   processes.sort(function(aProcessA, aProcessB) {
     assert(aProcessA != aProcessB,
            "Elements of Object.keys() should be unique, but " +
            "saw duplicate '" + aProcessA + "' elem.");
 
     // Always put the main process first.
     if (aProcessA == gUnnamedProcessStr) {
       return -1;
     }
     if (aProcessB == gUnnamedProcessStr) {
       return 1;
     }
 
     // Then sort by resident size.
-    let nodeA = degeneratesByProcess[aProcessA]['resident'];
-    let nodeB = degeneratesByProcess[aProcessB]['resident'];
+    let nodeA = pcollsByProcess[aProcessA]._degenerates['resident'];
+    let nodeB = pcollsByProcess[aProcessB]._degenerates['resident'];
     let residentA = nodeA ? nodeA._amount : -1;
     let residentB = nodeB ? nodeB._amount : -1;
 
     if (residentA > residentB) {
       return -1;
     }
     if (residentA < residentB) {
       return 1;
@@ -750,54 +758,44 @@ function appendAboutMemoryMain(aProcess,
   });
 
   // Generate output for each process.
   for (let i = 0; i < processes.length; i++) {
     let process = processes[i];
     let section = appendElement(gMain, 'div', 'section');
 
     appendProcessAboutMemoryElements(section, process,
-                                     treesByProcess[process],
-                                     degeneratesByProcess[process],
-                                     heapTotalByProcess[process],
+                                     pcollsByProcess[process]._trees,
+                                     pcollsByProcess[process]._degenerates,
+                                     pcollsByProcess[process]._heapTotal,
                                      aHasMozMallocUsableSize);
   }
 }
 
-//---------------------------------------------------------------------------
-
-// This regexp matches sentences and sentence fragments, i.e. strings that
-// start with a capital letter and ends with a '.'.  (The final sentence may be
-// in parentheses, so a ')' might appear after the '.'.)
-const gSentenceRegExp = /^[A-Z].*\.\)?$/m;
-
 /**
  * This function reads all the memory reports, and puts that data in structures
  * that will be used to generate the page.
  *
  * @param aProcessMemoryReports
  *        Function that extracts the memory reports from the reporters or from
  *        file.
- * @param aTreesByProcess
- *        Table of non-degenerate trees, indexed by process, which this
- *        function appends to.
- * @param aDegeneratesByProcess
- *        Table of degenerate trees, indexed by process, which this function
- *        appends to.
- * @param aHeapTotalByProcess
- *        Table of heap total counts, indexed by process, which this function
- *        appends to.
  * @param aForceShowSmaps
  *        True if we should show the smaps memory reporters even if we're not
  *        in verbose mode.
+ * @return The table of PColls by process.
  */
-function getTreesByProcess(aProcessMemoryReports, aTreesByProcess,
-                           aDegeneratesByProcess, aHeapTotalByProcess,
-                           aForceShowSmaps)
+function getPCollsByProcess(aProcessMemoryReports, aForceShowSmaps)
 {
+  let pcollsByProcess = {};
+
+  // This regexp matches sentences and sentence fragments, i.e. strings that
+  // start with a capital letter and ends with a '.'.  (The final sentence may
+  // be in parentheses, so a ')' might appear after the '.'.)
+  const gSentenceRegExp = /^[A-Z].*\.\)?$/m;
+
   // Ignore the "smaps" multi-reporter in non-verbose mode unless we're reading
   // from a file or the clipboard, and ignore the "compartments" and
   // "ghost-windows" multi-reporters all the time.  (Note that reports from
   // these multi-reporters can reach here as single reports if they were in the
   // child process.)
   //
   // Also ignore the resident-fast reporter; we use the vanilla resident
   // reporter because it's more important that we get accurate results than
@@ -844,30 +842,27 @@ function getTreesByProcess(aProcessMemor
                   "non-sentence other description");
     }
 
     let process = aProcess === "" ? gUnnamedProcessStr : aProcess;
     let unsafeNames = aUnsafePath.split('/');
     let unsafeName0 = unsafeNames[0];
     let isDegenerate = unsafeNames.length === 1;
 
-    // Get the appropriate trees table (non-degenerate or degenerate) for the
-    // process, creating it if necessary.
-    let t;
-    let thingsByProcess =
-      isDegenerate ? aDegeneratesByProcess : aTreesByProcess;
-    let things = thingsByProcess[process];
-    if (!thingsByProcess[process]) {
-      things = thingsByProcess[process] = {};
+    // Get the PColl table for the process, creating it if necessary.
+    let pcoll = pcollsByProcess[process];
+    if (!pcollsByProcess[process]) {
+      pcoll = pcollsByProcess[process] = new PColl();
     }
 
     // Get the root node, creating it if necessary.
-    t = things[unsafeName0];
+    let psubcoll = isDegenerate ? pcoll._degenerates : pcoll._trees;
+    let t = psubcoll[unsafeName0];
     if (!t) {
-      t = things[unsafeName0] =
+      t = psubcoll[unsafeName0] =
         new TreeNode(unsafeName0, aUnits, isDegenerate);
     }
 
     if (!isDegenerate) {
       // Add any missing nodes in the tree implied by aUnsafePath, and fill in
       // the properties that we can with a top-down traversal.
       for (let i = 1; i < unsafeNames.length; i++) {
         let unsafeName = unsafeNames[i];
@@ -879,35 +874,34 @@ function getTreesByProcess(aProcessMemor
           }
           t._kids.push(u);
         }
         t = u;
       }
 
       // Update the heap total if necessary.
       if (unsafeName0 === "explicit" && aKind == KIND_HEAP) {
-        if (!aHeapTotalByProcess[process]) {
-          aHeapTotalByProcess[process] = 0;
-        }
-        aHeapTotalByProcess[process] += aAmount;
+        pcollsByProcess[process]._heapTotal += aAmount;
       }
     }
 
     if (t._amount) {
       // Duplicate!  Sum the values and mark it as a dup.
       t._amount += aAmount;
       t._nMerged = t._nMerged ? t._nMerged + 1 : 2;
     } else {
       // New leaf node.  Fill in extra details node from the report.
       t._amount = aAmount;
       t._description = aDescription;
     }
   }
 
   aProcessMemoryReports(ignoreSingle, ignoreMulti, handleReport);
+
+  return pcollsByProcess;
 }
 
 //---------------------------------------------------------------------------
 
 // There are two kinds of TreeNode.
 // - Leaf TreeNodes correspond to reports.
 // - Non-leaf TreeNodes are just scaffolding nodes for the tree;  their values
 //   are derived from their children.
@@ -1216,45 +1210,46 @@ function appendProcessAboutMemoryElement
 
   // We'll fill this in later.
   let warningsDiv = appendElement(aP, "div", "accuracyWarning");
 
   // The explicit tree.
   let hasKnownHeapAllocated;
   {
     let treeName = "explicit";
+    let pre = appendSectionHeader(aP, kSectionNames[treeName]);
     let t = aTrees[treeName];
-    assertInput(t, "no explicit reports");
-    fillInTree(t);
-    hasKnownHeapAllocated =
-      aDegenerates &&
-      addHeapUnclassifiedNode(t, aDegenerates["heap-allocated"], aHeapTotal);
-    sortTreeAndInsertAggregateNodes(t._amount, t);
-    t._description = kTreeDescriptions[treeName];
-    let pre = appendSectionHeader(aP, kSectionNames[treeName]);
-    appendTreeElements(pre, t, aProcess, "");
+    if (t) {
+      fillInTree(t);
+      hasKnownHeapAllocated =
+        aDegenerates &&
+        addHeapUnclassifiedNode(t, aDegenerates["heap-allocated"], aHeapTotal);
+      sortTreeAndInsertAggregateNodes(t._amount, t);
+      t._description = kTreeDescriptions[treeName];
+      appendTreeElements(pre, t, aProcess, "");
+      delete aTrees[treeName];
+    }
     appendTextNode(aP, "\n");  // gives nice spacing when we cut and paste
-    delete aTrees[treeName];
   }
 
   // The smaps trees, which are only present in aTrees in verbose mode or when
   // we're reading from a file or the clipboard.
   kSmapsTreeNames.forEach(function(aTreeName) {
     // |t| will be undefined if we don't have any reports for the given
     // unsafePath.
     let t = aTrees[aTreeName];
     if (t) {
+      let pre = appendSectionHeader(aP, kSectionNames[aTreeName]);
       fillInTree(t);
       sortTreeAndInsertAggregateNodes(t._amount, t);
       t._description = kTreeDescriptions[aTreeName];
       t._hideKids = true;   // smaps trees are always initially collapsed
-      let pre = appendSectionHeader(aP, kSectionNames[aTreeName]);
       appendTreeElements(pre, t, aProcess, "");
+      delete aTrees[aTreeName];
       appendTextNode(aP, "\n");  // gives nice spacing when we cut and paste
-      delete aTrees[aTreeName];
     }
   });
 
   // Fill in and sort all the non-degenerate other trees.
   let otherTrees = [];
   for (let unsafeName in aTrees) {
     let t = aTrees[unsafeName];
     assert(!t._isDegenerate, "tree is degenerate");
--- a/toolkit/components/aboutmemory/tests/memory-reports-good.json
+++ b/toolkit/components/aboutmemory/tests/memory-reports-good.json
@@ -1,10 +1,16 @@
 {
   "version": 1,
   "hasMozMallocUsableSize": true,
   "reports": [
     {"process": "Main Process (pid NNN)", "path": "heap-allocated", "kind": 2, "units": 0, "amount": 262144000, "description": "Heap allocated."},
     {"process": "Main Process (pid NNN)", "path": "other/b", "kind": 2, "units": 0, "amount": 104857, "description": "Other b."},
     {"process": "Main Process (pid NNN)", "path": "other/a", "kind": 2, "units": 0, "amount": 209715, "description": "Other a."},
-    {"process": "Main Process (pid NNN)", "path": "explicit/a/b", "kind": 1, "units": 0, "amount": 52428800, "description": "A b."}
+    {"process": "Main Process (pid NNN)", "path": "explicit/a/b", "kind": 1, "units": 0, "amount": 52428800, "description": "A b."},
+
+    {"process": "Explicit-only process", "path": "explicit/a/b", "kind": 1, "units": 0, "amount": 100000, "description": "A b."},
+
+    {"process": "Other-only process", "path": "a/b", "kind": 1, "units": 0, "amount": 100000, "description": "A b."},
+    {"process": "Other-only process", "path": "a/c", "kind": 1, "units": 0, "amount": 100000, "description": "A c."},
+    {"process": "Other-only process", "path": "heap-allocated", "kind": 1, "units": 0, "amount": 500000, "description": "D."}
   ]
 }
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory3.xul
@@ -170,20 +170,60 @@
     let x = aFrameIds.shift();
     if (x) {
       return function() { test(x.frameId, x.filename, x.expected, x.dumpFirst, chain(aFrameIds)); }
     } else {
       return function() { finish(); };
     }
   }
 
-  // This is pretty simple output, but that's ok;  this file is about testing
-  // the loading of data from file.  If we got this far, we're doing fine.
   let expectedGood =
 "\
+Explicit-only process\n\
+\n\
+WARNING: the 'heap-allocated' memory reporter does not work for this platform and/or configuration. This means that 'heap-unclassified' is not shown and the 'explicit' tree shows less memory than it should.\n\
+Explicit Allocations\n\
+\n\
+0.10 MB (100.0%) -- explicit\n\
+└──0.10 MB (100.0%) ── a/b\n\
+\n\
+Other Measurements\n\
+\n\
+Main Process (pid NNN)\n\
+Explicit Allocations\n\
+\n\
+250.00 MB (100.0%) -- explicit\n\
+├──200.00 MB (80.00%) ── heap-unclassified\n\
+└───50.00 MB (20.00%) ── a/b\n\
+\n\
+Other Measurements\n\
+\n\
+0.30 MB (100.0%) -- other\n\
+├──0.20 MB (66.67%) ── a\n\
+└──0.10 MB (33.33%) ── b\n\
+\n\
+250.00 MB ── heap-allocated\n\
+\n\
+Other-only process\n\
+\n\
+WARNING: the 'heap-allocated' memory reporter does not work for this platform and/or configuration. This means that 'heap-unclassified' is not shown and the 'explicit' tree shows less memory than it should.\n\
+Explicit Allocations\n\
+\n\
+Other Measurements\n\
+\n\
+0.19 MB (100.0%) -- a\n\
+├──0.10 MB (50.00%) ── b\n\
+└──0.10 MB (50.00%) ── c\n\
+\n\
+0.48 MB ── heap-allocated\n\
+\n\
+";
+
+  let expectedGood2 =
+"\
 Main Process (pid NNN)\n\
 Explicit Allocations\n\
 \n\
 250.00 MB (100.0%) -- explicit\n\
 ├──200.00 MB (80.00%) ── heap-unclassified\n\
 └───50.00 MB (20.00%) ── a/b\n\
 \n\
 Other Measurements\n\
@@ -201,17 +241,17 @@ 250.00 MB ── heap-allocated\n\
 "\
 Invalid memory report(s): missing 'hasMozMallocUsableSize' property";
 
   let frames = [
     // This loads a pre-existing file that is valid.
     { frameId: "amGoodFrame", filename: "memory-reports-good.json", expected: expectedGood, dumpFirst: false },
 
     // This dumps to a file and then reads it back in.  The output is the same as the first test.
-    { frameId: "amGoodFrame2", filename: "memory-reports-dumped.json.gz", expected: expectedGood, dumpFirst: true },
+    { frameId: "amGoodFrame2", filename: "memory-reports-dumped.json.gz", expected: expectedGood2, dumpFirst: true },
 
     // This loads a pre-existing file that is invalid.
     { frameId: "amBadFrame",  filename: "memory-reports-bad.json",  expected: expectedBad, dumpFirst: false }
   ];
 
   SimpleTest.waitForFocus(chain(frames));
 
   SimpleTest.waitForExplicitFinish();
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory4.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory4.xul
@@ -84,31 +84,54 @@
       return function() { SimpleTest.finish(); };
     }
   }
 
   // This is pretty simple output, but that's ok;  this file is about testing
   // the loading of data from file.  If we got this far, we're doing fine.
   let expectedGood =
 "\
+Explicit-only process\n\
+\n\
+WARNING: the 'heap-allocated' memory reporter does not work for this platform and/or configuration. This means that 'heap-unclassified' is not shown and the 'explicit' tree shows less memory than it should.\n\
+Explicit Allocations\n\
+\n\
+0.10 MB (100.0%) -- explicit\n\
+└──0.10 MB (100.0%) ── a/b\n\
+\n\
+Other Measurements\n\
+\n\
 Main Process (pid NNN)\n\
 Explicit Allocations\n\
 \n\
 250.00 MB (100.0%) -- explicit\n\
 ├──200.00 MB (80.00%) ── heap-unclassified\n\
 └───50.00 MB (20.00%) ── a/b\n\
 \n\
 Other Measurements\n\
 \n\
 0.30 MB (100.0%) -- other\n\
 ├──0.20 MB (66.67%) ── a\n\
 └──0.10 MB (33.33%) ── b\n\
 \n\
 250.00 MB ── heap-allocated\n\
 \n\
+Other-only process\n\
+\n\
+WARNING: the 'heap-allocated' memory reporter does not work for this platform and/or configuration. This means that 'heap-unclassified' is not shown and the 'explicit' tree shows less memory than it should.\n\
+Explicit Allocations\n\
+\n\
+Other Measurements\n\
+\n\
+0.19 MB (100.0%) -- a\n\
+├──0.10 MB (50.00%) ── b\n\
+└──0.10 MB (50.00%) ── c\n\
+\n\
+0.48 MB ── heap-allocated\n\
+\n\
 ";
 
   // This is the output for a malformed data file.
   let expectedBad =
 "\
 Invalid memory report(s): missing 'hasMozMallocUsableSize' property";
 
   let frames = [