Commit 130931a9 authored by kochi's avatar kochi Committed by Commit bot

Avoid redundant hash calculations in CustomElementUpgradeSorter

Pointed out by tkent in https://codereview.chromium.org/2242743002/

The original code could do 2 hash calculations for
adding a new key-value pair to ParentChildMap
(for find() and add()), and rehashing the map
may involve copying the HeapHashSet<Member<Node>>
objects.

Change the value part of ParentChildMap from a HeapHashMap
object to its pointer, to avoid redundant hash calculation
and object copy.

This does not change any functionality of the class.

BUG=594918

Review-Url: https://codereview.chromium.org/2290543002
Cr-Commit-Position: refs/heads/master@{#415251}
parent df268d9d
...@@ -31,15 +31,16 @@ CustomElementUpgradeSorter::AddResult CustomElementUpgradeSorter::addToParentChi ...@@ -31,15 +31,16 @@ CustomElementUpgradeSorter::AddResult CustomElementUpgradeSorter::addToParentChi
Node* parent, Node* parent,
Node* child) Node* child)
{ {
ParentChildMap::iterator it = m_parentChildMap->find(parent); ParentChildMap::AddResult result = m_parentChildMap->add(parent, nullptr);
if (it != m_parentChildMap->end()) { if (!result.isNewEntry) {
it->value.add(child); result.storedValue->value->add(child);
// The entry for the parent exists; so must its parents. // The entry for the parent exists; so must its parents.
return kParentAlreadyExistsInMap; return kParentAlreadyExistsInMap;
} }
ParentChildMap::AddResult result =
m_parentChildMap->add(parent, HeapHashSet<Member<Node>>()); ChildSet* childSet = new ChildSet();
result.storedValue->value.add(child); childSet->add(child);
result.storedValue->value = childSet;
return kParentAddedToMap; return kParentAddedToMap;
} }
...@@ -86,10 +87,10 @@ void CustomElementUpgradeSorter::sorted( ...@@ -86,10 +87,10 @@ void CustomElementUpgradeSorter::sorted(
if (childrenIterator == m_parentChildMap->end()) if (childrenIterator == m_parentChildMap->end())
return; return;
ChildSet& children = childrenIterator->value; ChildSet* children = childrenIterator->value.get();
if (children.size() == 1) { if (children->size() == 1) {
visit(result, children, children.begin()); visit(result, *children, children->begin());
return; return;
} }
...@@ -99,19 +100,19 @@ void CustomElementUpgradeSorter::sorted( ...@@ -99,19 +100,19 @@ void CustomElementUpgradeSorter::sorted(
? toElement(parent)->authorShadowRoot() ? toElement(parent)->authorShadowRoot()
: nullptr; : nullptr;
if (shadowRoot) if (shadowRoot)
visit(result, children, children.find(shadowRoot)); visit(result, *children, children->find(shadowRoot));
for (Element* e = ElementTraversal::firstChild(*parent); for (Element* e = ElementTraversal::firstChild(*parent);
e && children.size() > 1; e && children->size() > 1;
e = ElementTraversal::nextSibling(*e)) { e = ElementTraversal::nextSibling(*e)) {
visit(result, children, children.find(e)); visit(result, *children, children->find(e));
} }
if (children.size() == 1) if (children->size() == 1)
visit(result, children, children.begin()); visit(result, *children, children->begin());
DCHECK(children.isEmpty()); DCHECK(children->isEmpty());
} }
} // namespace blink } // namespace blink
...@@ -32,7 +32,7 @@ public: ...@@ -32,7 +32,7 @@ public:
private: private:
using ChildSet = HeapHashSet<Member<Node>>; using ChildSet = HeapHashSet<Member<Node>>;
using ParentChildMap = HeapHashMap<Member<Node>, ChildSet>; using ParentChildMap = HeapHashMap<Member<Node>, Member<ChildSet>>;
enum AddResult { enum AddResult {
kParentAlreadyExistsInMap, kParentAlreadyExistsInMap,
......
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