Bug 1352898, add a cache for nsMappedAttributes to reduce malloc/free and bind nsMappedAttributes always to nsHTMLStyleSheet if owner document has such, r=bz
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Fri, 07 Apr 2017 19:21:48 +0300
changeset 351824 828bcefe18f26590c203ac89200ece21cd2e7bdc
parent 351823 801bf613b8de5c1393543509364e8e5eb2376edf
child 351825 96596bdeb7ecaa736e692b253ff7787fb35e69f3
push id88958
push useropettay@mozilla.com
push dateFri, 07 Apr 2017 16:42:45 +0000
treeherdermozilla-inbound@828bcefe18f2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1352898
milestone55.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 1352898, add a cache for nsMappedAttributes to reduce malloc/free and bind nsMappedAttributes always to nsHTMLStyleSheet if owner document has such, r=bz
dom/base/Element.cpp
dom/base/Element.h
dom/base/nsContentUtils.cpp
dom/base/nsMappedAttributeElement.cpp
dom/base/nsMappedAttributeElement.h
dom/base/nsMappedAttributes.cpp
dom/base/nsMappedAttributes.h
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1671,27 +1671,16 @@ Element::BindToTree(nsIDocument* aDocume
 
   if (MayHaveStyle() && !IsXULElement()) {
     // XXXbz if we already have a style attr parsed, this won't do
     // anything... need to fix that.
     // If MayHaveStyle() is true, we must be an nsStyledElement
     static_cast<nsStyledElement*>(this)->ReparseStyleAttribute(false, false);
   }
 
-  if (aDocument) {
-    // If we're in a document now, let our mapped attrs know what their new
-    // sheet is.  This is safe to run for non-mapped-attribute elements too;
-    // it'll just do a small bit of unnecessary work.  But most elements in
-    // practice are mapped-attribute elements.
-    nsHTMLStyleSheet* sheet = aDocument->GetAttributeStyleSheet();
-    if (sheet) {
-      mAttrsAndChildren.SetMappedAttrStyleSheet(sheet);
-    }
-  }
-
   // Call BindToTree on shadow root children.
   ShadowRoot* shadowRoot = GetShadowRoot();
   if (shadowRoot) {
     shadowRoot->SetIsComposedDocParticipant(IsInComposedDoc());
     for (nsIContent* child = shadowRoot->GetFirstChild(); child;
          child = child->GetNextSibling()) {
       rv = child->BindToTree(nullptr, shadowRoot,
                              shadowRoot->GetBindingParent(),
@@ -2459,17 +2448,17 @@ Element::SetAttrAndNotify(int32_t aNames
     if (aName == nsGkAtoms::dir) {
       hadValidDir = HasValidDir() || IsHTMLElement(nsGkAtoms::bdi);
       hadDirAuto = HasDirAuto(); // already takes bdi into account
     }
 
     // XXXbz Perhaps we should push up the attribute mapping function
     // stuff to Element?
     if (!IsAttributeMapped(aName) ||
-        !SetMappedAttribute(aComposedDocument, aName, aParsedValue, &rv)) {
+        !SetMappedAttribute(aName, aParsedValue, &rv)) {
       rv = mAttrsAndChildren.SetAndSwapAttr(aName, aParsedValue);
     }
   }
   else {
     RefPtr<mozilla::dom::NodeInfo> ni;
     ni = mNodeInfo->NodeInfoManager()->GetNodeInfo(aName, aPrefix,
                                                    aNamespaceID,
                                                    nsIDOMNode::ATTRIBUTE_NODE);
@@ -2578,18 +2567,17 @@ Element::ParseAttribute(int32_t aNamespa
       return true;
     }
   }
 
   return false;
 }
 
 bool
-Element::SetMappedAttribute(nsIDocument* aDocument,
-                            nsIAtom* aName,
+Element::SetMappedAttribute(nsIAtom* aName,
                             nsAttrValue& aValue,
                             nsresult* aRetval)
 {
   *aRetval = NS_OK;
   return false;
 }
 
 EventListenerManager*
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -1385,26 +1385,24 @@ protected:
 
   /**
    * Try to set the attribute as a mapped attribute, if applicable.  This will
    * only be called for attributes that are in the null namespace and only on
    * attributes that returned true when passed to IsAttributeMapped.  The
    * caller will not try to set the attr in any other way if this method
    * returns true (the value of aRetval does not matter for that purpose).
    *
-   * @param aDocument the current document of this node (an optimization)
    * @param aName the name of the attribute
    * @param aValue the nsAttrValue to set
    * @param [out] aRetval the nsresult status of the operation, if any.
    * @return true if the setting was attempted, false otherwise.
    */
-  virtual bool SetMappedAttribute(nsIDocument* aDocument,
-                                    nsIAtom* aName,
-                                    nsAttrValue& aValue,
-                                    nsresult* aRetval);
+  virtual bool SetMappedAttribute(nsIAtom* aName,
+                                  nsAttrValue& aValue,
+                                  nsresult* aRetval);
 
   /**
    * Hook that is called by Element::SetAttr to allow subclasses to
    * deal with attribute sets.  This will only be called after we verify that
    * we're actually doing an attr set and will be called before
    * AttributeWillChange and before ParseAttribute and hence before we've set
    * the new value.
    *
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -175,16 +175,17 @@
 #include "nsIURIWithPrincipal.h"
 #include "nsIURL.h"
 #include "nsIWebNavigation.h"
 #include "nsIWindowMediator.h"
 #include "nsIWordBreaker.h"
 #include "nsIXPConnect.h"
 #include "nsJSUtils.h"
 #include "nsLWBrkCIID.h"
+#include "nsMappedAttributes.h"
 #include "nsNetCID.h"
 #include "nsNetUtil.h"
 #include "nsNodeInfoManager.h"
 #include "NullPrincipal.h"
 #include "nsParserCIID.h"
 #include "nsParserConstants.h"
 #include "nsPIDOMWindow.h"
 #include "nsPresContext.h"
@@ -1988,16 +1989,17 @@ nsContentUtils::Shutdown()
   delete sAltText;
   sAltText = nullptr;
   delete sModifierSeparator;
   sModifierSeparator = nullptr;
 
   NS_IF_RELEASE(sSameOriginChecker);
 
   HTMLInputElement::Shutdown();
+  nsMappedAttributes::Shutdown();
 }
 
 /**
  * Checks whether two nodes come from the same origin. aTrustedNode is
  * considered 'safe' in that a user can operate on it and that it isn't
  * a js-object that implements nsIDOMNode.
  * Never call this function with the first node provided by script, it
  * must always be known to be a 'real' node!
--- a/dom/base/nsMappedAttributeElement.cpp
+++ b/dom/base/nsMappedAttributeElement.cpp
@@ -10,33 +10,37 @@
 nsresult
 nsMappedAttributeElement::WalkContentStyleRules(nsRuleWalker* aRuleWalker)
 {
   mAttrsAndChildren.WalkMappedAttributeStyleRules(aRuleWalker);
   return NS_OK;
 }
 
 bool
-nsMappedAttributeElement::SetMappedAttribute(nsIDocument* aDocument,
-                                             nsIAtom* aName,
+nsMappedAttributeElement::SetMappedAttribute(nsIAtom* aName,
                                              nsAttrValue& aValue,
                                              nsresult* aRetval)
 {
-  NS_PRECONDITION(aDocument == GetComposedDoc(), "Unexpected document");
-  nsHTMLStyleSheet* sheet = aDocument ?
-    aDocument->GetAttributeStyleSheet() : nullptr;
-
+  nsHTMLStyleSheet* sheet = OwnerDoc()->GetAttributeStyleSheet();
   *aRetval = mAttrsAndChildren.SetAndTakeMappedAttr(aName, aValue,
                                                     this, sheet);
   return true;
 }
 
 nsMapRuleToAttributesFunc
 nsMappedAttributeElement::GetAttributeMappingFunction() const
 {
   return &MapNoAttributesInto;
 }
 
 void
 nsMappedAttributeElement::MapNoAttributesInto(const nsMappedAttributes* aAttributes,
                                               mozilla::GenericSpecifiedValues* aGenericData)
 {
 }
+
+void
+nsMappedAttributeElement::NodeInfoChanged(nsIDocument* aOldDoc)
+{
+  nsHTMLStyleSheet* sheet = OwnerDoc()->GetAttributeStyleSheet();
+  mAttrsAndChildren.SetMappedAttrStyleSheet(sheet);
+  nsMappedAttributeElementBase::NodeInfoChanged(aOldDoc);
+}
--- a/dom/base/nsMappedAttributeElement.h
+++ b/dom/base/nsMappedAttributeElement.h
@@ -35,15 +35,16 @@ protected:
 
 public:
   virtual nsMapRuleToAttributesFunc GetAttributeMappingFunction() const;
 
   static void MapNoAttributesInto(const nsMappedAttributes* aAttributes, 
                                   mozilla::GenericSpecifiedValues* aGenericData);
 
   NS_IMETHOD WalkContentStyleRules(nsRuleWalker* aRuleWalker) override;
-  virtual bool SetMappedAttribute(nsIDocument* aDocument,
-                                    nsIAtom* aName,
-                                    nsAttrValue& aValue,
-                                    nsresult* aRetval) override;
+  virtual bool SetMappedAttribute(nsIAtom* aName,
+                                  nsAttrValue& aValue,
+                                  nsresult* aRetval) override;
+
+  virtual void NodeInfoChanged(nsIDocument* aOldDoc) override;
 };
 
 #endif // NS_MAPPEDATTRIBUTEELEMENT_H_
--- a/dom/base/nsMappedAttributes.cpp
+++ b/dom/base/nsMappedAttributes.cpp
@@ -16,34 +16,56 @@
 #include "mozilla/GenericSpecifiedValues.h"
 #include "mozilla/HashFunctions.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/ServoDeclarationBlock.h"
 #include "mozilla/ServoSpecifiedValues.h"
 
 using namespace mozilla;
 
+bool
+nsMappedAttributes::sShuttingDown = false;
+nsTArray<void*>*
+nsMappedAttributes::sCachedMappedAttributeAllocations = nullptr;
+
+void
+nsMappedAttributes::Shutdown()
+{
+  sShuttingDown = true;
+  if (sCachedMappedAttributeAllocations) {
+    for (uint32_t i = 0; i < sCachedMappedAttributeAllocations->Length(); ++i) {
+      void* cachedValue = (*sCachedMappedAttributeAllocations)[i];
+      ::operator delete(cachedValue);
+    }
+  }
+
+  delete sCachedMappedAttributeAllocations;
+  sCachedMappedAttributeAllocations = nullptr;
+}
+
 nsMappedAttributes::nsMappedAttributes(nsHTMLStyleSheet* aSheet,
                                        nsMapRuleToAttributesFunc aMapRuleFunc)
   : mAttrCount(0),
     mSheet(aSheet),
     mRuleMapper(aMapRuleFunc),
     mServoStyle(nullptr)
 {
+  MOZ_ASSERT(mRefCnt == 0); // Ensure caching works as expected.
 }
 
 nsMappedAttributes::nsMappedAttributes(const nsMappedAttributes& aCopy)
   : mAttrCount(aCopy.mAttrCount),
     mSheet(aCopy.mSheet),
     mRuleMapper(aCopy.mRuleMapper),
     // This is only called by ::Clone, which is used to create independent
     // nsMappedAttributes objects which should not share a ServoDeclarationBlock
     mServoStyle(nullptr)
 {
   NS_ASSERTION(mBufferSize >= aCopy.mAttrCount, "can't fit attributes");
+  MOZ_ASSERT(mRefCnt == 0); // Ensure caching works as expected.
 
   uint32_t i;
   for (i = 0; i < mAttrCount; ++i) {
     new (&Attrs()[i]) InternalAttr(aCopy.Attrs()[i]);
   }
 }
 
 nsMappedAttributes::~nsMappedAttributes()
@@ -76,26 +98,67 @@ void* nsMappedAttributes::operator new(s
   // aSize will include the mAttrs buffer so subtract that.
   // We don't want to under-allocate, however, so do not subtract
   // if we have zero attributes. The zero attribute case only happens
   // for <body>'s mapped attributes
   if (aAttrCount != 0) {
     size -= sizeof(void*[1]);
   }
 
+  if (sCachedMappedAttributeAllocations) {
+    void* cached =
+      sCachedMappedAttributeAllocations->SafeElementAt(aAttrCount);
+    if (cached) {
+      (*sCachedMappedAttributeAllocations)[aAttrCount] = nullptr;
+      return cached;
+    }
+  }
+
   void* newAttrs = ::operator new(size);
 
 #ifdef DEBUG
   static_cast<nsMappedAttributes*>(newAttrs)->mBufferSize = aAttrCount;
 #endif
   return newAttrs;
 }
 
-NS_IMPL_ISUPPORTS(nsMappedAttributes,
-                  nsIStyleRule)
+void
+nsMappedAttributes::LastRelease()
+{
+  if (!sShuttingDown) {
+    if (!sCachedMappedAttributeAllocations) {
+      sCachedMappedAttributeAllocations = new nsTArray<void*>();
+    }
+
+    // Ensure the cache array is at least mAttrCount + 1 long and
+    // that each item is either null or pointing to a cached item.
+    // The size of the array is capped because mapped attributes are defined
+    // statically in element implementations.
+    sCachedMappedAttributeAllocations->SetCapacity(mAttrCount + 1);
+    for (uint32_t i = sCachedMappedAttributeAllocations->Length();
+         i < (uint32_t(mAttrCount) + 1); ++i) {
+      sCachedMappedAttributeAllocations->AppendElement(nullptr);
+    }
+
+    if (!(*sCachedMappedAttributeAllocations)[mAttrCount]) {
+      void* memoryToCache = this;
+      this->~nsMappedAttributes();
+      (*sCachedMappedAttributeAllocations)[mAttrCount] = memoryToCache;
+      return;
+    }
+  }
+
+  delete this;
+}
+
+NS_IMPL_ADDREF(nsMappedAttributes)
+NS_IMPL_RELEASE_WITH_DESTROY(nsMappedAttributes, LastRelease())
+
+NS_IMPL_QUERY_INTERFACE(nsMappedAttributes,
+                        nsIStyleRule)
 
 void
 nsMappedAttributes::SetAndTakeAttr(nsIAtom* aAttrName, nsAttrValue& aValue)
 {
   NS_PRECONDITION(aAttrName, "null name");
   uint32_t i;
   for (i = 0; i < mAttrCount && !Attrs()[i].mName.IsSmaller(aAttrName); ++i) {
     if (Attrs()[i].mName.Equals(aAttrName)) {
--- a/dom/base/nsMappedAttributes.h
+++ b/dom/base/nsMappedAttributes.h
@@ -95,17 +95,21 @@ public:
   virtual bool GetDiscretelyAnimatedCSSValue(nsCSSPropertyID aProperty,
                                              nsCSSValue* aValue) override;
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override;
 #endif
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
+  static void Shutdown();
 private:
+
+  void LastRelease();
+
   nsMappedAttributes(const nsMappedAttributes& aCopy);
   ~nsMappedAttributes();
 
   struct InternalAttr
   {
     nsAttrName mName;
     nsAttrValue mValue;
   };
@@ -129,11 +133,17 @@ private:
   uint16_t mAttrCount;
 #ifdef DEBUG
   uint16_t mBufferSize;
 #endif
   nsHTMLStyleSheet* mSheet; //weak
   nsMapRuleToAttributesFunc mRuleMapper;
   RefPtr<RawServoDeclarationBlock> mServoStyle;
   void* mAttrs[1];
+
+  static bool sShuttingDown;
+
+  // We're caching some memory to avoid trashing the allocator.
+  // The memory stored at index N can hold N attribute values.
+  static nsTArray<void*>* sCachedMappedAttributeAllocations;
 };
 
 #endif /* nsMappedAttributes_h___ */