Bug 757299 (part 5) - Don't create Report objects in aboutMemory.js. r=jlebar.
authorNicholas Nethercote <nnethercote@mozilla.com>
Sun, 27 May 2012 16:13:50 -0700
changeset 99154 eff14a3bd0da51c814ca0db503ce569d38664735
parent 99153 f7894fb36f72b4495b5c992756cb59cc1659e181
child 99155 2f11a356655439157333a03e9399609c24c33755
push idunknown
push userunknown
push dateunknown
reviewersjlebar
bugs757299
milestone15.0a1
Bug 757299 (part 5) - Don't create Report objects in aboutMemory.js. r=jlebar.
toolkit/components/aboutmemory/content/aboutMemory.js
toolkit/components/aboutmemory/tests/test_aboutmemory.xul
--- a/toolkit/components/aboutmemory/content/aboutMemory.js
+++ b/toolkit/components/aboutmemory/content/aboutMemory.js
@@ -406,26 +406,30 @@ function updateAboutMemory()
   // First, clear the page contents.  Necessary because updateAboutMemory()
   // might be called more than once due to the "child-memory-reporter-update"
   // observer.
   let body = clearBody();
 
   let mgr = Cc["@mozilla.org/memory-reporter-manager;1"].
       getService(Ci.nsIMemoryReporterManager);
 
+  let treesByProcess = {}, othersByProcess = {};
+  getTreesAndOthersByProcess(mgr, treesByProcess, othersByProcess);
+
   // Generate output for one process at a time.  Always start with the
   // Main process.
-  let reportsByProcess = getReportsByProcess(mgr);
   let hasMozMallocUsableSize = mgr.hasMozMallocUsableSize;
-  appendProcessReportsElements(body, "Main", reportsByProcess["Main"],
-                               hasMozMallocUsableSize);
-  for (let process in reportsByProcess) {
+  appendProcessAboutMemoryElements(body, "Main", treesByProcess["Main"],
+                                   othersByProcess["Main"],
+                                   hasMozMallocUsableSize);
+  for (let process in treesByProcess) {
     if (process !== "Main") {
-      appendProcessReportsElements(body, process, reportsByProcess[process],
-                                   hasMozMallocUsableSize);
+      appendProcessAboutMemoryElements(body, process, treesByProcess[process],
+                                       othersByProcess[process],
+                                       hasMozMallocUsableSize);
     }
   }
 
   appendElement(body, "hr");
 
   // Memory-related actions.
   const UpDesc = "Re-measure.";
   const GCDesc = "Do a global garbage collection.";
@@ -473,38 +477,29 @@ function updateAboutMemory()
                     "to see a description of what it measures.";
 
   appendElementWithText(body, "div", "legend", legendText1);
   appendElementWithText(body, "div", "legend", legendText2);
 }
 
 //---------------------------------------------------------------------------
 
-function Report(aUnsafePath, aKind, aUnits, aAmount, aDescription)
-{
-  this._unsafePath  = aUnsafePath;
-  this._kind        = aKind;
-  this._units       = aUnits;
-  this._amount      = aAmount;
-  this._description = aDescription;
-  // this._nMerged is only defined if > 1
-  // this._done is defined and set to true when the Report's amount is read
-}
-
-Report.prototype = {
-  // Sum the values and mark |this| as a dup.  We mark dups because it's useful
-  // to know when a report is duplicated;  it might be worth investigating and
-  // splitting up to have non-duplicated names.
-  merge: function(r) {
-    this._amount += r._amount;
-    this._nMerged = this._nMerged ? this._nMerged + 1 : 2;
-  },
-};
-
-function getReportsByProcess(aMgr)
+/**
+ * This function reads all the memory reports, and puts that data in structures
+ * that will be used to generate the page.
+ *
+ * @param aMgr
+ *        The memory reporter manager.
+ * @param aTreesByProcess
+ *        Table of trees, indexed by process, which this function appends to.
+ * @param aOthersByProcess
+ *        Table of other lists, indexed by process, which this function appends
+ *        to.
+ */
+function getTreesAndOthersByProcess(aMgr, aTreesByProcess, aOthersByProcess)
 {
   // Ignore the "smaps" multi-reporter in non-verbose mode, and 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.)
 
   function ignoreSingle(aUnsafePath) 
   {
@@ -515,46 +510,82 @@ function getReportsByProcess(aMgr)
 
   function ignoreMulti(aMRName)
   {
     return (aMRName === "smaps" && !gVerbose) ||
            aMRName === "compartments" ||
            aMRName === "ghost-windows";
   }
 
-  let reportsByProcess = {};
-
   function handleReport(aProcess, aUnsafePath, aKind, aUnits, aAmount,
                         aDescription)
   {
     let process = aProcess === "" ? "Main" : aProcess;
-    let r = new Report(aUnsafePath, aKind, aUnits, aAmount, aDescription);
-    if (!reportsByProcess[process]) {
-      reportsByProcess[process] = {};
-    }
-    let reports = reportsByProcess[process];
-    let rOld = reports[r._unsafePath];
-    if (rOld) {
-      // Already an entry;  must be a duplicated report.  This can happen
-      // legitimately.  Merge them.
-      rOld.merge(r);
+
+    if (aUnsafePath.indexOf('/') !== -1) {
+      // Tree report.  Get the tree for the process, creating it if necessary.
+      // All the trees for each process ("explicit", "vsize", etc) are stored
+      // in a "tree-of-trees".  This makes things simple later.
+      if (!aTreesByProcess[process]) {
+        aTreesByProcess[process] = new TreeNode("tree-of-trees");
+      }
+      let t = aTreesByProcess[process]; 
+
+      // Add any missing nodes in the tree implied by aUnsafePath, and fill in
+      // the properties that we can with a top-down traversal.
+      let unsafeNames = aUnsafePath.split('/');
+      let u = t;
+      for (let i = 0; i < unsafeNames.length; i++) {
+        let unsafeName = unsafeNames[i];
+        let uMatch = u.findKid(unsafeName);
+        if (uMatch) {
+          u = uMatch;
+        } else {
+          let v = new TreeNode(unsafeName);
+          if (!u._kids) {
+            u._kids = [];
+          }
+          u._kids.push(v);
+          u = v;
+        }
+      }
+    
+      if (u._amount) {
+        // Duplicate!  Sum the values and mark it as a dup.
+        u._amount += aAmount;
+        u._nMerged = u._nMerged ? u._nMerged + 1 : 2;
+      } else {
+        // New leaf node.  Fill in extra details node from the report.
+        u._amount = aAmount;
+        u._description = aDescription;
+        u._kind = aKind;
+      }
+
     } else {
-      reports[r._unsafePath] = r;
+      // "Other" (non-tree) report.  Get the "others" for the process, creating
+      // it if necessary.
+      if (!aOthersByProcess[process]) {
+        aOthersByProcess[process] = {};
+      }
+      let others = aOthersByProcess[process]; 
+
+      // Record the report.
+      assert(!others[aUnsafePath], "dup'd OTHER report");
+      others[aUnsafePath] =
+        new OtherReport(aUnsafePath, aUnits, aAmount, aDescription);
     }
   }
 
   processMemoryReporters(aMgr, ignoreSingle, ignoreMulti, handleReport);
-
-  return reportsByProcess;
 }
 
 //---------------------------------------------------------------------------
 
 // There are two kinds of TreeNode.
-// - Leaf TreeNodes correspond to Reports and have more properties.
+// - Leaf TreeNodes correspond to reports.
 // - Non-leaf TreeNodes are just scaffolding nodes for the tree;  their values
 //   are derived from their children.
 function TreeNode(aUnsafeName)
 {
   // Nb: _units is not needed, it's always UNITS_BYTES.
   this._unsafeName = aUnsafeName;
   // Leaf TreeNodes have these properties added immediately after construction:
   // - _amount
@@ -586,80 +617,38 @@ TreeNode.prototype = {
   }
 };
 
 TreeNode.compare = function(a, b) {
   return b._amount - a._amount;
 };
 
 /**
- * From a table of Reports, builds a tree that mirrors the tree structure that
- * will be shown as output.
+ * Fill in the remaining properties for the specified tree in a bottom-up
+ * fashion.
  *
- * @param aReports
- *        The table of Reports, indexed by _unsafePath.
+ * @param aTreeOfTrees
+ *        The tree-of-trees.
  * @param aTreePrefix
  *        The prefix (name) of the tree being built.  Must have '/' on the end.
  * @return The built tree.
  */
-function buildTree(aReports, aTreePrefix)
+function fillInTree(aTreeOfTrees, aTreePrefix)
 {
   assert(aTreePrefix.indexOf('/') == aTreePrefix.length - 1,
          "aTreePrefix doesn't end in '/'");
 
-  // We want to process all reports that begin with |aTreePrefix|.  First we
-  // build the tree but only fill the properties that we can with a top-down
-  // traversal.
-
-  let foundReport = false;
-  let t = new TreeNode("falseRoot");
-  for (let unsafePath in aReports) {
-    // Add any missing nodes in the tree implied by the unsafePath.
-    if (unsafePath.startsWith(aTreePrefix)) {
-      foundReport = true;
-      let r = aReports[unsafePath];
-      let unsafeNames = r._unsafePath.split('/');
-      let u = t;
-      for (let i = 0; i < unsafeNames.length; i++) {
-        let unsafeName = unsafeNames[i];
-        let uMatch = u.findKid(unsafeName);
-        if (uMatch) {
-          u = uMatch;
-        } else {
-          let v = new TreeNode(unsafeName);
-          if (!u._kids) {
-            u._kids = [];
-          }
-          u._kids.push(v);
-          u = v;
-        }
-      }
-      // Fill in extra details in the leaf node from the Report object.
-      u._amount = r._amount;
-      u._description = r._description;
-      u._kind = r._kind;
-      if (r._nMerged) {
-        u._nMerged = r._nMerged;
-      }
-      r._done = true;
-    }
-  }
-
-  // There should always be at least one matching Report object when
-  // |aTreePrefix| is "explicit/".  But there may be zero for smaps trees;  if
-  // that happens, bail.
-  if (!foundReport) {
-    assert(aTreePrefix !== 'explicit/', "aTreePrefix !== 'explicit/'");
+  // There should always be an "explicit/" tree.  But smaps trees might not be
+  // present;  if that happens, return early.
+  let t = aTreeOfTrees.findKid(aTreePrefix.replace(/\//g, ''));
+  if (!t) {
+    assert(aTreePrefix !== 'explicit/', "missing explicit tree");
     return null;
   }
 
-  // Using falseRoot makes the above code simpler.  Now discard it, leaving
-  // aTreePrefix at the root.
-  t = t._kids[0];
-
   // Next, fill in the remaining properties bottom-up.
   function fillInNonLeafNodes(aT, aCannotMerge)
   {
     if (!aT._kids) {
       // Leaf node.  Has already been filled in.
       assert(aT._kind !== undefined, "aT._kind is undefined for leaf node");
 
     } else if (aT._kids.length === 1 && !aCannotMerge) {
@@ -703,42 +692,25 @@ function buildTree(aReports, aTreePrefix
 
   // Set the (unsafe) description on the root node.
   t._description = kTreeDescriptions[t._unsafeName];
 
   return t;
 }
 
 /**
- * Ignore all the memory reports that belong to a smaps tree;  this involves
- * explicitly marking them as done.
- *
- * @param aReports
- *        The table of Reports, indexed by _unsafePath.
- */
-function ignoreSmapsTrees(aReports)
-{
-  for (let unsafePath in aReports) {
-    let r = aReports[unsafePath];
-    if (isSmapsPath(r._unsafePath)) {
-      r._done = true;
-    }
-  }
-}
-
-/**
  * Do some work which only makes sense for the 'explicit' tree.
  *
  * @param aT
  *        The tree.
- * @param aReports
- *        Table of Reports for this process, indexed by _unsafePath.
+ * @param aOthers
+ *        "Other measurements" for this process, indexed by _unsafePath.
  * @return A boolean indicating if "heap-allocated" is known for the process.
  */
-function fixUpExplicitTree(aT, aReports)
+function fixUpExplicitTree(aT, aOthers)
 {
   // Determine how many bytes are in heap reports.
   function getKnownHeapUsedBytes(aT)
   {
     let n = 0;
     if (!aT._kids) {
       // Leaf node.
       assert(aT._kind !== undefined, "aT._kind is undefined for leaf node");
@@ -749,30 +721,28 @@ function fixUpExplicitTree(aT, aReports)
       }
     }
     return n;
   }
 
   // A special case:  compute the derived "heap-unclassified" value.  Don't
   // mark "heap-allocated" when we get its size because we want it to appear
   // in the "Other Measurements" list.
-  let heapAllocatedReport = aReports["heap-allocated"];
+  let heapAllocatedReport = aOthers["heap-allocated"];
   if (heapAllocatedReport === undefined)
     return false;
 
   let heapAllocatedBytes = heapAllocatedReport._amount;
   let heapUnclassifiedT = new TreeNode("heap-unclassified");
   heapUnclassifiedT._amount = heapAllocatedBytes - getKnownHeapUsedBytes(aT);
-  // This kindToString() ensures the "(Heap)" prefix is set without having to
-  // set the _kind property, which would mean that there is a corresponding
-  // Report object for this TreeNode object (which isn't true)
-  heapUnclassifiedT._description = kindToString(KIND_HEAP) +
+  heapUnclassifiedT._description =
       "Memory not classified by a more specific reporter. This includes " +
       "slop bytes due to internal fragmentation in the heap allocator " +
       "(caused when the allocator rounds up request sizes).";
+  heapUnclassifiedT._kind = KIND_HEAP;
   aT._kids.push(heapUnclassifiedT);
   aT._amount += heapUnclassifiedT._amount;
   return true;
 }
 
 /**
  * Sort all kid nodes from largest to smallest, and insert aggregate nodes
  * where appropriate.
@@ -896,64 +866,61 @@ function appendWarningElements(aP, aHasK
     appendElementWithText(div, "p", "",
       "This indicates a defect in one or more memory reporters.  The " +
       "invalid values are highlighted.\n\n");
     gUnsafePathsWithInvalidValuesForThisProcess = [];  // reset for the next process
   }
 }
 
 /**
- * Appends the elements for a single process's Reports.
+ * Appends the about:memory elements for a single process.
  *
  * @param aP
  *        The parent DOM node.
  * @param aProcess
  *        The name of the process.
- * @param aReports
- *        Table of Reports for this process, indexed by _unsafePath.
+ * @param aTreeOfTrees
+ *        The tree-of-trees for this process.
+ * @param aOthers
+ *        The "other measurements" for this process.
  * @param aHasMozMallocUsableSize
  *        Boolean indicating if moz_malloc_usable_size works.
  * @return The generated text.
  */
-function appendProcessReportsElements(aP, aProcess, aReports,
-                                      aHasMozMallocUsableSize)
+function appendProcessAboutMemoryElements(aP, aProcess, aTreeOfTrees, aOthers,
+                                          aHasMozMallocUsableSize)
 {
   appendElementWithText(aP, "h1", "", aProcess + " Process\n\n");
 
   // We'll fill this in later.
   let warningsDiv = appendElement(aP, "div", "accuracyWarning");
 
-  let explicitTree = buildTree(aReports, 'explicit/');
-  let hasKnownHeapAllocated = fixUpExplicitTree(explicitTree, aReports);
+  let explicitTree = fillInTree(aTreeOfTrees, "explicit/");
+  let hasKnownHeapAllocated = fixUpExplicitTree(explicitTree, aOthers);
   sortTreeAndInsertAggregateNodes(explicitTree._amount, explicitTree);
   appendTreeElements(aP, explicitTree, aProcess);
 
   // We only show these breakdown trees in verbose mode.
   if (gVerbose) {
     kSmapsTreePrefixes.forEach(function(aTreePrefix) {
-      let t = buildTree(aReports, aTreePrefix);
+      let t = fillInTree(aTreeOfTrees, aTreePrefix);
 
-      // |t| will be null if we don't have any reports for the given
+      // |t| will be undefined if we don't have any reports for the given
       // unsafePath.
       if (t) {
         sortTreeAndInsertAggregateNodes(t._amount, t);
         t._hideKids = true;   // smaps trees are always initially collapsed
         appendTreeElements(aP, t, aProcess);
       }
     });
-  } else {
-    // Although we skip the "smaps" multi-reporter in getReportsByProcess(),
-    // we might get some smaps reports from a child process, and they must be
-    // explicitly ignored.
-    ignoreSmapsTrees(aReports);
   }
 
   // We have to call appendOtherElements after we process all the trees,
   // because it looks at all the reports which aren't part of a tree.
-  appendOtherElements(aP, aReports);
+  appendOtherElements(aP, aOthers);
 
   // Add any warnings about inaccuracies due to platform limitations.
   // These must be computed after generating all the text.  The newlines give
   // nice spacing if we cut+paste into a text buffer.
   appendWarningElements(warningsDiv, hasKnownHeapAllocated,
                         aHasMozMallocUsableSize);
 }
 
@@ -1212,17 +1179,16 @@ function expandPathToThisElement(aElemen
  * Appends the elements for the tree, including its heading.
  *
  * @param aPOuter
  *        The parent DOM node.
  * @param aT
  *        The tree.
  * @param aProcess
  *        The process the tree corresponds to.
- * @return The generated text.
  */
 function appendTreeElements(aPOuter, aT, aProcess)
 {
   let treeBytes = aT._amount;
   let rootStringLength = aT.toString().length;
   let isExplicitTree = aT._unsafeName == 'explicit';
 
   /**
@@ -1239,17 +1205,16 @@ function appendTreeElements(aPOuter, aT,
    *        function.
    * @param aIndentGuide
    *        Records what indentation is required for this tree.  It has one
    *        entry per level of indentation.  For each entry, ._isLastKid
    *        records whether the node in question is the last child, and
    *        ._depth records how many chars of indentation are required.
    * @param aParentStringLength
    *        The length of the formatted byte count of the top node in the tree.
-   * @return The generated text.
    */
   function appendTreeElements2(aP, aUnsafePrePath, aT, aIndentGuide,
                                aBaseIndentText, aParentStringLength)
   {
     function repeatStr(aA, aC, aN)
     {
       for (let i = 0; i < aN; i++) {
         aA.push(aC);
@@ -1410,43 +1375,34 @@ OtherReport.compare = function(a, b) {
          0;
 };
 
 /**
  * Appends the elements for the "Other Measurements" section.
  *
  * @param aP
  *        The parent DOM node.
- * @param aReportsByProcess
- *        Table of Reports for this process, indexed by _unsafePath.
- * @param aProcess
- *        The process these Reports correspond to.
- * @return The generated text.
+ * @param aOthers
+ *        The "other measurements" for this process.
  */
-function appendOtherElements(aP, aReportsByProcess)
+function appendOtherElements(aP, aOthers)
 {
   appendSectionHeader(aP, kSectionNames['other']);
 
   let pre = appendElement(aP, "pre", "entries");
 
-  // Generate an array of Report-like elements, stripping out all the
-  // Reports that have already been handled.  Also find the width of the
+  // Convert the table of OtherReports to an array.  Also find the width of the
   // widest element, so we can format things nicely.
   let maxStringLength = 0;
   let otherReports = [];
-  for (let unsafePath in aReportsByProcess) {
-    let r = aReportsByProcess[unsafePath];
-    if (!r._done) {
-      assert(r._nMerged === undefined, "dup'd OTHER report");
-      let o = new OtherReport(r._unsafePath, r._units, r._amount,
-                              r._description);
-      otherReports.push(o);
-      if (o._asString.length > maxStringLength) {
-        maxStringLength = o._asString.length;
-      }
+  for (let unsafePath in aOthers) {
+    let o = aOthers[unsafePath];
+    otherReports.push(o);
+    if (o._asString.length > maxStringLength) {
+      maxStringLength = o._asString.length;
     }
   }
   otherReports.sort(OtherReport.compare);
 
   // Generate text for the not-yet-printed values.
   let text = "";
   for (let i = 0; i < otherReports.length; i++) {
     let o = otherReports[i];
@@ -1703,18 +1659,16 @@ function appendProcessAboutCompartmentsE
  * @param aP
  *        The parent DOM node.
  * @param aProcess
  *        The name of the process.
  * @param aCompartments
  *        Table of Compartments for this process, indexed by _unsafeName.
  * @param aGhostWindows
  *        Array of window URLs of ghost windows.
- *
- * @return The generated text.
  */
 function appendProcessAboutCompartmentsElements(aP, aProcess, aCompartments, aGhostWindows)
 {
   appendElementWithText(aP, "h1", "", aProcess + " Process\n\n");
 
   let userCompartments = {};
   let systemCompartments = {};
   for (let name in aCompartments) {
--- a/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
+++ b/toolkit/components/aboutmemory/tests/test_aboutmemory.xul
@@ -183,16 +183,17 @@
     // process won't cause these to be skipped;  the fall-back skipping will
     // be hit instead.
     f("2nd", "vsize/e",         NONHEAP, 24*4*KB),
     f("2nd", "vsize/f",         NONHEAP, 24*4*KB),
 
     // Check that we can handle "heap-allocated" not being present.
     f("3rd", "explicit/a/b",    HEAP,    333 * MB),
     f("3rd", "explicit/a/c",    HEAP,    444 * MB),
+    f2("3rd", "other1",         OTHER,   BYTES, 1 * MB),
 
     // Invalid values (negative, too-big) should be identified.
     f("4th", "heap-allocated",   OTHER,   100 * MB),
     f("4th", "explicit/js/compartment(http:\\\\too-big.com\\)/stuff",
                                  HEAP,    150 * MB),
     f("4th", "explicit/ok",      HEAP,      5 * MB),
     f("4th", "explicit/neg1",    NONHEAP,  -2 * MB),
     // -111 becomes "-0.00MB" in non-verbose mode, and getting the negative
@@ -285,16 +286,17 @@ WARNING: the 'heap-allocated' memory rep
 \n\
 Explicit Allocations\n\
 777.00 MB (100.0%) -- explicit\n\
 └──777.00 MB (100.0%) -- a\n\
    ├──444.00 MB (57.14%) ── c\n\
    └──333.00 MB (42.86%) ── b\n\
 \n\
 Other Measurements\n\
+1.00 MB ── other1\n\
 \n\
 4th Process\n\
 \n\
 WARNING: the following values are negative or unreasonably large.\n\
  explicit/js/compartment(http://too-big.com/)/stuff\n\
  explicit/(2 tiny)\n\
  explicit/(2 tiny)/neg1\n\
  explicit/(2 tiny)/heap-unclassified\n\
@@ -427,16 +429,17 @@ WARNING: the 'heap-allocated' memory rep
 \n\
 Explicit Allocations\n\
 814,743,552 B (100.0%) -- explicit\n\
 └──814,743,552 B (100.0%) -- a\n\
    ├──465,567,744 B (57.14%) ── c\n\
    └──349,175,808 B (42.86%) ── b\n\
 \n\
 Other Measurements\n\
+1,048,576 B ── other1\n\
 \n\
 4th Process\n\
 \n\
 WARNING: the following values are negative or unreasonably large.\n\
  explicit/js/compartment(http://too-big.com/)/stuff\n\
  explicit/neg1\n\
  explicit/heap-unclassified\n\
  other1\n\