Bug 1486623 - Skip less nodes when reporting memory, and report ShadowRoot's StyleSheets. r=bzbarsky
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 29 Aug 2018 22:19:42 +0000
changeset 433977 2f49db02e5f6b4528bb5fe04c9fb9e80581b9adb
parent 433976 4e195f7b2d6b8c12aba04ef69401e9af65daa05a
child 433978 ac2c1fea89ea57dee4a5b85dd046ca7ef71a9baf
push id34529
push useraiakab@mozilla.com
push dateThu, 30 Aug 2018 04:36:03 +0000
treeherdermozilla-central@95b4b8e25577 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1486623
milestone63.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 1486623 - Skip less nodes when reporting memory, and report ShadowRoot's StyleSheets. r=bzbarsky This moves all the node-specific reporting to nsIDocument. OrphanReporter delegates all the reporting to that and then returns the sum of all sizes, which is not ideal but was pre-existing. Also, I moved the main mStyleSheets size reporting to DocumentOrShadowRoot for it to be shared between document and ShadowRoot. I'll add memory reporting for the computed stylesheet maps and such in the ShadowRoot in a followup. I went through all the XBL bindings, though it seems I could just use GetBindingWithContent(), since according to: https://searchfox.org/mozilla-central/rev/55da592d85c2baf8d8818010c41d9738c97013d2/dom/xbl/nsXBLBinding.cpp#615 We don't allow multiple bindings to contribute anon content. Anyway it was the same amount of code... Differential Revision: https://phabricator.services.mozilla.com/D4433
dom/base/DocumentOrShadowRoot.cpp
dom/base/DocumentOrShadowRoot.h
dom/base/ShadowRoot.cpp
dom/base/ShadowRoot.h
dom/base/nsDocument.cpp
dom/base/nsIDocument.h
js/xpconnect/src/XPCJSRuntime.cpp
--- a/dom/base/DocumentOrShadowRoot.cpp
+++ b/dom/base/DocumentOrShadowRoot.cpp
@@ -7,30 +7,55 @@
 #include "DocumentOrShadowRoot.h"
 #include "mozilla/EventStateManager.h"
 #include "mozilla/dom/ShadowRoot.h"
 #include "mozilla/dom/StyleSheetList.h"
 #include "nsDocument.h"
 #include "nsFocusManager.h"
 #include "nsLayoutUtils.h"
 #include "nsSVGUtils.h"
+#include "nsWindowSizes.h"
 
 namespace mozilla {
 namespace dom {
 
 DocumentOrShadowRoot::DocumentOrShadowRoot(mozilla::dom::ShadowRoot& aShadowRoot)
   : mAsNode(aShadowRoot)
   , mKind(Kind::ShadowRoot)
 {}
 
 DocumentOrShadowRoot::DocumentOrShadowRoot(nsIDocument& aDoc)
   : mAsNode(aDoc)
   , mKind(Kind::Document)
 {}
 
+void
+DocumentOrShadowRoot::AddSizeOfOwnedSheetArrayExcludingThis(
+  nsWindowSizes& aSizes,
+  const nsTArray<RefPtr<StyleSheet>>& aSheets)
+{
+  size_t n = 0;
+  n += aSheets.ShallowSizeOfExcludingThis(aSizes.mState.mMallocSizeOf);
+  for (StyleSheet* sheet : aSheets) {
+    if (!sheet->GetAssociatedDocumentOrShadowRoot()) {
+      // Avoid over-reporting shared sheets.
+      continue;
+    }
+    n += sheet->SizeOfIncludingThis(aSizes.mState.mMallocSizeOf);
+  }
+
+  aSizes.mLayoutStyleSheetsSize += n;
+}
+
+void
+DocumentOrShadowRoot::AddSizeOfExcludingThis(nsWindowSizes& aSizes) const
+{
+  AddSizeOfOwnedSheetArrayExcludingThis(aSizes, mStyleSheets);
+}
+
 DocumentOrShadowRoot::~DocumentOrShadowRoot()
 {
   for (StyleSheet* sheet : mStyleSheets) {
     sheet->ClearAssociatedDocumentOrShadowRoot();
   }
 }
 
 StyleSheetList&
--- a/dom/base/DocumentOrShadowRoot.h
+++ b/dom/base/DocumentOrShadowRoot.h
@@ -10,16 +10,17 @@
 #include "mozilla/dom/NameSpaceConstants.h"
 #include "nsContentListDeclarations.h"
 #include "nsTArray.h"
 #include "nsIdentifierMapEntry.h"
 
 class nsContentList;
 class nsIDocument;
 class nsINode;
+class nsWindowSizes;
 
 namespace mozilla {
 class StyleSheet;
 
 namespace dom {
 
 class Element;
 class StyleSheetList;
@@ -39,16 +40,21 @@ class DocumentOrShadowRoot
     Document,
     ShadowRoot,
   };
 
 public:
   explicit DocumentOrShadowRoot(nsIDocument&);
   explicit DocumentOrShadowRoot(mozilla::dom::ShadowRoot&);
 
+  void AddSizeOfExcludingThis(nsWindowSizes&) const;
+  static void AddSizeOfOwnedSheetArrayExcludingThis(
+      nsWindowSizes&,
+      const nsTArray<RefPtr<StyleSheet>>&);
+
   nsINode& AsNode()
   {
     return mAsNode;
   }
 
   const nsINode& AsNode() const
   {
     return mAsNode;
--- a/dom/base/ShadowRoot.cpp
+++ b/dom/base/ShadowRoot.cpp
@@ -98,16 +98,24 @@ ShadowRoot::~ShadowRoot()
   MOZ_DIAGNOSTIC_ASSERT(!OwnerDoc()->IsComposedDocShadowRoot(*this));
 
   UnsetFlags(NODE_IS_IN_SHADOW_TREE);
 
   // nsINode destructor expects mSubtreeRoot == this.
   SetSubtreeRootPointer(this);
 }
 
+void
+ShadowRoot::AddSizeOfExcludingThis(nsWindowSizes& aSizes, size_t* aNodeSize) const
+{
+  DocumentFragment::AddSizeOfExcludingThis(aSizes, aNodeSize);
+  DocumentOrShadowRoot::AddSizeOfExcludingThis(aSizes);
+  // FIXME(emilio, bug 1486728): We should probably report mServoStyles here.
+}
+
 JSObject*
 ShadowRoot::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 {
   return mozilla::dom::ShadowRoot_Binding::Wrap(aCx, this, aGivenProto);
 }
 
 void
 ShadowRoot::CloneInternalDataFrom(ShadowRoot* aOther)
--- a/dom/base/ShadowRoot.h
+++ b/dom/base/ShadowRoot.h
@@ -50,16 +50,18 @@ public:
   NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED
 
   ShadowRoot(Element* aElement, ShadowRootMode aMode,
              already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo);
 
+  void AddSizeOfExcludingThis(nsWindowSizes&, size_t* aNodeSize) const final;
+
   // Shadow DOM v1
   Element* Host() const
   {
     MOZ_ASSERT(GetHost(), "ShadowRoot always has a host, how did we create "
                           "this ShadowRoot?");
     return GetHost();
   }
 
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -11759,18 +11759,16 @@ nsIDocument::MaybeActiveMediaComponents(
   }
 
   GetWindow()->MaybeActiveMediaComponents();
 }
 
 /* virtual */ void
 nsIDocument::DocAddSizeOfExcludingThis(nsWindowSizes& aSizes) const
 {
-  nsINode::AddSizeOfExcludingThis(aSizes, &aSizes.mDOMOtherSize);
-
   if (mPresShell) {
     mPresShell->AddSizeOfIncludingThis(aSizes);
   }
 
   aSizes.mPropertyTablesSize +=
     mPropertyTable.SizeOfExcludingThis(aSizes.mState.mMallocSizeOf);
 
   if (EventListenerManager* elm = GetExistingListenerManager()) {
@@ -11796,52 +11794,37 @@ nsIDocument::DocAddSizeOfExcludingThis(n
 
 void
 nsIDocument::DocAddSizeOfIncludingThis(nsWindowSizes& aWindowSizes) const
 {
   aWindowSizes.mDOMOtherSize += aWindowSizes.mState.mMallocSizeOf(this);
   DocAddSizeOfExcludingThis(aWindowSizes);
 }
 
-static size_t
-SizeOfOwnedSheetArrayExcludingThis(const nsTArray<RefPtr<StyleSheet>>& aSheets,
-                                   MallocSizeOf aMallocSizeOf)
-{
-  size_t n = 0;
-  n += aSheets.ShallowSizeOfExcludingThis(aMallocSizeOf);
-  for (StyleSheet* sheet : aSheets) {
-    if (!sheet->GetAssociatedDocumentOrShadowRoot()) {
-      // Avoid over-reporting shared sheets.
-      continue;
-    }
-    n += sheet->SizeOfIncludingThis(aMallocSizeOf);
-  }
-  return n;
-}
 
 void
 nsDocument::AddSizeOfExcludingThis(nsWindowSizes& aSizes,
                                    size_t* aNodeSize) const
 {
   // This AddSizeOfExcludingThis() overrides the one from nsINode.  But
   // nsDocuments can only appear at the top of the DOM tree, and we use the
   // specialized DocAddSizeOfExcludingThis() in that case.  So this should never
   // be called.
   MOZ_CRASH();
 }
 
-static void
-AddSizeOfNodeTree(nsIContent* aNode, nsWindowSizes& aWindowSizes)
+/* static */ void
+nsIDocument::AddSizeOfNodeTree(nsINode& aNode, nsWindowSizes& aWindowSizes)
 {
   size_t nodeSize = 0;
-  aNode->AddSizeOfIncludingThis(aWindowSizes, &nodeSize);
+  aNode.AddSizeOfIncludingThis(aWindowSizes, &nodeSize);
 
   // This is where we transfer the nodeSize obtained from
   // nsINode::AddSizeOfIncludingThis() to a value in nsWindowSizes.
-  switch (aNode->NodeType()) {
+  switch (aNode.NodeType()) {
   case nsINode::ELEMENT_NODE:
     aWindowSizes.mDOMElementNodesSize += nodeSize;
     break;
   case nsINode::TEXT_NODE:
     aWindowSizes.mDOMTextNodesSize += nodeSize;
     break;
   case nsINode::CDATA_SECTION_NODE:
     aWindowSizes.mDOMCDATANodesSize += nodeSize;
@@ -11849,58 +11832,74 @@ AddSizeOfNodeTree(nsIContent* aNode, nsW
   case nsINode::COMMENT_NODE:
     aWindowSizes.mDOMCommentNodesSize += nodeSize;
     break;
   default:
     aWindowSizes.mDOMOtherSize += nodeSize;
     break;
   }
 
-  if (EventListenerManager* elm = aNode->GetExistingListenerManager()) {
+  if (EventListenerManager* elm = aNode.GetExistingListenerManager()) {
     aWindowSizes.mDOMEventListenersCount += elm->ListenerCount();
   }
 
-  AllChildrenIterator iter(aNode, nsIContent::eAllChildren);
-  for (nsIContent* n = iter.GetNextChild(); n; n = iter.GetNextChild()) {
-    AddSizeOfNodeTree(n, aWindowSizes);
+  if (aNode.IsContent()) {
+    nsTArray<nsIContent*> anonKids;
+    nsContentUtils::AppendNativeAnonymousChildren(aNode.AsContent(),
+                                                  anonKids,
+                                                  nsIContent::eAllChildren);
+    for (nsIContent* anonKid : anonKids) {
+      AddSizeOfNodeTree(*anonKid, aWindowSizes);
+    }
+
+    if (auto* element = Element::FromNode(aNode)) {
+      if (ShadowRoot* shadow = element->GetShadowRoot()) {
+        AddSizeOfNodeTree(*shadow, aWindowSizes);
+      }
+
+      for (nsXBLBinding* binding = element->GetXBLBinding();
+           binding;
+           binding = binding->GetBaseBinding()) {
+        if (nsIContent* anonContent = binding->GetAnonymousContent()) {
+          AddSizeOfNodeTree(*anonContent, aWindowSizes);
+        }
+      }
+    }
+  }
+
+  // NOTE(emilio): If you feel smart and want to change this function to use
+  // GetNextNode(), think twice, since you'd need to handle <xbl:content> in a
+  // sane way, and kids of <content> won't point to the parent, so we'd never
+  // find the root node where we should stop at.
+  for (nsIContent* kid = aNode.GetFirstChild(); kid; kid = kid->GetNextSibling()) {
+    AddSizeOfNodeTree(*kid, aWindowSizes);
   }
 }
 
 void
 nsDocument::DocAddSizeOfExcludingThis(nsWindowSizes& aWindowSizes) const
 {
-  // We use AllChildrenIterator to iterate over DOM nodes in
-  // AddSizeOfNodeTree(). The obvious place to start is at the document's root
-  // element, using GetRootElement(). However, that will miss comment nodes
-  // that are siblings of the root element. Instead we use
-  // GetFirstChild()/GetNextSibling() to traverse the document's immediate
-  // child nodes, calling AddSizeOfNodeTree() on each to measure them and then
-  // all their descendants. (The comment nodes won't have any descendants).
-  for (nsIContent* node = nsINode::GetFirstChild();
-       node;
-       node = node->GetNextSibling()) {
-    AddSizeOfNodeTree(node, aWindowSizes);
+  nsINode::AddSizeOfExcludingThis(aWindowSizes, &aWindowSizes.mDOMOtherSize);
+
+  for (nsIContent* kid = GetFirstChild(); kid; kid = kid->GetNextSibling()) {
+    AddSizeOfNodeTree(*kid, aWindowSizes);
   }
 
   // IMPORTANT: for our ComputedValues measurements, we want to measure
   // ComputedValues accessible from DOM elements before ComputedValues not
   // accessible from DOM elements (i.e. accessible only from the frame tree).
   //
   // Therefore, the measurement of the nsIDocument superclass must happen after
   // the measurement of DOM nodes (above), because nsIDocument contains the
   // PresShell, which contains the frame tree.
   nsIDocument::DocAddSizeOfExcludingThis(aWindowSizes);
 
-  aWindowSizes.mLayoutStyleSheetsSize +=
-    SizeOfOwnedSheetArrayExcludingThis(mStyleSheets,
-                                       aWindowSizes.mState.mMallocSizeOf);
+  DocumentOrShadowRoot::AddSizeOfExcludingThis(aWindowSizes);
   for (auto& sheetArray : mAdditionalSheets) {
-    aWindowSizes.mLayoutStyleSheetsSize +=
-      SizeOfOwnedSheetArrayExcludingThis(sheetArray,
-                                         aWindowSizes.mState.mMallocSizeOf);
+    AddSizeOfOwnedSheetArrayExcludingThis(aWindowSizes, sheetArray);
   }
   // Lumping in the loader with the style-sheets size is not ideal,
   // but most of the things in there are in fact stylesheets, so it
   // doesn't seem worthwhile to separate it out.
   aWindowSizes.mLayoutStyleSheetsSize +=
     CSSLoader()->SizeOfIncludingThis(aWindowSizes.mState.mMallocSizeOf);
 
   aWindowSizes.mDOMOtherSize += mAttrStyleSheet
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -3012,16 +3012,20 @@ public:
                      const char16_t **aParams = nullptr,
                      uint32_t aParamsLength = 0) const;
 
   // Posts an event to call UpdateVisibilityState
   void PostVisibilityUpdateEvent();
 
   bool IsSyntheticDocument() const { return mIsSyntheticDocument; }
 
+  // Adds the size of a given node, which must not be a document node, to the
+  // window sizes passed-in.
+  static void AddSizeOfNodeTree(nsINode&, nsWindowSizes&);
+
   // Note: nsIDocument is a sub-class of nsINode, which has a
   // SizeOfExcludingThis function.  However, because nsIDocument objects can
   // only appear at the top of the DOM tree, we have a specialized measurement
   // function which returns multiple sizes.
   virtual void DocAddSizeOfExcludingThis(nsWindowSizes& aWindowSizes) const;
   // DocAddSizeOfIncludingThis doesn't need to be overridden by sub-classes
   // because nsIDocument inherits from nsINode;  see the comment above the
   // declaration of nsINode::SizeOfIncludingThis.
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -20,16 +20,17 @@
 #include "nsNetUtil.h"
 
 #include "nsExceptionHandler.h"
 #include "nsIMemoryInfoDumper.h"
 #include "nsIMemoryReporter.h"
 #include "nsIObserverService.h"
 #include "nsIDebug2.h"
 #include "nsIDocShell.h"
+#include "nsIDocument.h"
 #include "nsIRunnable.h"
 #include "nsPIDOMWindow.h"
 #include "nsPrintfCString.h"
 #include "nsWindowSizes.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/Services.h"
 #include "mozilla/dom/ScriptLoader.h"
@@ -2136,49 +2137,43 @@ class OrphanReporter : public JS::Object
   public:
     explicit OrphanReporter(GetISupportsFun aGetISupports)
       : JS::ObjectPrivateVisitor(aGetISupports)
       , mState(OrphanMallocSizeOf)
     {}
 
     virtual size_t sizeOfIncludingThis(nsISupports* aSupports) override
     {
-        size_t n = 0;
         nsCOMPtr<nsINode> node = do_QueryInterface(aSupports);
-        // https://bugzilla.mozilla.org/show_bug.cgi?id=773533#c11 explains
-        // that we have to skip XBL elements because they violate certain
-        // assumptions.  Yuk.
-        if (node && !node->IsInComposedDoc() &&
-            !(node->IsElement() && node->AsElement()->IsInNamespace(kNameSpaceID_XBL)))
-        {
-            // This is an orphan node.  If we haven't already handled the
-            // sub-tree that this node belongs to, measure the sub-tree's size
-            // and then record its root so we don't measure it again.
-            nsCOMPtr<nsINode> orphanTree = node->SubtreeRoot();
-            if (orphanTree && !mState.HaveSeenPtr(orphanTree.get())) {
-                n += SizeOfTreeIncludingThis(orphanTree);
-            }
+        // https://bugzilla.mozilla.org/show_bug.cgi?id=773533#c11 explains that we have to skip
+        // XBL elements because they violate certain assumptions.  Yuk.
+        if (!node || node->IsInComposedDoc() ||
+            (node->IsElement() && node->AsElement()->IsInNamespace(kNameSpaceID_XBL))) {
+            return 0;
         }
-        return n;
-    }
-
-    size_t SizeOfTreeIncludingThis(nsINode* tree)
-    {
-        size_t nodeSize = 0;
+
+        // This is an orphan node.  If we haven't already handled the sub-tree that this node
+        // belongs to, measure the sub-tree's size and then record its root so we don't measure it
+        // again.
+        nsCOMPtr<nsINode> orphanTree = node->SubtreeRoot();
+        if (!orphanTree || mState.HaveSeenPtr(orphanTree.get())) {
+            return 0;
+        }
+
         nsWindowSizes sizes(mState);
-        tree->AddSizeOfIncludingThis(sizes, &nodeSize);
-        for (nsIContent* child = tree->GetFirstChild(); child; child = child->GetNextNode(tree))
-            child->AddSizeOfIncludingThis(sizes, &nodeSize);
-
-        // We combine the node size with nsStyleSizes here. It's not ideal, but
-        // it's hard to get the style structs measurements out to
-        // nsWindowMemoryReporter. Also, we drop mServoData in
-        // UnbindFromTree(), so in theory any non-in-tree element won't have
-        // any style data to measure.
-        return nodeSize + sizes.getTotalSize();
+        nsIDocument::AddSizeOfNodeTree(*orphanTree, sizes);
+
+        // We combine the node size with nsStyleSizes here. It's not ideal, but it's hard to get
+        // the style structs measurements out to nsWindowMemoryReporter. Also, we drop mServoData
+        // in UnbindFromTree(), so in theory any non-in-tree element won't have any style data to
+        // measure.
+        //
+        // FIXME(emilio): We should ideally not do this, since ShadowRoots keep their StyleSheets
+        // alive even when detached from a document, and those could be significant in theory.
+        return sizes.getTotalSize();
     }
 
   private:
     SizeOfState mState;
 };
 
 #ifdef DEBUG
 static bool