From 44e4ea3d7aaf9aed6ad09cc9f049b2842e3e0d28 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 7 Sep 2024 11:45:50 +0200 Subject: [PATCH] LibWeb: Cache the qualified layer name in CSSRule This makes cascade layer filtering take <2% of CPU time when loading https://vercel.com instead of 30%. --- .../LibWeb/CSS/CSSLayerBlockRule.cpp | 3 +- .../LibWeb/CSS/CSSLayerStatementRule.cpp | 3 +- Userland/Libraries/LibWeb/CSS/CSSRule.cpp | 54 ++++++++++--------- Userland/Libraries/LibWeb/CSS/CSSRule.h | 4 +- 4 files changed, 33 insertions(+), 31 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/CSSLayerBlockRule.cpp b/Userland/Libraries/LibWeb/CSS/CSSLayerBlockRule.cpp index 475f91da24e..66265921398 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSLayerBlockRule.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSLayerBlockRule.cpp @@ -71,8 +71,7 @@ String CSSLayerBlockRule::serialized() const FlyString CSSLayerBlockRule::internal_qualified_name(Badge) const { - // TODO: Cache this? - auto parent_name = parent_layer_internal_qualified_name(); + auto const& parent_name = parent_layer_internal_qualified_name(); if (parent_name.is_empty()) return m_name_internal; return MUST(String::formatted("{}.{}", parent_name, m_name_internal)); diff --git a/Userland/Libraries/LibWeb/CSS/CSSLayerStatementRule.cpp b/Userland/Libraries/LibWeb/CSS/CSSLayerStatementRule.cpp index 82c53b268e9..cb9c1cda12f 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSLayerStatementRule.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSLayerStatementRule.cpp @@ -42,10 +42,9 @@ String CSSLayerStatementRule::serialized() const Vector CSSLayerStatementRule::internal_qualified_name_list(Badge) const { - // TODO: Cache these? Vector qualified_layer_names; - auto qualified_parent_layer_name = parent_layer_internal_qualified_name(); + auto const& qualified_parent_layer_name = parent_layer_internal_qualified_name(); if (qualified_parent_layer_name.is_empty()) return m_name_list; diff --git a/Userland/Libraries/LibWeb/CSS/CSSRule.cpp b/Userland/Libraries/LibWeb/CSS/CSSRule.cpp index 707f88195cb..16060d8629e 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSRule.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSRule.cpp @@ -48,37 +48,39 @@ void CSSRule::set_parent_style_sheet(CSSStyleSheet* parent_style_sheet) m_parent_style_sheet = parent_style_sheet; } -String CSSRule::parent_layer_internal_qualified_name() const +FlyString const& CSSRule::parent_layer_internal_qualified_name() const { - // TODO: Cache this? - Vector layer_names; - for (auto* rule = parent_rule(); rule; rule = rule->parent_rule()) { - switch (rule->type()) { - case CSSRule::Type::Import: - // TODO: Handle `layer(foo)` in import rules once we implement that. - break; + if (!m_cached_layer_name.has_value()) { + Vector layer_names; + for (auto* rule = parent_rule(); rule; rule = rule->parent_rule()) { + switch (rule->type()) { + case CSSRule::Type::Import: + // TODO: Handle `layer(foo)` in import rules once we implement that. + break; - case CSSRule::Type::LayerBlock: { - auto& layer_block = static_cast(*rule); - layer_names.append(layer_block.internal_name()); - break; + case CSSRule::Type::LayerBlock: { + auto& layer_block = static_cast(*rule); + layer_names.append(layer_block.internal_name()); + break; + } + + // Ignore everything else + // Note that LayerStatement cannot have child rules so we still ignore it here. + case CSSRule::Type::LayerStatement: + case CSSRule::Type::Style: + case CSSRule::Type::Media: + case CSSRule::Type::FontFace: + case CSSRule::Type::Keyframes: + case CSSRule::Type::Keyframe: + case CSSRule::Type::Namespace: + case CSSRule::Type::Supports: + break; + } } - // Ignore everything else - // Note that LayerStatement cannot have child rules so we still ignore it here. - case CSSRule::Type::LayerStatement: - case CSSRule::Type::Style: - case CSSRule::Type::Media: - case CSSRule::Type::FontFace: - case CSSRule::Type::Keyframes: - case CSSRule::Type::Keyframe: - case CSSRule::Type::Namespace: - case CSSRule::Type::Supports: - break; - } + m_cached_layer_name = MUST(String::join("."sv, layer_names.in_reverse())); } - - return MUST(String::join("."sv, layer_names.in_reverse())); + return m_cached_layer_name.value(); } } diff --git a/Userland/Libraries/LibWeb/CSS/CSSRule.h b/Userland/Libraries/LibWeb/CSS/CSSRule.h index 32fab795eb0..dcac9ce8689 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSRule.h +++ b/Userland/Libraries/LibWeb/CSS/CSSRule.h @@ -58,10 +58,12 @@ protected: virtual void visit_edges(Cell::Visitor&) override; - String parent_layer_internal_qualified_name() const; + FlyString const& parent_layer_internal_qualified_name() const; JS::GCPtr m_parent_rule; JS::GCPtr m_parent_style_sheet; + + mutable Optional m_cached_layer_name; }; }