Commit eaddcb28 authored by Rakina Zata Amni's avatar Rakina Zata Amni Committed by Commit Bot

Add assigned_nodes_index_ map for slot's assigned child

Currently when traversing to the next/prev sibling of a slot's assigned
child, we find the current node's position by looping through a list
of the slot's assigned child, taking O(N) time where N is the number of
assigned children in that slot. This means traversing through a slot's
children will take a total of O(N^2) time. This CL adds a node->index
map for slot's children so that we will instead take O(1) time to find
a node's position in the list.

The downside of this is in cases where there are only a small number
of children assigned to a slot, the performance will be slightly worse
because of the hashmap lookup.

Benchmarked with https://jsbin.com/ravolid/edit?html,output, comparing
shadow dom v0 vs v1, where v0 already uses maps.

For N = 500 (release build), v0 takes ~20ms, v1 without map takes ~200ms, so v0 is 10x
faster than v1.
For N = 500 (local build), v0 takes ~54ms, v1 with map takes ~90ms, so v0 is 1.8x
faster than v1.


Bug: 901063
Change-Id: Iec006fcafca1e368ab97f40d909362dbcf74a08e
Reviewed-on: https://chromium-review.googlesource.com/c/1328622Reviewed-by: default avatarHayato Ito <hayato@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606764}
parent d630de09
...@@ -184,15 +184,17 @@ void HTMLSlotElement::assign(HeapVector<Member<Node>> nodes) { ...@@ -184,15 +184,17 @@ void HTMLSlotElement::assign(HeapVector<Member<Node>> nodes) {
void HTMLSlotElement::AppendAssignedNode(Node& host_child) { void HTMLSlotElement::AppendAssignedNode(Node& host_child) {
DCHECK(host_child.IsSlotable()); DCHECK(host_child.IsSlotable());
assigned_nodes_index_.insert(&host_child, assigned_nodes_.size());
assigned_nodes_.push_back(&host_child); assigned_nodes_.push_back(&host_child);
} }
void HTMLSlotElement::ClearAssignedNodes() { void HTMLSlotElement::ClearAssignedNodes() {
assigned_nodes_.clear(); assigned_nodes_.clear();
assigned_nodes_index_.clear();
} }
void HTMLSlotElement::ClearAssignedNodesAndFlatTreeChildren() { void HTMLSlotElement::ClearAssignedNodesAndFlatTreeChildren() {
assigned_nodes_.clear(); ClearAssignedNodes();
flat_tree_children_.clear(); flat_tree_children_.clear();
} }
...@@ -223,9 +225,9 @@ void HTMLSlotElement::DispatchSlotChangeEvent() { ...@@ -223,9 +225,9 @@ void HTMLSlotElement::DispatchSlotChangeEvent() {
Node* HTMLSlotElement::AssignedNodeNextTo(const Node& node) const { Node* HTMLSlotElement::AssignedNodeNextTo(const Node& node) const {
DCHECK(SupportsAssignment()); DCHECK(SupportsAssignment());
ContainingShadowRoot()->GetSlotAssignment().RecalcAssignment(); ContainingShadowRoot()->GetSlotAssignment().RecalcAssignment();
// TODO(hayato): Use {node -> index} map to avoid O(N) linear search auto it = assigned_nodes_index_.find(&node);
wtf_size_t index = assigned_nodes_.Find(&node); DCHECK(it != assigned_nodes_index_.end());
DCHECK(index != WTF::kNotFound); unsigned index = it->value;
if (index + 1 == assigned_nodes_.size()) if (index + 1 == assigned_nodes_.size())
return nullptr; return nullptr;
return assigned_nodes_[index + 1].Get(); return assigned_nodes_[index + 1].Get();
...@@ -234,9 +236,9 @@ Node* HTMLSlotElement::AssignedNodeNextTo(const Node& node) const { ...@@ -234,9 +236,9 @@ Node* HTMLSlotElement::AssignedNodeNextTo(const Node& node) const {
Node* HTMLSlotElement::AssignedNodePreviousTo(const Node& node) const { Node* HTMLSlotElement::AssignedNodePreviousTo(const Node& node) const {
DCHECK(SupportsAssignment()); DCHECK(SupportsAssignment());
ContainingShadowRoot()->GetSlotAssignment().RecalcAssignment(); ContainingShadowRoot()->GetSlotAssignment().RecalcAssignment();
// TODO(hayato): Use {node -> index} map to avoid O(N) linear search auto it = assigned_nodes_index_.find(&node);
wtf_size_t index = assigned_nodes_.Find(&node); DCHECK(it != assigned_nodes_index_.end());
DCHECK(index != WTF::kNotFound); unsigned index = it->value;
if (index == 0) if (index == 0)
return nullptr; return nullptr;
return assigned_nodes_[index - 1].Get(); return assigned_nodes_[index - 1].Get();
...@@ -554,6 +556,7 @@ int HTMLSlotElement::tabIndex() const { ...@@ -554,6 +556,7 @@ int HTMLSlotElement::tabIndex() const {
void HTMLSlotElement::Trace(blink::Visitor* visitor) { void HTMLSlotElement::Trace(blink::Visitor* visitor) {
visitor->Trace(assigned_nodes_); visitor->Trace(assigned_nodes_);
visitor->Trace(assigned_nodes_index_);
visitor->Trace(flat_tree_children_); visitor->Trace(flat_tree_children_);
visitor->Trace(assigned_nodes_candidates_); visitor->Trace(assigned_nodes_candidates_);
HTMLElement::Trace(visitor); HTMLElement::Trace(visitor);
......
...@@ -137,6 +137,7 @@ class CORE_EXPORT HTMLSlotElement final : public HTMLElement { ...@@ -137,6 +137,7 @@ class CORE_EXPORT HTMLSlotElement final : public HTMLElement {
void ClearAssignedNodesAndFlatTreeChildren(); void ClearAssignedNodesAndFlatTreeChildren();
HeapVector<Member<Node>> assigned_nodes_; HeapVector<Member<Node>> assigned_nodes_;
HeapHashMap<Member<const Node>, unsigned> assigned_nodes_index_;
HeapVector<Member<Node>> flat_tree_children_; HeapVector<Member<Node>> flat_tree_children_;
bool slotchange_event_enqueued_ = false; bool slotchange_event_enqueued_ = false;
......
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