From df1bb0ff49b0f52f61da8d19c46cdbfa4ef6cfeb Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 23 May 2023 11:25:07 +0200 Subject: [PATCH] LibWeb: Make HTMLCollection faster when it only cares about children Some of the live HTMLCollection only ever contain children of their root node. When we know that's the case, we can avoid doing a full subtree traversal of all descendants and only visit children. This cuts the ECMA262 spec loading time by over 10 seconds. :^) --- Userland/Libraries/LibWeb/DOM/Document.cpp | 22 ++++++++-------- Userland/Libraries/LibWeb/DOM/Element.cpp | 2 +- .../Libraries/LibWeb/DOM/HTMLCollection.cpp | 25 +++++++++++++------ .../Libraries/LibWeb/DOM/HTMLCollection.h | 10 ++++++-- Userland/Libraries/LibWeb/DOM/ParentNode.cpp | 18 ++++++------- .../Libraries/LibWeb/HTML/HTMLFormElement.cpp | 2 +- .../LibWeb/HTML/HTMLOptionsCollection.cpp | 2 +- .../LibWeb/HTML/HTMLTableElement.cpp | 4 +-- .../LibWeb/HTML/HTMLTableRowElement.cpp | 5 ++-- .../LibWeb/HTML/HTMLTableSectionElement.cpp | 5 ++-- 10 files changed, 54 insertions(+), 41 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index d2c5312f47f..79964e87232 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -1054,7 +1054,7 @@ void Document::set_hovered_node(Node* node) JS::NonnullGCPtr Document::get_elements_by_name(DeprecatedString const& name) { - return HTMLCollection::create(*this, [name](Element const& element) { + return HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [name](Element const& element) { return element.name() == name; }).release_value_but_fixme_should_propagate_errors(); } @@ -1065,7 +1065,7 @@ JS::NonnullGCPtr Document::get_elements_by_class_name(Deprecated for (auto& name : class_names.view().split_view(' ')) { list_of_class_names.append(FlyString::from_utf8(name).release_value_but_fixme_should_propagate_errors()); } - return HTMLCollection::create(*this, [list_of_class_names = move(list_of_class_names), quirks_mode = document().in_quirks_mode()](Element const& element) { + return HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [list_of_class_names = move(list_of_class_names), quirks_mode = document().in_quirks_mode()](Element const& element) { for (auto& name : list_of_class_names) { if (!element.has_class(name, quirks_mode ? CaseSensitivity::CaseInsensitive : CaseSensitivity::CaseSensitive)) return false; @@ -1078,7 +1078,7 @@ JS::NonnullGCPtr Document::get_elements_by_class_name(Deprecated JS::NonnullGCPtr Document::applets() { if (!m_applets) - m_applets = HTMLCollection::create(*this, [](auto&) { return false; }).release_value_but_fixme_should_propagate_errors(); + m_applets = HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](auto&) { return false; }).release_value_but_fixme_should_propagate_errors(); return *m_applets; } @@ -1086,7 +1086,7 @@ JS::NonnullGCPtr Document::applets() JS::NonnullGCPtr Document::anchors() { if (!m_anchors) { - m_anchors = HTMLCollection::create(*this, [](Element const& element) { + m_anchors = HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](Element const& element) { return is(element) && element.has_attribute(HTML::AttributeNames::name); }).release_value_but_fixme_should_propagate_errors(); } @@ -1097,7 +1097,7 @@ JS::NonnullGCPtr Document::anchors() JS::NonnullGCPtr Document::images() { if (!m_images) { - m_images = HTMLCollection::create(*this, [](Element const& element) { + m_images = HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](Element const& element) { return is(element); }).release_value_but_fixme_should_propagate_errors(); } @@ -1108,7 +1108,7 @@ JS::NonnullGCPtr Document::images() JS::NonnullGCPtr Document::embeds() { if (!m_embeds) { - m_embeds = HTMLCollection::create(*this, [](Element const& element) { + m_embeds = HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](Element const& element) { return is(element); }).release_value_but_fixme_should_propagate_errors(); } @@ -1125,7 +1125,7 @@ JS::NonnullGCPtr Document::plugins() JS::NonnullGCPtr Document::links() { if (!m_links) { - m_links = HTMLCollection::create(*this, [](Element const& element) { + m_links = HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](Element const& element) { return (is(element) || is(element)) && element.has_attribute(HTML::AttributeNames::href); }).release_value_but_fixme_should_propagate_errors(); } @@ -1136,7 +1136,7 @@ JS::NonnullGCPtr Document::links() JS::NonnullGCPtr Document::forms() { if (!m_forms) { - m_forms = HTMLCollection::create(*this, [](Element const& element) { + m_forms = HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](Element const& element) { return is(element); }).release_value_but_fixme_should_propagate_errors(); } @@ -1147,7 +1147,7 @@ JS::NonnullGCPtr Document::forms() JS::NonnullGCPtr Document::scripts() { if (!m_scripts) { - m_scripts = HTMLCollection::create(*this, [](Element const& element) { + m_scripts = HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](Element const& element) { return is(element); }).release_value_but_fixme_should_propagate_errors(); } @@ -1158,7 +1158,7 @@ JS::NonnullGCPtr Document::scripts() JS::NonnullGCPtr Document::all() { if (!m_all) { - m_all = HTMLCollection::create(*this, [](Element const&) { + m_all = HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](Element const&) { return true; }).release_value_but_fixme_should_propagate_errors(); } @@ -2092,7 +2092,7 @@ void Document::check_favicon_after_loading_link_resource() if (!head_element) return; - auto favicon_link_elements = HTMLCollection::create(*head_element, [](Element const& element) { + auto favicon_link_elements = HTMLCollection::create(*head_element, HTMLCollection::Scope::Descendants, [](Element const& element) { if (!is(element)) return false; diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index 7d5e38ee536..56b0e7d315c 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -631,7 +631,7 @@ JS::NonnullGCPtr Element::get_elements_by_class_name(DeprecatedF for (auto& name : class_names.view().split_view_if(Infra::is_ascii_whitespace)) { list_of_class_names.append(FlyString::from_utf8(name).release_value_but_fixme_should_propagate_errors()); } - return HTMLCollection::create(*this, [list_of_class_names = move(list_of_class_names), quirks_mode = document().in_quirks_mode()](Element const& element) { + return HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [list_of_class_names = move(list_of_class_names), quirks_mode = document().in_quirks_mode()](Element const& element) { for (auto& name : list_of_class_names) { if (!element.has_class(name, quirks_mode ? CaseSensitivity::CaseInsensitive : CaseSensitivity::CaseSensitive)) return false; diff --git a/Userland/Libraries/LibWeb/DOM/HTMLCollection.cpp b/Userland/Libraries/LibWeb/DOM/HTMLCollection.cpp index 106adb80b95..efa9a5855f5 100644 --- a/Userland/Libraries/LibWeb/DOM/HTMLCollection.cpp +++ b/Userland/Libraries/LibWeb/DOM/HTMLCollection.cpp @@ -13,15 +13,16 @@ namespace Web::DOM { -WebIDL::ExceptionOr> HTMLCollection::create(ParentNode& root, Function filter) +WebIDL::ExceptionOr> HTMLCollection::create(ParentNode& root, Scope scope, Function filter) { - return MUST_OR_THROW_OOM(root.heap().allocate(root.realm(), root, move(filter))); + return MUST_OR_THROW_OOM(root.heap().allocate(root.realm(), root, scope, move(filter))); } -HTMLCollection::HTMLCollection(ParentNode& root, Function filter) +HTMLCollection::HTMLCollection(ParentNode& root, Scope scope, Function filter) : LegacyPlatformObject(root.realm()) , m_root(root) , m_filter(move(filter)) + , m_scope(scope) { } @@ -44,11 +45,19 @@ void HTMLCollection::visit_edges(Cell::Visitor& visitor) JS::MarkedVector HTMLCollection::collect_matching_elements() const { JS::MarkedVector elements(m_root->heap()); - m_root->for_each_in_subtree_of_type([&](auto& element) { - if (m_filter(element)) - elements.append(const_cast(&element)); - return IterationDecision::Continue; - }); + if (m_scope == Scope::Descendants) { + m_root->for_each_in_subtree_of_type([&](auto& element) { + if (m_filter(element)) + elements.append(const_cast(&element)); + return IterationDecision::Continue; + }); + } else { + m_root->for_each_child_of_type([&](auto& element) { + if (m_filter(element)) + elements.append(const_cast(&element)); + return IterationDecision::Continue; + }); + } return elements; } diff --git a/Userland/Libraries/LibWeb/DOM/HTMLCollection.h b/Userland/Libraries/LibWeb/DOM/HTMLCollection.h index f10f253ced7..252ef75fe58 100644 --- a/Userland/Libraries/LibWeb/DOM/HTMLCollection.h +++ b/Userland/Libraries/LibWeb/DOM/HTMLCollection.h @@ -30,7 +30,11 @@ class HTMLCollection : public Bindings::LegacyPlatformObject { WEB_PLATFORM_OBJECT(HTMLCollection, Bindings::LegacyPlatformObject); public: - static WebIDL::ExceptionOr> create(ParentNode& root, Function filter); + enum class Scope { + Children, + Descendants, + }; + static WebIDL::ExceptionOr> create(ParentNode& root, Scope, Function filter); virtual ~HTMLCollection() override; @@ -46,7 +50,7 @@ public: virtual bool is_supported_property_index(u32) const override; protected: - HTMLCollection(ParentNode& root, Function filter); + HTMLCollection(ParentNode& root, Scope, Function filter); virtual JS::ThrowCompletionOr initialize(JS::Realm&) override; @@ -70,6 +74,8 @@ private: JS::NonnullGCPtr m_root; Function m_filter; + + Scope m_scope { Scope::Descendants }; }; } diff --git a/Userland/Libraries/LibWeb/DOM/ParentNode.cpp b/Userland/Libraries/LibWeb/DOM/ParentNode.cpp index f32a80e4df2..de799726fc8 100644 --- a/Userland/Libraries/LibWeb/DOM/ParentNode.cpp +++ b/Userland/Libraries/LibWeb/DOM/ParentNode.cpp @@ -113,8 +113,8 @@ JS::NonnullGCPtr ParentNode::children() { // The children getter steps are to return an HTMLCollection collection rooted at this matching only element children. if (!m_children) { - m_children = HTMLCollection::create(*this, [this](Element const& element) { - return is_parent_of(element); + m_children = HTMLCollection::create(*this, HTMLCollection::Scope::Children, [](Element const&) { + return true; }).release_value_but_fixme_should_propagate_errors(); } return *m_children; @@ -126,14 +126,14 @@ JS::NonnullGCPtr ParentNode::get_elements_by_tag_name(Deprecated { // 1. If qualifiedName is "*" (U+002A), return a HTMLCollection rooted at root, whose filter matches only descendant elements. if (qualified_name == "*") { - return HTMLCollection::create(*this, [](Element const&) { + return HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](Element const&) { return true; }).release_value_but_fixme_should_propagate_errors(); } // 2. Otherwise, if root’s node document is an HTML document, return a HTMLCollection rooted at root, whose filter matches the following descendant elements: if (root().document().document_type() == Document::Type::HTML) { - return HTMLCollection::create(*this, [qualified_name](Element const& element) { + return HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [qualified_name](Element const& element) { // - Whose namespace is the HTML namespace and whose qualified name is qualifiedName, in ASCII lowercase. if (element.namespace_() == Namespace::HTML) return element.qualified_name().to_lowercase() == qualified_name.to_lowercase(); @@ -144,7 +144,7 @@ JS::NonnullGCPtr ParentNode::get_elements_by_tag_name(Deprecated } // 3. Otherwise, return a HTMLCollection rooted at root, whose filter matches descendant elements whose qualified name is qualifiedName. - return HTMLCollection::create(*this, [qualified_name](Element const& element) { + return HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [qualified_name](Element const& element) { return element.qualified_name() == qualified_name; }).release_value_but_fixme_should_propagate_errors(); } @@ -160,27 +160,27 @@ JS::NonnullGCPtr ParentNode::get_elements_by_tag_name_ns(Depreca // 2. If both namespace and localName are "*" (U+002A), return a HTMLCollection rooted at root, whose filter matches descendant elements. if (namespace_ == "*" && local_name == "*") { - return HTMLCollection::create(*this, [](Element const&) { + return HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [](Element const&) { return true; }).release_value_but_fixme_should_propagate_errors(); } // 3. Otherwise, if namespace is "*" (U+002A), return a HTMLCollection rooted at root, whose filter matches descendant elements whose local name is localName. if (namespace_ == "*") { - return HTMLCollection::create(*this, [local_name](Element const& element) { + return HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [local_name](Element const& element) { return element.local_name() == local_name; }).release_value_but_fixme_should_propagate_errors(); } // 4. Otherwise, if localName is "*" (U+002A), return a HTMLCollection rooted at root, whose filter matches descendant elements whose namespace is namespace. if (local_name == "*") { - return HTMLCollection::create(*this, [namespace_](Element const& element) { + return HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [namespace_](Element const& element) { return element.namespace_() == namespace_; }).release_value_but_fixme_should_propagate_errors(); } // 5. Otherwise, return a HTMLCollection rooted at root, whose filter matches descendant elements whose namespace is namespace and local name is localName. - return HTMLCollection::create(*this, [namespace_, local_name](Element const& element) { + return HTMLCollection::create(*this, HTMLCollection::Scope::Descendants, [namespace_, local_name](Element const& element) { return element.namespace_() == namespace_ && element.local_name() == local_name; }).release_value_but_fixme_should_propagate_errors(); } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp index 1ebfe882062..aa81e2a7136 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.cpp @@ -244,7 +244,7 @@ static bool is_form_control(DOM::Element const& element) JS::NonnullGCPtr HTMLFormElement::elements() const { if (!m_elements) { - m_elements = DOM::HTMLCollection::create(const_cast(*this), [](Element const& element) { + m_elements = DOM::HTMLCollection::create(const_cast(*this), DOM::HTMLCollection::Scope::Descendants, [](Element const& element) { return is_form_control(element); }).release_value_but_fixme_should_propagate_errors(); } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLOptionsCollection.cpp b/Userland/Libraries/LibWeb/HTML/HTMLOptionsCollection.cpp index 013a26ffa0e..e3a2a10def9 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLOptionsCollection.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLOptionsCollection.cpp @@ -19,7 +19,7 @@ WebIDL::ExceptionOr> HTMLOptionsCollecti } HTMLOptionsCollection::HTMLOptionsCollection(DOM::ParentNode& root, Function filter) - : DOM::HTMLCollection(root, move(filter)) + : DOM::HTMLCollection(root, Scope::Descendants, move(filter)) { } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLTableElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLTableElement.cpp index 8f8a3281c30..41b20d8cb06 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLTableElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLTableElement.cpp @@ -264,7 +264,7 @@ JS::NonnullGCPtr HTMLTableElement::t_bodies() // The tBodies attribute must return an HTMLCollection rooted at the table node, // whose filter matches only tbody elements that are children of the table element. if (!m_t_bodies) { - m_t_bodies = DOM::HTMLCollection::create(*this, [](DOM::Element const& element) { + m_t_bodies = DOM::HTMLCollection::create(*this, DOM::HTMLCollection::Scope::Children, [](DOM::Element const& element) { return element.local_name() == TagNames::tbody; }).release_value_but_fixme_should_propagate_errors(); } @@ -307,7 +307,7 @@ JS::NonnullGCPtr HTMLTableElement::rows() // How do you sort HTMLCollection? if (!m_rows) { - m_rows = DOM::HTMLCollection::create(*this, [table_node](DOM::Element const& element) { + m_rows = DOM::HTMLCollection::create(*this, DOM::HTMLCollection::Scope::Descendants, [table_node](DOM::Element const& element) { // Only match TR elements which are: // * children of the table element // * children of the thead, tbody, or tfoot elements that are themselves children of the table element diff --git a/Userland/Libraries/LibWeb/HTML/HTMLTableRowElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLTableRowElement.cpp index efe5044f2c3..c09dfc7fa49 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLTableRowElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLTableRowElement.cpp @@ -42,9 +42,8 @@ JS::NonnullGCPtr HTMLTableRowElement::cells() const // The cells attribute must return an HTMLCollection rooted at this tr element, // whose filter matches only td and th elements that are children of the tr element. if (!m_cells) { - m_cells = DOM::HTMLCollection::create(const_cast(*this), [this](Element const& element) { - return element.parent() == this - && is(element); + m_cells = DOM::HTMLCollection::create(const_cast(*this), DOM::HTMLCollection::Scope::Children, [](Element const& element) { + return is(element); }).release_value_but_fixme_should_propagate_errors(); } return *m_cells; diff --git a/Userland/Libraries/LibWeb/HTML/HTMLTableSectionElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLTableSectionElement.cpp index 1a3c00ccfeb..4fd9f72758c 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLTableSectionElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLTableSectionElement.cpp @@ -41,9 +41,8 @@ JS::NonnullGCPtr HTMLTableSectionElement::rows() const // The rows attribute must return an HTMLCollection rooted at this element, // whose filter matches only tr elements that are children of this element. if (!m_rows) { - m_rows = DOM::HTMLCollection::create(const_cast(*this), [this](Element const& element) { - return element.parent() == this - && is(element); + m_rows = DOM::HTMLCollection::create(const_cast(*this), DOM::HTMLCollection::Scope::Children, [](Element const& element) { + return is(element); }).release_value_but_fixme_should_propagate_errors(); } return *m_rows;