From f7badd24e215fff3a9720d0818244c9e9ae276e5 Mon Sep 17 00:00:00 2001 From: Cameron Kaiser Date: Wed, 17 May 2017 21:30:02 -0700 Subject: [PATCH] #375: M1351303 --- dom/base/Element.cpp | 10 +++---- dom/base/nsAttrValue.cpp | 10 +++---- dom/base/nsContentUtils.cpp | 12 ++++----- parser/html/nsHtml5TreeOperation.h | 2 +- xpcom/ds/nsAtomTable.cpp | 42 ++++++++++++++++++++++++++++++ xpcom/ds/nsIAtom.idl | 5 ++++ 6 files changed, 64 insertions(+), 17 deletions(-) diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 43a5eff41..2f482091f 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -1197,10 +1197,10 @@ Element::SetAttribute(const nsAString& aName, if (IsHTMLElement() && IsInHTMLDocument()) { nsAutoString lower; nsContentUtils::ASCIIToLower(aName, lower); - nameAtom = do_GetAtom(lower); + nameAtom = NS_AtomizeMainThread(lower); } else { - nameAtom = do_GetAtom(aName); + nameAtom = NS_AtomizeMainThread(aName); } if (!nameAtom) { aError.Throw(NS_ERROR_OUT_OF_MEMORY); @@ -1282,7 +1282,7 @@ Element::GetAttributeNS(const nsAString& aNamespaceURI, return; } - nsCOMPtr name = do_GetAtom(aLocalName); + nsCOMPtr name = NS_AtomizeMainThread(aLocalName); bool hasAttr = GetAttr(nsid, name, aReturn); if (!hasAttr) { SetDOMStringToNull(aReturn); @@ -1314,7 +1314,7 @@ Element::RemoveAttributeNS(const nsAString& aNamespaceURI, const nsAString& aLocalName, ErrorResult& aError) { - nsCOMPtr name = do_GetAtom(aLocalName); + nsCOMPtr name = NS_AtomizeMainThread(aLocalName); int32_t nsid = nsContentUtils::NameSpaceManager()->GetNameSpaceID(aNamespaceURI); @@ -1400,7 +1400,7 @@ Element::HasAttributeNS(const nsAString& aNamespaceURI, return false; } - nsCOMPtr name = do_GetAtom(aLocalName); + nsCOMPtr name = NS_AtomizeMainThread(aLocalName); return HasAttr(nsid, name); } diff --git a/dom/base/nsAttrValue.cpp b/dom/base/nsAttrValue.cpp index 976d3f2fd..3ee8bd348 100644 --- a/dom/base/nsAttrValue.cpp +++ b/dom/base/nsAttrValue.cpp @@ -735,7 +735,7 @@ nsAttrValue::GetAsAtom() const { switch (Type()) { case eString: - return do_GetAtom(GetStringValue()); + return NS_AtomizeMainThread(GetStringValue()); case eAtom: { @@ -747,7 +747,7 @@ nsAttrValue::GetAsAtom() const { nsAutoString val; ToString(val); - return do_GetAtom(val); + return NS_AtomizeMainThread(val); } } } @@ -1260,7 +1260,7 @@ nsAttrValue::ParseAtomArray(const nsAString& aValue) ++iter; } while (iter != end && !nsContentUtils::IsHTMLWhitespace(*iter)); - nsCOMPtr classAtom = do_GetAtom(Substring(start, iter)); + nsCOMPtr classAtom = NS_AtomizeMainThread(Substring(start, iter)); if (!classAtom) { Reset(); return; @@ -1301,7 +1301,7 @@ nsAttrValue::ParseAtomArray(const nsAString& aValue) ++iter; } while (iter != end && !nsContentUtils::IsHTMLWhitespace(*iter)); - classAtom = do_GetAtom(Substring(start, iter)); + classAtom = NS_AtomizeMainThread(Substring(start, iter)); if (!array->AppendElement(classAtom)) { Reset(); @@ -1718,7 +1718,7 @@ nsAttrValue::SetMiscAtomOrString(const nsAString* aValue) "Empty string?"); MiscContainer* cont = GetMiscContainer(); if (len <= NS_ATTRVALUE_MAX_STRINGLENGTH_ATOM) { - nsCOMPtr atom = NS_NewAtom(*aValue); + nsCOMPtr atom = NS_AtomizeMainThread(*aValue); if (atom) { cont->mStringBits = reinterpret_cast(atom.forget().take()) | eAtomBase; diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index c73709865..42fe7d5ae 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -2850,11 +2850,11 @@ nsContentUtils::SplitQName(const nsIContent* aNamespaceResolver, if (*aNamespace == kNameSpaceID_Unknown) return NS_ERROR_FAILURE; - *aLocalName = NS_NewAtom(Substring(colon + 1, end)).take(); + *aLocalName = NS_AtomizeMainThread(Substring(colon + 1, end)).take(); } else { *aNamespace = kNameSpaceID_None; - *aLocalName = NS_NewAtom(aQName).take(); + *aLocalName = NS_AtomizeMainThread(aQName).take(); } NS_ENSURE_TRUE(aLocalName, NS_ERROR_OUT_OF_MEMORY); return NS_OK; @@ -2879,7 +2879,7 @@ nsContentUtils::GetNodeInfoFromQName(const nsAString& aNamespaceURI, const char16_t* end; qName.EndReading(end); - nsCOMPtr prefix = do_GetAtom(Substring(qName.get(), colon)); + nsCOMPtr prefix = NS_AtomizeMainThread(Substring(qName.get(), colon)); rv = aNodeInfoManager->GetNodeInfo(Substring(colon + 1, end), prefix, nsID, aNodeType, aNodeInfo); @@ -2939,7 +2939,7 @@ nsContentUtils::SplitExpatName(const char16_t *aExpatName, nsIAtom **aPrefix, nameStart = (uriEnd + 1); if (nameEnd) { const char16_t *prefixStart = nameEnd + 1; - *aPrefix = NS_NewAtom(Substring(prefixStart, pos)).take(); + *aPrefix = NS_AtomizeMainThread(Substring(prefixStart, pos)).take(); } else { nameEnd = pos; @@ -2952,7 +2952,7 @@ nsContentUtils::SplitExpatName(const char16_t *aExpatName, nsIAtom **aPrefix, nameEnd = pos; *aPrefix = nullptr; } - *aLocalName = NS_NewAtom(Substring(nameStart, nameEnd)).take(); + *aLocalName = NS_AtomizeMainThread(Substring(nameStart, nameEnd)).take(); } // static @@ -3708,7 +3708,7 @@ nsContentUtils::GetEventMessageAndAtom(const nsAString& aName, } *aEventMessage = eUnidentifiedEvent; - nsCOMPtr atom = do_GetAtom(NS_LITERAL_STRING("on") + aName); + nsCOMPtr atom = NS_AtomizeMainThread(NS_LITERAL_STRING("on") + aName); sUserDefinedEvents->AppendObject(atom); mapping.mAtom = atom; mapping.mMessage = eUnidentifiedEvent; diff --git a/parser/html/nsHtml5TreeOperation.h b/parser/html/nsHtml5TreeOperation.h index e4881b4d1..5958cfe36 100644 --- a/parser/html/nsHtml5TreeOperation.h +++ b/parser/html/nsHtml5TreeOperation.h @@ -110,7 +110,7 @@ class nsHtml5TreeOperation { } nsAutoString str; aAtom->ToString(str); - return do_GetAtom(str); + return NS_AtomizeMainThread(str); } static nsresult AppendTextToTextNode(const char16_t* aBuffer, diff --git a/xpcom/ds/nsAtomTable.cpp b/xpcom/ds/nsAtomTable.cpp index 6ca0e8001..ae8f61f91 100644 --- a/xpcom/ds/nsAtomTable.cpp +++ b/xpcom/ds/nsAtomTable.cpp @@ -298,6 +298,9 @@ static const PLDHashTableOps AtomTableOps = { AtomTableInitEntry }; +#define RECENTLY_USED_MAIN_THREAD_ATOM_CACHE_SIZE 31 +static nsIAtom* + sRecentlyUsedMainThreadAtoms[RECENTLY_USED_MAIN_THREAD_ATOM_CACHE_SIZE] = {}; static inline void @@ -651,6 +654,8 @@ NS_NewAtom(const char16_t* aUTF16String) return NS_NewAtom(nsDependentString(aUTF16String)); } +// Equivalent to current NS_Atomize and called by NS_AtomizeMainThread. +// Left as such for legacy callers in our older 45-era codebase. already_AddRefed NS_NewAtom(const nsAString& aUTF16String) { @@ -671,6 +676,43 @@ NS_NewAtom(const nsAString& aUTF16String) return atom.forget(); } +// From bug 1351303, modified for Mozilla 45. +already_AddRefed +NS_AtomizeMainThread(const nsAString& aUTF16String) +{ + MOZ_ASSERT(NS_IsMainThread()); + nsCOMPtr retVal; + uint32_t hash; + AtomTableKey key(aUTF16String.Data(), aUTF16String.Length(), &hash); + uint32_t index = hash % RECENTLY_USED_MAIN_THREAD_ATOM_CACHE_SIZE; + nsIAtom* atom = sRecentlyUsedMainThreadAtoms[index]; + + if (atom) { + // This isn't ideal, but covers for the collision case, I guess. + // The atom names shouldn't be very long in any event. + uint32_t length = atom->GetLength(); + if (length == key.mLength && + (memcmp(atom->GetUTF16String(), + key.mUTF16String, length * sizeof(char16_t)) == 0)) { + retVal = atom; + return retVal.forget(); + } + } + + // Inline relevant parts of GetAtomHashEntry. + AtomTableEntry* he = static_cast(gAtomTable->Add(&key)); + if (he->mAtom) { + retVal = he->mAtom; + } else { + RefPtr atom = new AtomImpl(aUTF16String, hash); + he->mAtom = atom; + retVal = he->mAtom; // XXX? + } + + sRecentlyUsedMainThreadAtoms[index] = retVal; + return retVal.forget(); +} + nsIAtom* NS_NewPermanentAtom(const nsAString& aUTF16String) { diff --git a/xpcom/ds/nsIAtom.idl b/xpcom/ds/nsIAtom.idl index da7d23792..f5f67ab23 100644 --- a/xpcom/ds/nsIAtom.idl +++ b/xpcom/ds/nsIAtom.idl @@ -127,6 +127,11 @@ extern nsIAtom* NS_NewPermanentAtom(const nsAString& aUTF16String); inline already_AddRefed do_GetAtom(const nsAString& aUTF16String) { return NS_NewAtom(aUTF16String); } +/** + * An optimized version of the method above for the main thread (bug 1351303). + */ +extern already_AddRefed NS_AtomizeMainThread(const nsAString& aUTF16String); + /** * Return a count of the total number of atoms currently * alive in the system.