Bug 1355787 - nsIdentifierMapEntry should let one to use either strings or atoms as keys to avoid slow string assignments when possible.

Tag #1375

3 files changed, 75 insertions, 21 deletions

diff --git a/dom/base/ShadowRoot.cpp b/dom/base/ShadowRoot.cpp
index 2383c951a6..d6c103e8ef 100644
--- a/dom/base/ShadowRoot.cpp
+++ b/dom/base/ShadowRoot.cpp
@@ -319,8 +319,7 @@ ShadowRoot::GetElementsByTagNameNS(const nsAString& aNamespaceURI,
 void
 ShadowRoot::AddToIdTable(Element* aElement, nsIAtom* aId)
 {
-  nsIdentifierMapEntry *entry =
-    mIdentifierMap.PutEntry(nsDependentAtomString(aId));
+  nsIdentifierMapEntry* entry = mIdentifierMap.PutEntry(aId);
   if (entry) {
     entry->AddIdElement(aElement);
   }
@@ -329,8 +328,7 @@ ShadowRoot::AddToIdTable(Element* aElement, nsIAtom* aId)
 void
 ShadowRoot::RemoveFromIdTable(Element* aElement, nsIAtom* aId)
 {
-  nsIdentifierMapEntry *entry =
-    mIdentifierMap.GetEntry(nsDependentAtomString(aId));
+  nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aId);
   if (entry) {
     entry->RemoveIdElement(aElement);
     if (entry->IsEmpty()) {
diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp
index 7b280a188d..203c44cbf4 100644
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -537,7 +537,7 @@ nsIdentifierMapEntry::HasIdElementExposedAsHTMLDocumentProperty()
 size_t
 nsIdentifierMapEntry::SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const
 {
-  return nsStringHashKey::SizeOfExcludingThis(aMallocSizeOf);
+  return mKey.mString.SizeOfExcludingThisIfUnshared(aMallocSizeOf);
 }
 
 // Helper structs for the content->subdoc map
@@ -2704,8 +2704,7 @@ nsDocument::AddToNameTable(Element *aElement, nsIAtom* aName)
              "Only put elements that need to be exposed as document['name'] in "
              "the named table.");
 
-  nsIdentifierMapEntry *entry =
-    mIdentifierMap.PutEntry(nsDependentAtomString(aName));
+  nsIdentifierMapEntry* entry = mIdentifierMap.PutEntry(aName);
 
   // Null for out-of-memory
   if (entry) {
@@ -2724,8 +2723,7 @@ nsDocument::RemoveFromNameTable(Element *aElement, nsIAtom* aName)
   if (mIdentifierMap.Count() == 0)
     return;
 
-  nsIdentifierMapEntry *entry =
-    mIdentifierMap.GetEntry(nsDependentAtomString(aName));
+  nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aName);
   if (!entry) // Could be false if the element was anonymous, hence never added
     return;
 
@@ -2739,8 +2737,7 @@ nsDocument::RemoveFromNameTable(Element *aElement, nsIAtom* aName)
 void
 nsDocument::AddToIdTable(Element *aElement, nsIAtom* aId)
 {
-  nsIdentifierMapEntry *entry =
-    mIdentifierMap.PutEntry(nsDependentAtomString(aId));
+  nsIdentifierMapEntry* entry = mIdentifierMap.PutEntry(aId);
 
   if (entry) { /* True except on OOM */
     if (nsGenericHTMLElement::ShouldExposeIdAsHTMLDocumentProperty(aElement) &&
@@ -2762,8 +2759,7 @@ nsDocument::RemoveFromIdTable(Element *aElement, nsIAtom* aId)
     return;
   }
 
-  nsIdentifierMapEntry *entry =
-    mIdentifierMap.GetEntry(nsDependentAtomString(aId));
+  nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aId);
   if (!entry) // Can be null for XML elements with changing ids.
     return;
 
@@ -4783,7 +4779,7 @@ nsDocument::AddIDTargetObserver(nsIAtom* aID, IDTargetObserver aObserver,
   if (!CheckGetElementByIdArg(id))
     return nullptr;
 
-  nsIdentifierMapEntry *entry = mIdentifierMap.PutEntry(id);
+  nsIdentifierMapEntry* entry = mIdentifierMap.PutEntry(aID);
   NS_ENSURE_TRUE(entry, nullptr);
 
   entry->AddContentChangeCallback(aObserver, aData, aForImage);
@@ -4799,7 +4795,7 @@ nsDocument::RemoveIDTargetObserver(nsIAtom* aID, IDTargetObserver aObserver,
   if (!CheckGetElementByIdArg(id))
     return;
 
-  nsIdentifierMapEntry *entry = mIdentifierMap.GetEntry(id);
+  nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aID);
   if (!entry) {
     return;
   }
diff --git a/dom/base/nsIdentifierMapEntry.h b/dom/base/nsIdentifierMapEntry.h
index ea60d33221..41b8b4a836 100644
--- a/dom/base/nsIdentifierMapEntry.h
+++ b/dom/base/nsIdentifierMapEntry.h
@@ -40,22 +40,45 @@ class nsIContent;
  * Perhaps the document.all results should have their own hashtable
  * in nsHTMLDocument.
  */
-class nsIdentifierMapEntry : public nsStringHashKey
+class nsIdentifierMapEntry : public PLDHashEntryHdr
 {
 public:
+  struct AtomOrString
+  {
+    MOZ_IMPLICIT AtomOrString(nsIAtom* aAtom) : mAtom(aAtom) {}
+    MOZ_IMPLICIT AtomOrString(const nsAString& aString) : mString(aString) {}
+    AtomOrString(const AtomOrString& aOther)
+      : mAtom(aOther.mAtom)
+      , mString(aOther.mString)
+    {
+    }
+
+    AtomOrString(AtomOrString&& aOther)
+      : mAtom(aOther.mAtom.forget())
+      , mString(aOther.mString)
+    {
+    }
+
+    nsCOMPtr<nsIAtom> mAtom;
+    const nsString mString;
+  };
+
+  typedef const AtomOrString& KeyType;
+  typedef const AtomOrString* KeyTypePointer;
+
   typedef mozilla::dom::Element Element;
   typedef mozilla::net::ReferrerPolicy ReferrerPolicy;
 
-  explicit nsIdentifierMapEntry(const nsAString& aKey) :
-    nsStringHashKey(&aKey), mNameContentList(nullptr)
+  explicit nsIdentifierMapEntry(const AtomOrString& aKey)
+    : mKey(aKey)
   {
   }
-  explicit nsIdentifierMapEntry(const nsAString* aKey) :
-    nsStringHashKey(aKey), mNameContentList(nullptr)
+  explicit nsIdentifierMapEntry(const AtomOrString* aKey)
+    : mKey(aKey ? *aKey : nullptr)
   {
   }
   nsIdentifierMapEntry(nsIdentifierMapEntry&& aOther) :
-    nsStringHashKey(&aOther.GetKey()),
+    mKey(mozilla::Move(aOther.GetKey())),
     mIdContentList(mozilla::Move(aOther.mIdContentList)),
     mNameContentList(aOther.mNameContentList.forget()),
     mChangeCallbacks(aOther.mChangeCallbacks.forget()),
@@ -64,6 +87,42 @@ public:
   }
   ~nsIdentifierMapEntry();
 
+  KeyType GetKey() const { return mKey; }
+
+  nsString GetKeyAsString() const
+  {
+    if (mKey.mAtom) {
+      return nsAtomString(mKey.mAtom);
+    }
+
+    return mKey.mString;
+  }
+
+  bool KeyEquals(const KeyTypePointer aOtherKey) const
+  {
+    if (mKey.mAtom) {
+      if (aOtherKey->mAtom) {
+        return mKey.mAtom == aOtherKey->mAtom;
+      }
+
+      return mKey.mAtom->Equals(aOtherKey->mString);
+    }
+
+    if (aOtherKey->mAtom) {
+      return aOtherKey->mAtom->Equals(mKey.mString);
+    }
+
+    return mKey.mString.Equals(aOtherKey->mString);
+  }
+
+  static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; }
+
+  static PLDHashNumber HashKey(const KeyTypePointer aKey)
+  {
+    return aKey->mAtom ?
+      aKey->mAtom->hash() : mozilla::HashString(aKey->mString);
+  }
+
   enum { ALLOW_MEMMOVE = false };
 
   void AddNameElement(nsINode* aDocument, Element* aElement);
@@ -167,6 +226,7 @@ private:
   void FireChangeCallbacks(Element* aOldElement, Element* aNewElement,
                            bool aImageOnly = false);
 
+  AtomOrString mKey;
   // empty if there are no elements with this ID.
   // The elements are stored as weak pointers.
   AutoTArray<Element*, 1> mIdContentList;