From 205208db11f93eb54ed8db22669676dab7551369 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 3 Mar 2022 13:40:06 +0100 Subject: [PATCH] LibWeb: Reduce HashMap thrashing during custom property cascade Build the final custom property map right away instead of first making a temporary pointer-only map. We also precompute the final needed capacity for the map to avoid incremental rehashing. --- .../Libraries/LibWeb/CSS/StyleComputer.cpp | 35 ++++++++++--------- Userland/Libraries/LibWeb/CSS/StyleComputer.h | 6 ++-- Userland/Libraries/LibWeb/DOM/Element.h | 7 +--- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index ac5ca7d1c3..adebde9d62 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -439,7 +439,7 @@ static void set_property_expanding_shorthands(StyleProperties& style, CSS::Prope style.set_property(property_id, value); } -bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView property_name, HashMap>& dependencies, Vector const& source, Vector& dest, size_t source_start_index, HashMap const& custom_properties) const +bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView property_name, HashMap>& dependencies, Vector const& source, Vector& dest, size_t source_start_index, HashMap const& custom_properties) const { // FIXME: Do this better! // We build a copy of the tree of StyleComponentValueRules, with all var()s replaced with their contents. @@ -456,7 +456,7 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p auto get_custom_property = [&custom_properties](auto& name) -> RefPtr { auto it = custom_properties.find(name); if (it != custom_properties.end()) - return it->value->value; + return it->value.value; return nullptr; }; @@ -532,7 +532,7 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p return true; } -RefPtr StyleComputer::resolve_unresolved_style_value(DOM::Element& element, PropertyID property_id, UnresolvedStyleValue const& unresolved, HashMap const& custom_properties) const +RefPtr StyleComputer::resolve_unresolved_style_value(DOM::Element& element, PropertyID property_id, UnresolvedStyleValue const& unresolved, HashMap const& custom_properties) const { // Unresolved always contains a var(), unless it is a custom property's value, in which case we shouldn't be trying // to produce a different StyleValue from it. @@ -549,7 +549,7 @@ RefPtr StyleComputer::resolve_unresolved_style_value(DOM::Element& e return {}; } -void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& element, Vector const& matching_rules, CascadeOrigin cascade_origin, Important important, HashMap const& custom_properties) const +void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& element, Vector const& matching_rules, CascadeOrigin cascade_origin, Important important, HashMap const& custom_properties) const { for (auto const& match : matching_rules) { for (auto const& property : verify_cast(match.rule->declaration()).properties()) { @@ -575,26 +575,29 @@ void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& e } } -static HashMap cascade_custom_properties(DOM::Element& element, Vector const& matching_rules) +static HashMap const& cascade_custom_properties(DOM::Element& element, Vector const& matching_rules) { - HashMap custom_properties; + size_t needed_capacity = 0; + if (auto* parent_element = element.parent_element()) + needed_capacity += parent_element->custom_properties().size(); + for (auto const& matching_rule : matching_rules) + needed_capacity += verify_cast(matching_rule.rule->declaration()).custom_properties().size(); + + HashMap custom_properties; + custom_properties.ensure_capacity(needed_capacity); if (auto* parent_element = element.parent_element()) { for (auto const& it : parent_element->custom_properties()) - custom_properties.set(it.key, &it.value); + custom_properties.set(it.key, it.value); } for (auto const& matching_rule : matching_rules) { - for (auto const& it : verify_cast(matching_rule.rule->declaration()).custom_properties()) { - custom_properties.set(it.key, &it.value); - } + for (auto const& it : verify_cast(matching_rule.rule->declaration()).custom_properties()) + custom_properties.set(it.key, it.value); } - element.custom_properties().clear(); - for (auto& it : custom_properties) - element.add_custom_property(it.key, *it.value); - - return custom_properties; + element.set_custom_properties(move(custom_properties)); + return element.custom_properties(); } // https://www.w3.org/TR/css-cascade/#cascading @@ -608,7 +611,7 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element sort_matching_rules(matching_rule_set.author_rules); // Then we resolve all the CSS custom properties ("variables") for this element: - auto custom_properties = cascade_custom_properties(element, matching_rule_set.author_rules); + auto const& custom_properties = cascade_custom_properties(element, matching_rule_set.author_rules); // Then we apply the declarations from the matched rules in cascade order: diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.h b/Userland/Libraries/LibWeb/CSS/StyleComputer.h index 11714844db..7383269fed 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.h +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.h @@ -78,8 +78,8 @@ private: void compute_defaulted_property_value(StyleProperties&, DOM::Element const*, CSS::PropertyID, Optional) const; - RefPtr resolve_unresolved_style_value(DOM::Element&, PropertyID, UnresolvedStyleValue const&, HashMap const&) const; - bool expand_unresolved_values(DOM::Element&, StringView property_name, HashMap>& dependencies, Vector const& source, Vector& dest, size_t source_start_index, HashMap const& custom_properties) const; + RefPtr resolve_unresolved_style_value(DOM::Element&, PropertyID, UnresolvedStyleValue const&, HashMap const&) const; + bool expand_unresolved_values(DOM::Element&, StringView property_name, HashMap>& dependencies, Vector const& source, Vector& dest, size_t source_start_index, HashMap const& custom_properties) const; template void for_each_stylesheet(CascadeOrigin, Callback) const; @@ -89,7 +89,7 @@ private: Vector author_rules; }; - void cascade_declarations(StyleProperties&, DOM::Element&, Vector const&, CascadeOrigin, Important important, HashMap const&) const; + void cascade_declarations(StyleProperties&, DOM::Element&, Vector const&, CascadeOrigin, Important important, HashMap const&) const; void build_rule_cache(); void build_rule_cache_if_needed() const; diff --git a/Userland/Libraries/LibWeb/DOM/Element.h b/Userland/Libraries/LibWeb/DOM/Element.h index e1bdb47ab6..94cc4d7cf8 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.h +++ b/Userland/Libraries/LibWeb/DOM/Element.h @@ -113,13 +113,8 @@ public: const ShadowRoot* shadow_root() const { return m_shadow_root; } void set_shadow_root(RefPtr); - void add_custom_property(String custom_property_name, CSS::StyleProperty style_property) - { - m_custom_properties.set(move(custom_property_name), move(style_property)); - } - + void set_custom_properties(HashMap custom_properties) { m_custom_properties = move(custom_properties); } HashMap const& custom_properties() const { return m_custom_properties; } - HashMap& custom_properties() { return m_custom_properties; } void queue_an_element_task(HTML::Task::Source, Function);