nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1843946 - Use an atom for the full class attribute value, to reduce string hash key cost during the AtomArrayCache lookup. r=emilio

Browse source 
This generalizes the previous "set single class from parser" fast-path to
also apply when there are multiple classes in the class attribute.

The main benefit is the cheaper cache lookup.
Another benefit is that this avoids a string copy (the same copy that the original
"set single class from parser" optimization avoided).

We now always atomize the full class attribute value before parsing it,
e.g. when somebody uses setAttribute. In the common case of a single class,
this atomization cost would have been paid anyway.

Before: https://share.firefox.dev/4576ulw
After: https://share.firefox.dev/3rNGBsH (though this is not the full story
because the atomization work is moved outside of nsHtml5TreeOperation::SetHTMLElementAttributes)

Differential Revision: https://phabricator.services.mozilla.com/D183813
This commit is contained in:
Markus Stange 2023-08-17 19:02:40 +00:00
parent eacd86d305
commit 943e3c05a7
6 changed files with 58 additions and 42 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

5
dom/base/Element.cpp
View file

@ -2451,10 +2451,11 @@ bool Element::OnlyNotifySameValueSet(int32_t aNamespaceID, nsAtom* aName,
return true;
}
nsresult Element::SetSingleClassFromParser(nsAtom* aSingleClassName) {
nsresult Element::SetClassAttrFromParser(nsAtom* aValue) {
// Keep this in sync with SetAttr and SetParsedAttr below.
nsAttrValue value(aSingleClassName);
nsAttrValue value;
value.ParseAtomArray(aValue);
Document* document = GetComposedDoc();
mozAutoDocUpdate updateBatch(document, false);

4
dom/base/Element.h
View file

@ -878,11 +878,11 @@ class Element : public FragmentOrElement {
bool* aHasListeners, bool* aOldValueSet);
/**
* Sets the class attribute to a value that contains no whitespace.
* Sets the class attribute.
* Assumes that we are not notifying and that the attribute hasn't been
* set previously.
*/
nsresult SetSingleClassFromParser(nsAtom* aSingleClassName);
nsresult SetClassAttrFromParser(nsAtom* aValue);
// aParsedValue receives the old value of the attribute. That's useful if
// either the input or output value of aParsedValue is StoresOwnData.

72
dom/base/nsAttrValue.cpp
View file

@ -49,22 +49,26 @@ static uint32_t gMiscContainerCount = 0;
* the cache once its last reference is dropped.
*/
struct AtomArrayCache {
using MapType = nsTHashMap<nsStringHashKey, MiscContainer*>;
// We don't keep any strong references, neither to the atom nor to the
// MiscContainer. The MiscContainer removes itself from the cache when
// the last reference to it is dropped, and the atom is kept alive by
// the MiscContainer.
using MapType = nsTHashMap<nsPtrHashKey<nsAtom>, MiscContainer*>;
static MiscContainer* Lookup(const nsAString& aValue) {
static MiscContainer* Lookup(nsAtom* aValue) {
if (auto* instance = GetInstance()) {
return instance->LookupImpl(aValue);
}
return nullptr;
}
static void Insert(const nsAString& aValue, MiscContainer* aCont) {
static void Insert(nsAtom* aValue, MiscContainer* aCont) {
if (auto* instance = GetInstance()) {
instance->InsertImpl(aValue, aCont);
}
}
static void Remove(const nsAString& aValue) {
static void Remove(nsAtom* aValue) {
if (auto* instance = GetInstance()) {
instance->RemoveImpl(aValue);
}
@ -80,17 +84,17 @@ struct AtomArrayCache {
}
private:
MiscContainer* LookupImpl(const nsAString& aValue) {
MiscContainer* LookupImpl(nsAtom* aValue) {
auto lookupResult = mMap.Lookup(aValue);
return lookupResult ? *lookupResult : nullptr;
}
void InsertImpl(const nsAString& aValue, MiscContainer* aCont) {
void InsertImpl(nsAtom* aValue, MiscContainer* aCont) {
MOZ_ASSERT(aCont);
mMap.InsertOrUpdate(aValue, aCont);
}
void RemoveImpl(const nsAString& aValue) { mMap.Remove(aValue); }
void RemoveImpl(nsAtom* aValue) { mMap.Remove(aValue); }
MapType mMap;
};
@ -177,13 +181,12 @@ void MiscContainer::Cache() {
MOZ_ASSERT(mValue.mRefCount > 0);
MOZ_ASSERT(!mValue.mCached);
nsString str;
bool gotString = GetString(str);
if (!gotString) {
nsAtom* atom = GetStoredAtom();
if (!atom) {
return;
}
AtomArrayCache::Insert(str, this);
AtomArrayCache::Insert(atom, this);
mValue.mCached = 1;
break;
}
@ -223,11 +226,10 @@ void MiscContainer::Evict() {
return;
}
nsString str;
DebugOnly<bool> gotString = GetString(str);
MOZ_ASSERT(gotString);
nsAtom* atom = GetStoredAtom();
MOZ_ASSERT(atom);
AtomArrayCache::Remove(str);
AtomArrayCache::Remove(atom);
mValue.mCached = 0;
break;
@ -1339,7 +1341,7 @@ void nsAttrValue::ParseAtom(const nsAString& aValue) {
}
}
void nsAttrValue::ParseAtomArray(const nsAString& aValue) {
void nsAttrValue::ParseAtomArray(nsAtom* aValue) {
if (MiscContainer* cont = AtomArrayCache::Lookup(aValue)) {
// Set our MiscContainer to the cached one.
NS_ADDREF(cont);
@ -1347,9 +1349,8 @@ void nsAttrValue::ParseAtomArray(const nsAString& aValue) {
return;
}
nsAString::const_iterator iter, end;
aValue.BeginReading(iter);
aValue.EndReading(end);
const char16_t* iter = aValue->GetUTF16String();
const char16_t* end = iter + aValue->GetLength();
bool hasSpace = false;
// skip initial whitespace
@ -1359,20 +1360,26 @@ void nsAttrValue::ParseAtomArray(const nsAString& aValue) {
}
if (iter == end) {
SetTo(aValue);
// The value is empty or only contains whitespace.
// Set this attribute to the string value.
// We don't call the SetTo(nsAtom*) overload because doing so would
// leave us with a classList of length 1.
SetTo(nsDependentAtomString(aValue));
return;
}
nsAString::const_iterator start(iter);
const char16_t* start = iter;
// get first - and often only - atom
do {
++iter;
} while (iter != end && !nsContentUtils::IsHTMLWhitespace(*iter));
RefPtr<nsAtom> classAtom = NS_AtomizeMainThread(Substring(start, iter));
RefPtr<nsAtom> classAtom = iter == end && !hasSpace
? RefPtr<nsAtom>(aValue).forget()
: NS_AtomizeMainThread(Substring(start, iter));
if (!classAtom) {
Reset();
ResetIfSet();
return;
}
@ -1392,6 +1399,7 @@ void nsAttrValue::ParseAtomArray(const nsAString& aValue) {
return;
}
// We have at least one class atom. Create a new AttrAtomArray.
AttrAtomArray* array = new AttrAtomArray;
// XXX(Bug 1631371) Check if this should use a fallible operation as it
@ -1419,6 +1427,7 @@ void nsAttrValue::ParseAtomArray(const nsAString& aValue) {
}
}
// Wrap the AtomArray into a fresh MiscContainer.
MiscContainer* cont = EnsureEmptyMiscContainer();
MOZ_ASSERT(cont->mValue.mRefCount == 0);
cont->mValue.mAtomArray = array;
@ -1426,11 +1435,26 @@ void nsAttrValue::ParseAtomArray(const nsAString& aValue) {
NS_ADDREF(cont);
MOZ_ASSERT(cont->mValue.mRefCount == 1);
SetMiscAtomOrString(&aValue);
// Assign the atom to the container's string bits (like SetMiscAtomOrString
// would do).
MOZ_ASSERT(!IsInServoTraversal());
aValue->AddRef();
uintptr_t bits = reinterpret_cast<uintptr_t>(aValue) | eAtomBase;
cont->SetStringBitsMainThread(bits);
// Put the container in the cache.
cont->Cache();
}
void nsAttrValue::ParseAtomArray(const nsAString& aValue) {
if (aValue.IsVoid()) {
ResetIfSet();
} else {
RefPtr<nsAtom> atom = NS_AtomizeMainThread(aValue);
ParseAtomArray(atom);
}
}
void nsAttrValue::ParseStringOrAtom(const nsAString& aValue) {
uint32_t len = aValue.Length();
// Don't bother with atoms if it's an empty string since

1
dom/base/nsAttrValue.h
View file

@ -293,6 +293,7 @@ class nsAttrValue {
void ParseAtom(const nsAString& aValue);
void ParseAtomArray(const nsAString& aValue);
void ParseAtomArray(nsAtom* aValue);
void ParseStringOrAtom(const nsAString& aValue);
/**

12
parser/html/nsHtml5Portability.cpp
View file

@ -24,23 +24,13 @@ nsAtom* nsHtml5Portability::newLocalNameFromBuffer(char16_t* buf,
return interner->GetAtom(nsDependentSubstring(buf, buf + length));
}
static bool ContainsWhiteSpace(mozilla::Span<char16_t> aSpan) {
for (char16_t c : aSpan) {
if (nsContentUtils::IsHTMLWhitespace(c)) {
return true;
}
}
return false;
}
nsHtml5String nsHtml5Portability::newStringFromBuffer(
char16_t* buf, int32_t offset, int32_t length,
nsHtml5TreeBuilder* treeBuilder, bool maybeAtomize) {
if (!length) {
return nsHtml5String::EmptyString();
}
if (maybeAtomize &&
!ContainsWhiteSpace(mozilla::Span(buf + offset, length))) {
if (maybeAtomize) {
return nsHtml5String::FromAtom(
NS_AtomizeMainThread(nsDependentSubstring(buf + offset, length)));
}

6
parser/html/nsHtml5TreeOperation.cpp
View file

@ -410,7 +410,7 @@ void nsHtml5TreeOperation::SetHTMLElementAttributes(
nsHtml5String val = aAttributes->getValueNoBoundsCheck(i);
nsAtom* klass = val.MaybeAsAtom();
if (klass) {
aElement->SetSingleClassFromParser(klass);
aElement->SetClassAttrFromParser(klass);
} else {
nsAtom* localName = aAttributes->getLocalNameNoBoundsCheck(i);
nsAtom* prefix = aAttributes->getPrefixNoBoundsCheck(i);
@ -545,7 +545,7 @@ nsIContent* nsHtml5TreeOperation::CreateSVGElement(
nsHtml5String val = aAttributes->getValueNoBoundsCheck(i);
nsAtom* klass = val.MaybeAsAtom();
if (klass) {
newContent->SetSingleClassFromParser(klass);
newContent->SetClassAttrFromParser(klass);
} else {
nsAtom* localName = aAttributes->getLocalNameNoBoundsCheck(i);
nsAtom* prefix = aAttributes->getPrefixNoBoundsCheck(i);
@ -593,7 +593,7 @@ nsIContent* nsHtml5TreeOperation::CreateMathMLElement(
nsHtml5String val = aAttributes->getValueNoBoundsCheck(i);
nsAtom* klass = val.MaybeAsAtom();
if (klass) {
newContent->SetSingleClassFromParser(klass);
newContent->SetClassAttrFromParser(klass);
} else {
nsAtom* localName = aAttributes->getLocalNameNoBoundsCheck(i);
nsAtom* prefix = aAttributes->getPrefixNoBoundsCheck(i);