Commit 88bd95fa authored by Albert J. Wong's avatar Albert J. Wong Committed by Commit Bot

Use StringViews in places in hot paths.

This is necessary to prevent reduce refcount thrashing when
AtomicStrings become threadsafe.

This CL is broken out of
https://chromium-review.googlesource.com/c/chromium/src/+/1557854

See this design doc for more details on the larger motivation for
this change:
https://docs.google.com/document/d/1hp7h6zfD5S6mnMI4cHe1PpacRnb2rNpRRkn1gBBJHuQ/edit#heading=h.erv1bv18t616

Bug: 1083392
Change-Id: Ib6f3a71d0153cc8cff178597e4e9cbd18cd38ea4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2285518
Commit-Queue: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: default avatarJeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789119}
parent 46824e8d
...@@ -234,7 +234,7 @@ StringImpl* InvalidationSet::FindAnyClass(Element& element) const { ...@@ -234,7 +234,7 @@ StringImpl* InvalidationSet::FindAnyClass(Element& element) const {
StringImpl* InvalidationSet::FindAnyAttribute(Element& element) const { StringImpl* InvalidationSet::FindAnyAttribute(Element& element) const {
if (StringImpl* string_impl = attributes_.GetStringImpl(backing_flags_)) { if (StringImpl* string_impl = attributes_.GetStringImpl(backing_flags_)) {
if (element.HasAttributeIgnoringNamespace(AtomicString(string_impl))) if (element.HasAttributeIgnoringNamespace(string_impl))
return string_impl; return string_impl;
} }
if (const HashSet<AtomicString>* set = if (const HashSet<AtomicString>* set =
......
...@@ -1552,7 +1552,7 @@ RadioNodeList* ContainerNode::GetRadioNodeList(const AtomicString& name, ...@@ -1552,7 +1552,7 @@ RadioNodeList* ContainerNode::GetRadioNodeList(const AtomicString& name,
return EnsureCachedCollection<RadioNodeList>(type, name); return EnsureCachedCollection<RadioNodeList>(type, name);
} }
Element* ContainerNode::getElementById(const AtomicString& id) const { Element* ContainerNode::getElementById(const StringView& id) const {
// According to https://dom.spec.whatwg.org/#concept-id, empty IDs are // According to https://dom.spec.whatwg.org/#concept-id, empty IDs are
// treated as equivalent to the lack of an id attribute. // treated as equivalent to the lack of an id attribute.
if (id.IsEmpty()) { if (id.IsEmpty()) {
......
...@@ -125,7 +125,7 @@ class CORE_EXPORT ContainerNode : public Node { ...@@ -125,7 +125,7 @@ class CORE_EXPORT ContainerNode : public Node {
const Node* old_child, const Node* old_child,
ExceptionState&) const; ExceptionState&) const;
Element* getElementById(const AtomicString& id) const; Element* getElementById(const StringView& id) const;
HTMLCollection* getElementsByTagName(const AtomicString&); HTMLCollection* getElementsByTagName(const AtomicString&);
HTMLCollection* getElementsByTagNameNS(const AtomicString& namespace_uri, HTMLCollection* getElementsByTagNameNS(const AtomicString& namespace_uri,
const AtomicString& local_name); const AtomicString& local_name);
......
...@@ -12,12 +12,12 @@ namespace blink { ...@@ -12,12 +12,12 @@ namespace blink {
class NonElementParentNode { class NonElementParentNode {
public: public:
static Element* getElementById(Document& document, const AtomicString& id) { static Element* getElementById(Document& document, const StringView& id) {
return document.getElementById(id); return document.getElementById(id);
} }
static Element* getElementById(DocumentFragment& fragment, static Element* getElementById(DocumentFragment& fragment,
const AtomicString& id) { const StringView& id) {
return fragment.getElementById(id); return fragment.getElementById(id);
} }
}; };
......
...@@ -41,6 +41,44 @@ ...@@ -41,6 +41,44 @@
namespace blink { namespace blink {
namespace {
inline bool KeyMatchesId(const StringView& key, const Element& element) {
return element.GetIdAttribute() == key;
}
inline bool KeyMatchesMapName(const StringView& key, const Element& element) {
auto* html_map_element = DynamicTo<HTMLMapElement>(element);
return html_map_element && html_map_element->GetName() == key;
}
inline bool KeyMatchesSlotName(const StringView& key, const Element& element) {
auto* html_slot_element = DynamicTo<HTMLSlotElement>(element);
return html_slot_element && html_slot_element->GetName() == key;
}
struct StringViewLookupTranslator {
static unsigned GetHash(const StringView& buf) {
StringImpl* shared_impl = buf.SharedImpl();
if (LIKELY(shared_impl))
return shared_impl->GetHash();
if (buf.Is8Bit()) {
return StringHasher::ComputeHashAndMaskTop8Bits(buf.Characters8(),
buf.length());
} else {
return StringHasher::ComputeHashAndMaskTop8Bits(buf.Characters16(),
buf.length());
}
}
static bool Equal(const AtomicString& str, const StringView& buf) {
return str == buf;
}
};
} // namespace
TreeOrderedMap::TreeOrderedMap() = default; TreeOrderedMap::TreeOrderedMap() = default;
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
...@@ -56,21 +94,6 @@ TreeOrderedMap::RemoveScope::~RemoveScope() { ...@@ -56,21 +94,6 @@ TreeOrderedMap::RemoveScope::~RemoveScope() {
} }
#endif #endif
inline bool KeyMatchesId(const AtomicString& key, const Element& element) {
return element.GetIdAttribute() == key;
}
inline bool KeyMatchesMapName(const AtomicString& key, const Element& element) {
auto* html_map_element = DynamicTo<HTMLMapElement>(element);
return html_map_element && html_map_element->GetName() == key;
}
inline bool KeyMatchesSlotName(const AtomicString& key,
const Element& element) {
auto* html_slot_element = DynamicTo<HTMLSlotElement>(element);
return html_slot_element && html_slot_element->GetName() == key;
}
void TreeOrderedMap::Add(const AtomicString& key, Element& element) { void TreeOrderedMap::Add(const AtomicString& key, Element& element) {
DCHECK(key); DCHECK(key);
...@@ -86,10 +109,10 @@ void TreeOrderedMap::Add(const AtomicString& key, Element& element) { ...@@ -86,10 +109,10 @@ void TreeOrderedMap::Add(const AtomicString& key, Element& element) {
entry->ordered_list.clear(); entry->ordered_list.clear();
} }
void TreeOrderedMap::Remove(const AtomicString& key, Element& element) { void TreeOrderedMap::Remove(const StringView& key, Element& element) {
DCHECK(key); DCHECK(!key.IsNull());
Map::iterator it = map_.find(key); Map::iterator it = map_.Find<StringViewLookupTranslator>(key);
if (it == map_.end()) if (it == map_.end())
return; return;
...@@ -110,15 +133,17 @@ void TreeOrderedMap::Remove(const AtomicString& key, Element& element) { ...@@ -110,15 +133,17 @@ void TreeOrderedMap::Remove(const AtomicString& key, Element& element) {
} }
} }
template <bool keyMatches(const AtomicString&, const Element&)> template <bool keyMatches(const StringView&, const Element&)>
inline Element* TreeOrderedMap::Get(const AtomicString& key, inline Element* TreeOrderedMap::Get(const StringView& key,
const TreeScope& scope) const { const TreeScope& scope) const {
DCHECK(key); DCHECK(!key.IsNull());
MapEntry* entry = map_.at(key); Map::iterator it = map_.Find<StringViewLookupTranslator>(key);
if (!entry) if (it == map_.end())
return nullptr; return nullptr;
MapEntry* entry = it->value;
DCHECK(entry);
DCHECK(entry->count); DCHECK(entry->count);
if (entry->element) if (entry->element)
return entry->element; return entry->element;
...@@ -141,19 +166,19 @@ inline Element* TreeOrderedMap::Get(const AtomicString& key, ...@@ -141,19 +166,19 @@ inline Element* TreeOrderedMap::Get(const AtomicString& key,
return nullptr; return nullptr;
} }
Element* TreeOrderedMap::GetElementById(const AtomicString& key, Element* TreeOrderedMap::GetElementById(const StringView& key,
const TreeScope& scope) const { const TreeScope& scope) const {
return Get<KeyMatchesId>(key, scope); return Get<KeyMatchesId>(key, scope);
} }
const HeapVector<Member<Element>>& TreeOrderedMap::GetAllElementsById( const HeapVector<Member<Element>>& TreeOrderedMap::GetAllElementsById(
const AtomicString& key, const StringView& key,
const TreeScope& scope) const { const TreeScope& scope) const {
DCHECK(key); DCHECK(!key.IsNull());
DEFINE_STATIC_LOCAL(Persistent<HeapVector<Member<Element>>>, empty_vector, DEFINE_STATIC_LOCAL(Persistent<HeapVector<Member<Element>>>, empty_vector,
(MakeGarbageCollected<HeapVector<Member<Element>>>())); (MakeGarbageCollected<HeapVector<Member<Element>>>()));
Map::iterator it = map_.find(key); Map::iterator it = map_.Find<StringViewLookupTranslator>(key);
if (it == map_.end()) if (it == map_.end())
return *empty_vector; return *empty_vector;
...@@ -179,13 +204,13 @@ const HeapVector<Member<Element>>& TreeOrderedMap::GetAllElementsById( ...@@ -179,13 +204,13 @@ const HeapVector<Member<Element>>& TreeOrderedMap::GetAllElementsById(
return entry->ordered_list; return entry->ordered_list;
} }
Element* TreeOrderedMap::GetElementByMapName(const AtomicString& key, Element* TreeOrderedMap::GetElementByMapName(const StringView& key,
const TreeScope& scope) const { const TreeScope& scope) const {
return Get<KeyMatchesMapName>(key, scope); return Get<KeyMatchesMapName>(key, scope);
} }
// TODO(hayato): Template get<> by return type. // TODO(hayato): Template get<> by return type.
HTMLSlotElement* TreeOrderedMap::GetSlotByName(const AtomicString& key, HTMLSlotElement* TreeOrderedMap::GetSlotByName(const StringView& key,
const TreeScope& scope) const { const TreeScope& scope) const {
if (Element* slot = Get<KeyMatchesSlotName>(key, scope)) if (Element* slot = Get<KeyMatchesSlotName>(key, scope))
return To<HTMLSlotElement>(slot); return To<HTMLSlotElement>(slot);
...@@ -193,10 +218,13 @@ HTMLSlotElement* TreeOrderedMap::GetSlotByName(const AtomicString& key, ...@@ -193,10 +218,13 @@ HTMLSlotElement* TreeOrderedMap::GetSlotByName(const AtomicString& key,
} }
Element* TreeOrderedMap::GetCachedFirstElementWithoutAccessingNodeTree( Element* TreeOrderedMap::GetCachedFirstElementWithoutAccessingNodeTree(
const AtomicString& key) { const StringView& key) {
MapEntry* entry = map_.at(key); Map::iterator it = map_.Find<StringViewLookupTranslator>(key);
if (!entry) if (it == map_.end())
return nullptr; return nullptr;
MapEntry* entry = it->value;
DCHECK(entry);
DCHECK(entry->count); DCHECK(entry->count);
return entry->element; return entry->element;
} }
......
...@@ -51,19 +51,19 @@ class TreeOrderedMap : public GarbageCollected<TreeOrderedMap> { ...@@ -51,19 +51,19 @@ class TreeOrderedMap : public GarbageCollected<TreeOrderedMap> {
TreeOrderedMap(); TreeOrderedMap();
void Add(const AtomicString&, Element&); void Add(const AtomicString&, Element&);
void Remove(const AtomicString&, Element&); void Remove(const StringView&, Element&);
bool Contains(const AtomicString&) const; bool Contains(const AtomicString&) const;
bool ContainsMultiple(const AtomicString&) const; bool ContainsMultiple(const AtomicString&) const;
// concrete instantiations of the get<>() method template // concrete instantiations of the get<>() method template
Element* GetElementById(const AtomicString&, const TreeScope&) const; Element* GetElementById(const StringView&, const TreeScope&) const;
const HeapVector<Member<Element>>& GetAllElementsById(const AtomicString&, const HeapVector<Member<Element>>& GetAllElementsById(const StringView&,
const TreeScope&) const; const TreeScope&) const;
Element* GetElementByMapName(const AtomicString&, const TreeScope&) const; Element* GetElementByMapName(const StringView&, const TreeScope&) const;
HTMLSlotElement* GetSlotByName(const AtomicString&, const TreeScope&) const; HTMLSlotElement* GetSlotByName(const StringView&, const TreeScope&) const;
// Don't use this unless the caller can know the internal state of // Don't use this unless the caller can know the internal state of
// TreeOrderedMap exactly. // TreeOrderedMap exactly.
Element* GetCachedFirstElementWithoutAccessingNodeTree(const AtomicString&); Element* GetCachedFirstElementWithoutAccessingNodeTree(const StringView&);
void Trace(Visitor*) const; void Trace(Visitor*) const;
...@@ -91,8 +91,8 @@ class TreeOrderedMap : public GarbageCollected<TreeOrderedMap> { ...@@ -91,8 +91,8 @@ class TreeOrderedMap : public GarbageCollected<TreeOrderedMap> {
#endif #endif
private: private:
template <bool keyMatches(const AtomicString&, const Element&)> template <bool keyMatches(const StringView&, const Element&)>
Element* Get(const AtomicString&, const TreeScope&) const; Element* Get(const StringView&, const TreeScope&) const;
class MapEntry : public GarbageCollected<MapEntry> { class MapEntry : public GarbageCollected<MapEntry> {
public: public:
......
...@@ -109,7 +109,7 @@ void TreeScope::ClearScopedStyleResolver() { ...@@ -109,7 +109,7 @@ void TreeScope::ClearScopedStyleResolver() {
scoped_style_resolver_.Clear(); scoped_style_resolver_.Clear();
} }
Element* TreeScope::getElementById(const AtomicString& element_id) const { Element* TreeScope::getElementById(const StringView& element_id) const {
if (element_id.IsEmpty()) if (element_id.IsEmpty())
return nullptr; return nullptr;
if (!elements_by_id_) if (!elements_by_id_)
...@@ -382,7 +382,7 @@ DOMSelection* TreeScope::GetSelection() const { ...@@ -382,7 +382,7 @@ DOMSelection* TreeScope::GetSelection() const {
Element* TreeScope::FindAnchor(const String& name) { Element* TreeScope::FindAnchor(const String& name) {
if (name.IsEmpty()) if (name.IsEmpty())
return nullptr; return nullptr;
if (Element* element = getElementById(AtomicString(name))) if (Element* element = getElementById(name))
return element; return element;
for (HTMLAnchorElement& anchor : for (HTMLAnchorElement& anchor :
Traversal<HTMLAnchorElement>::StartsAfter(RootNode())) { Traversal<HTMLAnchorElement>::StartsAfter(RootNode())) {
......
...@@ -72,7 +72,7 @@ class CORE_EXPORT TreeScope : public GarbageCollectedMixin { ...@@ -72,7 +72,7 @@ class CORE_EXPORT TreeScope : public GarbageCollectedMixin {
// TODO(kochi): once this algorithm is named in the spec, rename the method // TODO(kochi): once this algorithm is named in the spec, rename the method
// name. // name.
Element* AdjustedElement(const Element&) const; Element* AdjustedElement(const Element&) const;
Element* getElementById(const AtomicString&) const; Element* getElementById(const StringView&) const;
const HeapVector<Member<Element>>& GetAllElementsById( const HeapVector<Member<Element>>& GetAllElementsById(
const AtomicString&) const; const AtomicString&) const;
bool HasElementWithId(const AtomicString& id) const; bool HasElementWithId(const AtomicString& id) const;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment