Pārlūkot izejas kodu

LibWeb: Don't create mutation record node lists if nobody is interested

By deferring allocation of StaticNodeList objects until we know somebody
actually wants the MutationRecord, we avoid a *lot* of allocation work.

This shaves several seconds off of loading https://tc39.es/ecma262/

At least one other engine (WebKit) skips creating mutation records if
nobody is interested, so even if this is observable somehow, we would
at least match the behavior of a major engine.
Andreas Kling 2 gadi atpakaļ
vecāks
revīzija
80d6330a26

+ 1 - 3
Userland/Libraries/LibWeb/DOM/Attr.cpp

@@ -89,9 +89,7 @@ void Attr::set_value(DeprecatedString value)
 void Attr::handle_attribute_changes(Element& element, DeprecatedString const& old_value, DeprecatedString const& new_value)
 void Attr::handle_attribute_changes(Element& element, DeprecatedString const& old_value, DeprecatedString const& new_value)
 {
 {
     // 1. Queue a mutation record of "attributes" for element with attribute’s local name, attribute’s namespace, oldValue, « », « », null, and null.
     // 1. Queue a mutation record of "attributes" for element with attribute’s local name, attribute’s namespace, oldValue, « », « », null, and null.
-    auto added_node_list = StaticNodeList::create(realm(), {}).release_value_but_fixme_should_propagate_errors();
-    auto removed_node_list = StaticNodeList::create(realm(), {}).release_value_but_fixme_should_propagate_errors();
-    element.queue_mutation_record(MutationType::attributes, local_name(), namespace_uri(), old_value, added_node_list, removed_node_list, nullptr, nullptr);
+    element.queue_mutation_record(MutationType::attributes, local_name(), namespace_uri(), old_value, {}, {}, nullptr, nullptr);
 
 
     // 2. If element is custom, then enqueue a custom element callback reaction with element, callback name "attributeChangedCallback", and an argument list containing attribute’s local name, oldValue, newValue, and attribute’s namespace.
     // 2. If element is custom, then enqueue a custom element callback reaction with element, callback name "attributeChangedCallback", and an argument list containing attribute’s local name, oldValue, newValue, and attribute’s namespace.
     if (element.is_custom()) {
     if (element.is_custom()) {

+ 1 - 3
Userland/Libraries/LibWeb/DOM/CharacterData.cpp

@@ -71,9 +71,7 @@ WebIDL::ExceptionOr<void> CharacterData::replace_data(size_t offset, size_t coun
         count = length - offset;
         count = length - offset;
 
 
     // 4. Queue a mutation record of "characterData" for node with null, null, node’s data, « », « », null, and null.
     // 4. Queue a mutation record of "characterData" for node with null, null, node’s data, « », « », null, and null.
-    auto added_node_list = TRY(StaticNodeList::create(realm(), {}));
-    auto removed_node_list = TRY(StaticNodeList::create(realm(), {}));
-    queue_mutation_record(MutationType::characterData, {}, {}, m_data, added_node_list, removed_node_list, nullptr, nullptr);
+    queue_mutation_record(MutationType::characterData, {}, {}, m_data, {}, {}, nullptr, nullptr);
 
 
     // 5. Insert data into node’s data after offset code units.
     // 5. Insert data into node’s data after offset code units.
     // 6. Let delete offset be offset + data’s length.
     // 6. Let delete offset be offset + data’s length.

+ 17 - 22
Userland/Libraries/LibWeb/DOM/Node.cpp

@@ -1,5 +1,5 @@
 /*
 /*
- * Copyright (c) 2018-2022, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2018-2023, Andreas Kling <kling@serenityos.org>
  * Copyright (c) 2021-2022, Linus Groh <linusg@serenityos.org>
  * Copyright (c) 2021-2022, Linus Groh <linusg@serenityos.org>
  * Copyright (c) 2021, Luke Wilde <lukew@serenityos.org>
  * Copyright (c) 2021, Luke Wilde <lukew@serenityos.org>
  *
  *
@@ -405,9 +405,7 @@ void Node::insert_before(JS::NonnullGCPtr<Node> node, JS::GCPtr<Node> child, boo
 
 
         // 2. Queue a tree mutation record for node with « », nodes, null, and null.
         // 2. Queue a tree mutation record for node with « », nodes, null, and null.
         // NOTE: This step intentionally does not pay attention to the suppress observers flag.
         // NOTE: This step intentionally does not pay attention to the suppress observers flag.
-        auto added_node_list = StaticNodeList::create(realm(), {}).release_value_but_fixme_should_propagate_errors();
-        auto removed_node_list = StaticNodeList::create(realm(), nodes).release_value_but_fixme_should_propagate_errors();
-        node->queue_tree_mutation_record(added_node_list, removed_node_list, nullptr, nullptr);
+        node->queue_tree_mutation_record({}, nodes, nullptr, nullptr);
     }
     }
 
 
     // 5. If child is non-null, then:
     // 5. If child is non-null, then:
@@ -480,9 +478,7 @@ void Node::insert_before(JS::NonnullGCPtr<Node> node, JS::GCPtr<Node> child, boo
 
 
     // 8. If suppress observers flag is unset, then queue a tree mutation record for parent with nodes, « », previousSibling, and child.
     // 8. If suppress observers flag is unset, then queue a tree mutation record for parent with nodes, « », previousSibling, and child.
     if (!suppress_observers) {
     if (!suppress_observers) {
-        auto added_node_list = StaticNodeList::create(realm(), move(nodes)).release_value_but_fixme_should_propagate_errors();
-        auto removed_node_list = StaticNodeList::create(realm(), {}).release_value_but_fixme_should_propagate_errors();
-        queue_tree_mutation_record(added_node_list, removed_node_list, previous_sibling.ptr(), child.ptr());
+        queue_tree_mutation_record(move(nodes), {}, previous_sibling.ptr(), child.ptr());
     }
     }
 
 
     // 9. Run the children changed steps for parent.
     // 9. Run the children changed steps for parent.
@@ -650,11 +646,7 @@ void Node::remove(bool suppress_observers)
 
 
     // 20. If suppress observers flag is unset, then queue a tree mutation record for parent with « », « node », oldPreviousSibling, and oldNextSibling.
     // 20. If suppress observers flag is unset, then queue a tree mutation record for parent with « », « node », oldPreviousSibling, and oldNextSibling.
     if (!suppress_observers) {
     if (!suppress_observers) {
-        Vector<JS::Handle<Node>> removed_nodes;
-        removed_nodes.append(JS::make_handle(*this));
-        auto added_node_list = StaticNodeList::create(realm(), {}).release_value_but_fixme_should_propagate_errors();
-        auto removed_node_list = StaticNodeList::create(realm(), move(removed_nodes)).release_value_but_fixme_should_propagate_errors();
-        parent->queue_tree_mutation_record(added_node_list, removed_node_list, old_previous_sibling.ptr(), old_next_sibling.ptr());
+        parent->queue_tree_mutation_record({}, { *this }, old_previous_sibling.ptr(), old_next_sibling.ptr());
     }
     }
 
 
     // 21. Run the children changed steps for parent.
     // 21. Run the children changed steps for parent.
@@ -749,9 +741,7 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<Node>> Node::replace_child(JS::NonnullGCPtr
     insert_before(node, reference_child, true);
     insert_before(node, reference_child, true);
 
 
     // 14. Queue a tree mutation record for parent with nodes, removedNodes, previousSibling, and referenceChild.
     // 14. Queue a tree mutation record for parent with nodes, removedNodes, previousSibling, and referenceChild.
-    auto added_node_list = TRY(StaticNodeList::create(realm(), move(nodes)));
-    auto removed_node_list = TRY(StaticNodeList::create(realm(), move(removed_nodes)));
-    queue_tree_mutation_record(added_node_list, removed_node_list, previous_sibling.ptr(), reference_child.ptr());
+    queue_tree_mutation_record(move(nodes), move(removed_nodes), previous_sibling.ptr(), reference_child.ptr());
 
 
     // 15. Return child.
     // 15. Return child.
     return child;
     return child;
@@ -1220,9 +1210,7 @@ void Node::replace_all(JS::GCPtr<Node> node)
 
 
     // 7. If either addedNodes or removedNodes is not empty, then queue a tree mutation record for parent with addedNodes, removedNodes, null, and null.
     // 7. If either addedNodes or removedNodes is not empty, then queue a tree mutation record for parent with addedNodes, removedNodes, null, and null.
     if (!added_nodes.is_empty() || !removed_nodes.is_empty()) {
     if (!added_nodes.is_empty() || !removed_nodes.is_empty()) {
-        auto added_node_list = StaticNodeList::create(realm(), move(added_nodes)).release_value_but_fixme_should_propagate_errors();
-        auto removed_node_list = StaticNodeList::create(realm(), move(removed_nodes)).release_value_but_fixme_should_propagate_errors();
-        queue_tree_mutation_record(added_node_list, removed_node_list, nullptr, nullptr);
+        queue_tree_mutation_record(move(added_nodes), move(removed_nodes), nullptr, nullptr);
     }
     }
 }
 }
 
 
@@ -1430,7 +1418,7 @@ Painting::PaintableBox const* Node::paintable_box() const
 }
 }
 
 
 // https://dom.spec.whatwg.org/#queue-a-mutation-record
 // https://dom.spec.whatwg.org/#queue-a-mutation-record
-void Node::queue_mutation_record(FlyString const& type, DeprecatedString attribute_name, DeprecatedString attribute_namespace, DeprecatedString old_value, JS::NonnullGCPtr<NodeList> added_nodes, JS::NonnullGCPtr<NodeList> removed_nodes, Node* previous_sibling, Node* next_sibling) const
+void Node::queue_mutation_record(FlyString const& type, DeprecatedString attribute_name, DeprecatedString attribute_namespace, DeprecatedString old_value, Vector<JS::Handle<Node>> added_nodes, Vector<JS::Handle<Node>> removed_nodes, Node* previous_sibling, Node* next_sibling) const
 {
 {
     // NOTE: We defer garbage collection until the end of the scope, since we can't safely use MutationObserver* as a hashmap key otherwise.
     // NOTE: We defer garbage collection until the end of the scope, since we can't safely use MutationObserver* as a hashmap key otherwise.
     // FIXME: This is a total hack.
     // FIXME: This is a total hack.
@@ -1479,11 +1467,18 @@ void Node::queue_mutation_record(FlyString const& type, DeprecatedString attribu
         }
         }
     }
     }
 
 
+    // OPTIMIZATION: If there are no interested observers, bail without doing any more work.
+    if (interested_observers.is_empty())
+        return;
+
+    auto added_nodes_list = StaticNodeList::create(realm(), move(added_nodes)).release_value_but_fixme_should_propagate_errors();
+    auto removed_nodes_list = StaticNodeList::create(realm(), move(removed_nodes)).release_value_but_fixme_should_propagate_errors();
+
     // 4. For each observer → mappedOldValue of interestedObservers:
     // 4. For each observer → mappedOldValue of interestedObservers:
     for (auto& interested_observer : interested_observers) {
     for (auto& interested_observer : interested_observers) {
         // 1. Let record be a new MutationRecord object with its type set to type, target set to target, attributeName set to name, attributeNamespace set to namespace, oldValue set to mappedOldValue,
         // 1. Let record be a new MutationRecord object with its type set to type, target set to target, attributeName set to name, attributeNamespace set to namespace, oldValue set to mappedOldValue,
         //    addedNodes set to addedNodes, removedNodes set to removedNodes, previousSibling set to previousSibling, and nextSibling set to nextSibling.
         //    addedNodes set to addedNodes, removedNodes set to removedNodes, previousSibling set to previousSibling, and nextSibling set to nextSibling.
-        auto record = MutationRecord::create(realm(), type, *this, added_nodes, removed_nodes, previous_sibling, next_sibling, attribute_name, attribute_namespace, /* mappedOldValue */ interested_observer.value).release_value_but_fixme_should_propagate_errors();
+        auto record = MutationRecord::create(realm(), type, *this, added_nodes_list, removed_nodes_list, previous_sibling, next_sibling, attribute_name, attribute_namespace, /* mappedOldValue */ interested_observer.value).release_value_but_fixme_should_propagate_errors();
 
 
         // 2. Enqueue record to observer’s record queue.
         // 2. Enqueue record to observer’s record queue.
         interested_observer.key->enqueue_record({}, move(record));
         interested_observer.key->enqueue_record({}, move(record));
@@ -1494,10 +1489,10 @@ void Node::queue_mutation_record(FlyString const& type, DeprecatedString attribu
 }
 }
 
 
 // https://dom.spec.whatwg.org/#queue-a-tree-mutation-record
 // https://dom.spec.whatwg.org/#queue-a-tree-mutation-record
-void Node::queue_tree_mutation_record(JS::NonnullGCPtr<NodeList> added_nodes, JS::NonnullGCPtr<NodeList> removed_nodes, Node* previous_sibling, Node* next_sibling)
+void Node::queue_tree_mutation_record(Vector<JS::Handle<Node>> added_nodes, Vector<JS::Handle<Node>> removed_nodes, Node* previous_sibling, Node* next_sibling)
 {
 {
     // 1. Assert: either addedNodes or removedNodes is not empty.
     // 1. Assert: either addedNodes or removedNodes is not empty.
-    VERIFY(added_nodes->length() > 0 || removed_nodes->length() > 0);
+    VERIFY(added_nodes.size() > 0 || removed_nodes.size() > 0);
 
 
     // 2. Queue a mutation record of "childList" for target with null, null, null, addedNodes, removedNodes, previousSibling, and nextSibling.
     // 2. Queue a mutation record of "childList" for target with null, null, null, addedNodes, removedNodes, previousSibling, and nextSibling.
     queue_mutation_record(MutationType::childList, {}, {}, {}, move(added_nodes), move(removed_nodes), previous_sibling, next_sibling);
     queue_mutation_record(MutationType::childList, {}, {}, {}, move(added_nodes), move(removed_nodes), previous_sibling, next_sibling);

+ 3 - 3
Userland/Libraries/LibWeb/DOM/Node.h

@@ -1,5 +1,5 @@
 /*
 /*
- * Copyright (c) 2018-2022, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2018-2023, Andreas Kling <kling@serenityos.org>
  *
  *
  * SPDX-License-Identifier: BSD-2-Clause
  * SPDX-License-Identifier: BSD-2-Clause
  */
  */
@@ -242,7 +242,7 @@ public:
 
 
     void add_registered_observer(RegisteredObserver& registered_observer) { m_registered_observer_list.append(registered_observer); }
     void add_registered_observer(RegisteredObserver& registered_observer) { m_registered_observer_list.append(registered_observer); }
 
 
-    void queue_mutation_record(FlyString const& type, DeprecatedString attribute_name, DeprecatedString attribute_namespace, DeprecatedString old_value, JS::NonnullGCPtr<NodeList> added_nodes, JS::NonnullGCPtr<NodeList> removed_nodes, Node* previous_sibling, Node* next_sibling) const;
+    void queue_mutation_record(FlyString const& type, DeprecatedString attribute_name, DeprecatedString attribute_namespace, DeprecatedString old_value, Vector<JS::Handle<Node>> added_nodes, Vector<JS::Handle<Node>> removed_nodes, Node* previous_sibling, Node* next_sibling) const;
 
 
     // https://dom.spec.whatwg.org/#concept-shadow-including-inclusive-descendant
     // https://dom.spec.whatwg.org/#concept-shadow-including-inclusive-descendant
     template<typename Callback>
     template<typename Callback>
@@ -671,7 +671,7 @@ protected:
     ErrorOr<String> name_or_description(NameOrDescription, Document const&, HashTable<i32>&) const;
     ErrorOr<String> name_or_description(NameOrDescription, Document const&, HashTable<i32>&) const;
 
 
 private:
 private:
-    void queue_tree_mutation_record(JS::NonnullGCPtr<NodeList> added_nodes, JS::NonnullGCPtr<NodeList> removed_nodes, Node* previous_sibling, Node* next_sibling);
+    void queue_tree_mutation_record(Vector<JS::Handle<Node>> added_nodes, Vector<JS::Handle<Node>> removed_nodes, Node* previous_sibling, Node* next_sibling);
 
 
     void insert_before_impl(JS::NonnullGCPtr<Node>, JS::GCPtr<Node> child);
     void insert_before_impl(JS::NonnullGCPtr<Node>, JS::GCPtr<Node> child);
     void append_child_impl(JS::NonnullGCPtr<Node>);
     void append_child_impl(JS::NonnullGCPtr<Node>);