From 35754dd1af72abcae49edd2eeb76855d999a4e6d Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Sat, 13 Jun 2020 08:18:46 -0400 Subject: Bug 1355787 - nsIdentifierMapEntry should let one to use either strings or atoms as keys to avoid slow string assignments when possible. Tag #1375 --- dom/base/ShadowRoot.cpp | 6 ++-- dom/base/nsDocument.cpp | 18 ++++------- dom/base/nsIdentifierMapEntry.h | 72 +++++++++++++++++++++++++++++++++++++---- dom/html/nsHTMLDocument.cpp | 2 +- 4 files changed, 76 insertions(+), 22 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 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 mIdContentList; diff --git a/dom/html/nsHTMLDocument.cpp b/dom/html/nsHTMLDocument.cpp index bd9de39f03..f3cb096b9a 100644 --- a/dom/html/nsHTMLDocument.cpp +++ b/dom/html/nsHTMLDocument.cpp @@ -2094,7 +2094,7 @@ nsHTMLDocument::GetSupportedNames(nsTArray& aNames) nsIdentifierMapEntry* entry = iter.Get(); if (entry->HasNameElement() || entry->HasIdElementExposedAsHTMLDocumentProperty()) { - aNames.AppendElement(entry->GetKey()); + aNames.AppendElement(entry->GetKeyAsString()); } } } -- cgit v1.2.3