Bug 1129786 (part 2) - Instantiate nsDOMAttributeMap::mAttributeCache eagerly. r=bz.
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 03 Feb 2015 00:03:28 -0800
changeset 265345 327a93e5cc397700118cfa20a184f17c9aeb3cbc
parent 265344 f75beaf3149fd745e760143ef08053f461210f6e
child 265346 41f424ff87542c6cd42c4fb36605c970d95e1d17
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1129786, 1059056
milestone39.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 1129786 (part 2) - Instantiate nsDOMAttributeMap::mAttributeCache eagerly. r=bz. Now that empty PLDHashTables don't allocate any entry storage, we can undo the lazification of nsDOMAttributeMap::mAttributeCache from bug 1059056.
dom/base/nsDOMAttributeMap.cpp
dom/base/nsDOMAttributeMap.h
--- a/dom/base/nsDOMAttributeMap.cpp
+++ b/dom/base/nsDOMAttributeMap.cpp
@@ -46,27 +46,23 @@ RemoveMapRef(nsAttrHashKey::KeyType aKey
 {
   aData->SetMap(nullptr);
 
   return PL_DHASH_REMOVE;
 }
 
 nsDOMAttributeMap::~nsDOMAttributeMap()
 {
-  if (mAttributeCache) {
-    mAttributeCache->Enumerate(RemoveMapRef, nullptr);
-  }
+  mAttributeCache.Enumerate(RemoveMapRef, nullptr);
 }
 
 void
 nsDOMAttributeMap::DropReference()
 {
-  if (mAttributeCache) {
-    mAttributeCache->Enumerate(RemoveMapRef, nullptr);
-  }
+  mAttributeCache.Enumerate(RemoveMapRef, nullptr);
   mContent = nullptr;
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsDOMAttributeMap)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsDOMAttributeMap)
   tmp->DropReference();
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
@@ -82,19 +78,17 @@ TraverseMapEntry(nsAttrHashKey::KeyType 
     static_cast<nsCycleCollectionTraversalCallback*>(aUserArg);
 
   cb->NoteXPCOMChild(static_cast<nsINode*>(aData.get()));
 
   return PL_DHASH_NEXT;
 }
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsDOMAttributeMap)
-  if (tmp->mAttributeCache) {
-    tmp->mAttributeCache->Enumerate(TraverseMapEntry, &cb);
-  }
+  tmp->mAttributeCache.Enumerate(TraverseMapEntry, &cb);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mContent)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(nsDOMAttributeMap)
 
 NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_BEGIN(nsDOMAttributeMap)
   if (tmp->IsBlack()) {
@@ -138,79 +132,76 @@ SetOwnerDocumentFunc(nsAttrHashKey::KeyT
   nsresult rv = aData->SetOwnerDocument(static_cast<nsIDocument*>(aUserArg));
 
   return NS_FAILED(rv) ? PL_DHASH_STOP : PL_DHASH_NEXT;
 }
 
 nsresult
 nsDOMAttributeMap::SetOwnerDocument(nsIDocument* aDocument)
 {
-  if (mAttributeCache) {
-    uint32_t n = mAttributeCache->Enumerate(SetOwnerDocumentFunc, aDocument);
-    NS_ENSURE_TRUE(n == mAttributeCache->Count(), NS_ERROR_FAILURE);
-  }
+  uint32_t n = mAttributeCache.Enumerate(SetOwnerDocumentFunc, aDocument);
+  NS_ENSURE_TRUE(n == mAttributeCache.Count(), NS_ERROR_FAILURE);
+
   return NS_OK;
 }
 
 void
 nsDOMAttributeMap::DropAttribute(int32_t aNamespaceID, nsIAtom* aLocalName)
 {
   nsAttrKey attr(aNamespaceID, aLocalName);
-  if (mAttributeCache) {
-    Attr *node = mAttributeCache->GetWeak(attr);
-    if (node) {
-      // Break link to map
-      node->SetMap(nullptr);
+  Attr *node = mAttributeCache.GetWeak(attr);
+  if (node) {
+    // Break link to map
+    node->SetMap(nullptr);
 
-      // Remove from cache
-      mAttributeCache->Remove(attr);
-    }
+    // Remove from cache
+    mAttributeCache.Remove(attr);
   }
 }
 
 already_AddRefed<Attr>
 nsDOMAttributeMap::RemoveAttribute(mozilla::dom::NodeInfo* aNodeInfo)
 {
   NS_ASSERTION(aNodeInfo, "RemoveAttribute() called with aNodeInfo == nullptr!");
 
   nsAttrKey attr(aNodeInfo->NamespaceID(), aNodeInfo->NameAtom());
 
   nsRefPtr<Attr> node;
-  if (mAttributeCache && mAttributeCache->Get(attr, getter_AddRefs(node))) {
-    // Break link to map
-    node->SetMap(nullptr);
-
-    // Remove from cache
-    mAttributeCache->Remove(attr);
-  } else {
+  if (!mAttributeCache.Get(attr, getter_AddRefs(node))) {
     nsAutoString value;
     // As we are removing the attribute we need to set the current value in
     // the attribute node.
     mContent->GetAttr(aNodeInfo->NamespaceID(), aNodeInfo->NameAtom(), value);
     nsRefPtr<mozilla::dom::NodeInfo> ni = aNodeInfo;
     node = new Attr(nullptr, ni.forget(), value, true);
   }
+  else {
+    // Break link to map
+    node->SetMap(nullptr);
+
+    // Remove from cache
+    mAttributeCache.Remove(attr);
+  }
 
   return node.forget();
 }
 
 Attr*
 nsDOMAttributeMap::GetAttribute(mozilla::dom::NodeInfo* aNodeInfo, bool aNsAware)
 {
   NS_ASSERTION(aNodeInfo, "GetAttribute() called with aNodeInfo == nullptr!");
 
   nsAttrKey attr(aNodeInfo->NamespaceID(), aNodeInfo->NameAtom());
 
-  EnsureAttributeCache();
-  Attr* node = mAttributeCache->GetWeak(attr);
+  Attr* node = mAttributeCache.GetWeak(attr);
   if (!node) {
     nsRefPtr<mozilla::dom::NodeInfo> ni = aNodeInfo;
     nsRefPtr<Attr> newAttr =
       new Attr(this, ni.forget(), EmptyString(), aNsAware);
-    mAttributeCache->Put(attr, newAttr);
+    mAttributeCache.Put(attr, newAttr);
     node = newAttr;
   }
 
   return node;
 }
 
 Attr*
 nsDOMAttributeMap::NamedGetter(const nsAString& aAttrName, bool& aFound)
@@ -246,24 +237,16 @@ nsDOMAttributeMap::GetNamedItem(const ns
 {
   NS_ENSURE_ARG_POINTER(aAttribute);
 
   NS_IF_ADDREF(*aAttribute = GetNamedItem(aAttrName));
 
   return NS_OK;
 }
 
-void
-nsDOMAttributeMap::EnsureAttributeCache()
-{
-  if (!mAttributeCache) {
-    mAttributeCache = MakeUnique<AttrCache>();
-  }
-}
-
 NS_IMETHODIMP
 nsDOMAttributeMap::SetNamedItem(nsIDOMAttr* aAttr, nsIDOMAttr** aReturn)
 {
   Attr* attribute = static_cast<Attr*>(aAttr);
   NS_ENSURE_ARG(attribute);
 
   ErrorResult rv;
   *aReturn = SetNamedItem(*attribute, rv).take();
@@ -360,18 +343,17 @@ nsDOMAttributeMap::SetNamedItemInternal(
   nsAutoString value;
   aAttr.GetValue(value);
 
   nsRefPtr<NodeInfo> ni = aAttr.NodeInfo();
 
   // Add the new attribute to the attribute map before updating
   // its value in the element. @see bug 364413.
   nsAttrKey attrkey(ni->NamespaceID(), ni->NameAtom());
-  EnsureAttributeCache();
-  mAttributeCache->Put(attrkey, &aAttr);
+  mAttributeCache.Put(attrkey, &aAttr);
   aAttr.SetMap(this);
 
   rv = mContent->SetAttr(ni->NamespaceID(), ni->NameAtom(),
                          ni->GetPrefixAtom(), value, true);
   if (NS_FAILED(rv)) {
     aError.Throw(rv);
     DropAttribute(ni->NamespaceID(), ni->NameAtom());
   }
@@ -549,43 +531,41 @@ nsDOMAttributeMap::RemoveNamedItemNS(con
   }
 
   return RemoveNamedItem(ni, aError);
 }
 
 uint32_t
 nsDOMAttributeMap::Count() const
 {
-  return mAttributeCache ? mAttributeCache->Count() : 0;
+  return mAttributeCache.Count();
 }
 
 uint32_t
 nsDOMAttributeMap::Enumerate(AttrCache::EnumReadFunction aFunc,
                              void *aUserArg) const
 {
-  return mAttributeCache ? mAttributeCache->EnumerateRead(aFunc, aUserArg) : 0;
+  return mAttributeCache.EnumerateRead(aFunc, aUserArg);
 }
 
 size_t
 AttrCacheSizeEnumerator(const nsAttrKey& aKey,
                         const nsRefPtr<Attr>& aValue,
                         MallocSizeOf aMallocSizeOf,
                         void* aUserArg)
 {
   return aMallocSizeOf(aValue.get());
 }
 
 size_t
 nsDOMAttributeMap::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
 {
   size_t n = aMallocSizeOf(this);
-  n += mAttributeCache
-     ? mAttributeCache->SizeOfExcludingThis(AttrCacheSizeEnumerator,
-                                            aMallocSizeOf)
-     : 0;
+  n += mAttributeCache.SizeOfExcludingThis(AttrCacheSizeEnumerator,
+                                           aMallocSizeOf);
 
   // NB: mContent is non-owning and thus not counted.
   return n;
 }
 
 /* virtual */ JSObject*
 nsDOMAttributeMap::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 {
--- a/dom/base/nsDOMAttributeMap.h
+++ b/dom/base/nsDOMAttributeMap.h
@@ -6,17 +6,16 @@
 /*
  * Implementation of the |attributes| property of DOM Core's Element object.
  */
 
 #ifndef nsDOMAttributeMap_h
 #define nsDOMAttributeMap_h
 
 #include "mozilla/MemoryReporting.h"
-#include "mozilla/UniquePtr.h"
 #include "mozilla/dom/Attr.h"
 #include "mozilla/ErrorResult.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsIDOMMozNamedAttrMap.h"
 #include "nsRefPtrHashtable.h"
 #include "nsString.h"
 #include "nsWrapperCache.h"
 
@@ -181,21 +180,19 @@ public:
 
 protected:
   virtual ~nsDOMAttributeMap();
 
 private:
   nsCOMPtr<Element> mContent;
 
   /**
-   * Cache of Attrs. It's usually empty, and thus initialized lazily.
+   * Cache of Attrs.
    */
-  mozilla::UniquePtr<AttrCache> mAttributeCache;
-
-  void EnsureAttributeCache();
+  AttrCache mAttributeCache;
 
   /**
    * SetNamedItem() (aWithNS = false) and SetNamedItemNS() (aWithNS =
    * true) implementation.
    */
   already_AddRefed<Attr>
   SetNamedItemInternal(Attr& aNode, bool aWithNS, ErrorResult& aError);