From 9edfd5e360e5bf09817ef33765d7247b2f5a782d Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 18 Nov 2023 11:22:51 +0100 Subject: [PATCH] LibWeb: Only allocate DOM::Node registered observer list on demand Most DOM nodes don't have registered mutation observers, so let's put the metadata about them behind an OwnPtr to save space in the common case. Saves 16 bytes per DOM node that doesn't have registered observers. --- .../LibWeb/Bindings/MainThreadVM.cpp | 8 ++-- .../Libraries/LibWeb/DOM/MutationObserver.cpp | 46 +++++++++++-------- Userland/Libraries/LibWeb/DOM/Node.cpp | 23 ++++++++-- Userland/Libraries/LibWeb/DOM/Node.h | 8 ++-- 4 files changed, 53 insertions(+), 32 deletions(-) diff --git a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp index 9890796c32..82d9ab9257 100644 --- a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp +++ b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp @@ -608,9 +608,11 @@ void queue_mutation_observer_microtask(DOM::Document const& document) if (node.is_null()) continue; - node->registered_observers_list().remove_all_matching([&mutation_observer](DOM::RegisteredObserver& registered_observer) { - return is(registered_observer) && static_cast(registered_observer).observer().ptr() == mutation_observer.ptr(); - }); + if (node->registered_observer_list()) { + node->registered_observer_list()->remove_all_matching([&mutation_observer](DOM::RegisteredObserver& registered_observer) { + return is(registered_observer) && static_cast(registered_observer).observer().ptr() == mutation_observer.ptr(); + }); + } } // 4. If records is not empty, then invoke mo’s callback with « records, mo », and mo. If this throws an exception, catch it, and report the exception. diff --git a/Userland/Libraries/LibWeb/DOM/MutationObserver.cpp b/Userland/Libraries/LibWeb/DOM/MutationObserver.cpp index d6cf1927a0..c964fa53ab 100644 --- a/Userland/Libraries/LibWeb/DOM/MutationObserver.cpp +++ b/Userland/Libraries/LibWeb/DOM/MutationObserver.cpp @@ -85,26 +85,30 @@ WebIDL::ExceptionOr MutationObserver::observe(Node& target, MutationObserv // 7. For each registered of target’s registered observer list, if registered’s observer is this: bool updated_existing_observer = false; - for (auto& registered_observer : target.registered_observers_list()) { - if (registered_observer->observer().ptr() != this) - continue; - - updated_existing_observer = true; - - // 1. For each node of this’s node list, remove all transient registered observers whose source is registered from node’s registered observer list. - for (auto& node : m_node_list) { - // FIXME: Is this correct? - if (node.is_null()) + if (target.registered_observer_list()) { + for (auto& registered_observer : *target.registered_observer_list()) { + if (registered_observer->observer().ptr() != this) continue; - node->registered_observers_list().remove_all_matching([®istered_observer](RegisteredObserver& observer) { - return is(observer) && verify_cast(observer).source().ptr() == registered_observer; - }); - } + updated_existing_observer = true; - // 2. Set registered’s options to options. - registered_observer->set_options(options); - break; + // 1. For each node of this’s node list, remove all transient registered observers whose source is registered from node’s registered observer list. + for (auto& node : m_node_list) { + // FIXME: Is this correct? + if (node.is_null()) + continue; + + if (node->registered_observer_list()) { + node->registered_observer_list()->remove_all_matching([®istered_observer](RegisteredObserver& observer) { + return is(observer) && verify_cast(observer).source().ptr() == registered_observer; + }); + } + } + + // 2. Set registered’s options to options. + registered_observer->set_options(options); + break; + } } // 8. Otherwise: @@ -129,9 +133,11 @@ void MutationObserver::disconnect() if (node.is_null()) continue; - node->registered_observers_list().remove_all_matching([this](RegisteredObserver& registered_observer) { - return registered_observer.observer().ptr() == this; - }); + if (node->registered_observer_list()) { + node->registered_observer_list()->remove_all_matching([this](RegisteredObserver& registered_observer) { + return registered_observer.observer().ptr() == this; + }); + } } // 2. Empty this’s record queue. diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index f2616c4bc0..b43506c52d 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -103,8 +103,10 @@ void Node::visit_edges(Cell::Visitor& visitor) visitor.visit(m_layout_node); visitor.visit(m_paintable); - for (auto& registered_observer : m_registered_observer_list) - visitor.visit(registered_observer); + if (m_registered_observer_list) { + for (auto& registered_observer : *m_registered_observer_list) + visitor.visit(registered_observer); + } } // https://dom.spec.whatwg.org/#dom-node-baseuri @@ -686,10 +688,12 @@ void Node::remove(bool suppress_observers) // if registered’s options["subtree"] is true, then append a new transient registered observer // whose observer is registered’s observer, options is registered’s options, and source is registered to node’s registered observer list. for (auto* inclusive_ancestor = parent; inclusive_ancestor; inclusive_ancestor = inclusive_ancestor->parent()) { - for (auto& registered : inclusive_ancestor->m_registered_observer_list) { + if (!inclusive_ancestor->m_registered_observer_list) + continue; + for (auto& registered : *inclusive_ancestor->m_registered_observer_list) { if (registered->options().subtree) { auto transient_observer = TransientRegisteredObserver::create(registered->observer(), registered->options(), registered); - m_registered_observer_list.append(move(transient_observer)); + m_registered_observer_list->append(move(transient_observer)); } } } @@ -1546,7 +1550,9 @@ void Node::queue_mutation_record(FlyString const& type, Optional cons // 2. Let nodes be the inclusive ancestors of target. // 3. For each node in nodes, and then for each registered of node’s registered observer list: for (auto* node = this; node; node = node->parent()) { - for (auto& registered_observer : node->m_registered_observer_list) { + if (!node->m_registered_observer_list) + continue; + for (auto& registered_observer : *node->m_registered_observer_list) { // 1. Let options be registered’s options. auto& options = registered_observer->options(); @@ -1995,4 +2001,11 @@ ErrorOr Node::prepend_with_space(StringBuilder x, StringView const& result return {}; } +void Node::add_registered_observer(RegisteredObserver& registered_observer) +{ + if (!m_registered_observer_list) + m_registered_observer_list = make>>(); + m_registered_observer_list->append(registered_observer); +} + } diff --git a/Userland/Libraries/LibWeb/DOM/Node.h b/Userland/Libraries/LibWeb/DOM/Node.h index 96ec472a8a..08081a6ae2 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.h +++ b/Userland/Libraries/LibWeb/DOM/Node.h @@ -254,10 +254,10 @@ public: size_t length() const; - auto& registered_observers_list() { return m_registered_observer_list; } - auto const& registered_observers_list() const { return m_registered_observer_list; } + auto& registered_observer_list() { return m_registered_observer_list; } + auto const& registered_observer_list() const { return m_registered_observer_list; } - void add_registered_observer(RegisteredObserver& registered_observer) { m_registered_observer_list.append(registered_observer); } + void add_registered_observer(RegisteredObserver&); void queue_mutation_record(FlyString const& type, Optional const& attribute_name, Optional const& attribute_namespace, Optional const& old_value, Vector> added_nodes, Vector> removed_nodes, Node* previous_sibling, Node* next_sibling) const; @@ -693,7 +693,7 @@ protected: // https://dom.spec.whatwg.org/#registered-observer-list // "Nodes have a strong reference to registered observers in their registered observer list." https://dom.spec.whatwg.org/#garbage-collection - Vector> m_registered_observer_list; + OwnPtr>> m_registered_observer_list; void build_accessibility_tree(AccessibilityTreeNode& parent);