[WebCore] Do not lookup HashSet multiple times in PropertyCascade

https://bugs.webkit.org/show_bug.cgi?id=261722 rdar://115706784 Reviewed by Antti Koivisto. Let's not lookup the same HashSet / HashMap multiple times. And use HashSet<AtomString> instead of HashSet<String> since name is already atomized. * Source/WebCore/style/PropertyCascade.cpp: (WebCore::Style::PropertyCascade::set): * Source/WebCore/style/StyleBuilder.cpp: (WebCore::Style::Builder::applyCustomProperties): (WebCore::Style::Builder::applyCustomProperty): (WebCore::Style::Builder::applyCustomPropertyImpl): (WebCore::Style::Builder::applyProperty): * Source/WebCore/style/StyleBuilder.h: * Source/WebCore/style/StyleBuilderState.h: Canonical link: https://commits.webkit.org/268114@main
WebKit · Sep 19, 2023 · 6442b8b · 6442b8b
1 parent 7fd0774
commit 6442b8b
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 21 deletions.
diff --git a/Source/WebCore/style/PropertyCascade.cpp b/Source/WebCore/style/PropertyCascade.cpp
@@ -114,26 +114,22 @@ void PropertyCascade::set(CSSPropertyID id, CSSValue& cssValue, const MatchedPro
     ASSERT(!CSSProperty::isInLogicalPropertyGroup(id));
     ASSERT(id < firstDeferredProperty);
 
-    auto& property = m_properties[id];
     ASSERT(id < m_propertyIsPresent.size());
     if (id == CSSPropertyCustom) {
         m_propertyIsPresent.set(id);
         const auto& customValue = downcast<CSSCustomPropertyValue>(cssValue);
-        bool hasValue = m_customProperties.contains(customValue.name());
-        if (!hasValue) {
+        auto result = m_customProperties.ensure(customValue.name(), [&]() {
             Property property;
-            property.id = id;
             property.cssValue = { };
             setPropertyInternal(property, id, cssValue, matchedProperties, cascadeLevel);
-            m_customProperties.set(customValue.name(), property);
-        } else {
-            Property property = customProperty(customValue.name());
-            setPropertyInternal(property, id, cssValue, matchedProperties, cascadeLevel);
-            m_customProperties.set(customValue.name(), property);
-        }
+            return property;
+        });
+        if (!result.isNewEntry)
+            setPropertyInternal(result.iterator->value, id, cssValue, matchedProperties, cascadeLevel);
         return;
     }
 
+    auto& property = m_properties[id];
     if (!m_propertyIsPresent[id])
         property.cssValue = { };
     m_propertyIsPresent.set(id);

diff --git a/Source/WebCore/style/StyleBuilder.cpp b/Source/WebCore/style/StyleBuilder.cpp
@@ -163,16 +163,27 @@ inline void Builder::applyPropertiesImpl(int firstProperty, int lastProperty)
 
 void Builder::applyCustomProperties()
 {
-    for (auto& name : m_cascade.customProperties().keys())
-        applyCustomProperty(name);
+    for (auto& [name, value] : m_cascade.customProperties()) {
+        if (m_state.m_appliedCustomProperties.contains(name))
+            continue;
+        applyCustomPropertyImpl(name, value);
+    }
 }
 
 void Builder::applyCustomProperty(const AtomString& name)
 {
-    if (m_state.m_appliedCustomProperties.contains(name) || !m_cascade.customProperties().contains(name))
+    if (m_state.m_appliedCustomProperties.contains(name))
         return;
 
-    auto& property = m_cascade.customProperty(name);
+    auto iterator = m_cascade.customProperties().find(name);
+    if (iterator == m_cascade.customProperties().end())
+        return;
+
+    applyCustomPropertyImpl(name, iterator->value);
+}
+
+void Builder::applyCustomPropertyImpl(const AtomString& name, const PropertyCascade::Property& property)
+{
     if (!property.cssValue[SelectorChecker::MatchDefault])
         return;
 
@@ -296,10 +307,12 @@ void Builder::applyProperty(CSSPropertyID id, CSSValue& value, SelectorChecker::
             // With the rollback cascade built, we need to obtain the property and apply it. If the property is
             // not present, then we behave like "unset." Otherwise we apply the property instead of our own.
             if (customPropertyValue) {
-                if (registeredCustomProperty && registeredCustomProperty->inherits && rollbackCascade->hasCustomProperty(customPropertyValue->name())) {
-                    auto property = rollbackCascade->customProperty(customPropertyValue->name());
-                    applyRollbackCascadeProperty(property, linkMatchMask);
-                    return;
+                if (registeredCustomProperty && registeredCustomProperty->inherits) {
+                    auto iterator = rollbackCascade->customProperties().find(customPropertyValue->name());
+                    if (iterator != rollbackCascade->customProperties().end()) {
+                        applyRollbackCascadeProperty(iterator->value, linkMatchMask);
+                        return;
+                    }
                 }
             } else if (id < firstDeferredProperty) {
                 if (rollbackCascade->hasNormalProperty(id)) {

diff --git a/Source/WebCore/style/StyleBuilder.h b/Source/WebCore/style/StyleBuilder.h
@@ -54,6 +54,7 @@ class Builder {
     void applyProperties(int firstProperty, int lastProperty);
     void applyDeferredProperties();
     void applyCustomProperties();
+    void applyCustomPropertyImpl(const AtomString&, const PropertyCascade::Property&);
 
     enum CustomPropertyCycleTracking { Enabled = 0, Disabled };
     template<CustomPropertyCycleTracking trackCycles>

diff --git a/Source/WebCore/style/StyleBuilderState.h b/Source/WebCore/style/StyleBuilderState.h
@@ -134,9 +134,9 @@ class BuilderState {
 
     const CSSToLengthConversionData m_cssToLengthConversionData;
 
-    HashSet<String> m_appliedCustomProperties;
-    HashSet<String> m_inProgressCustomProperties;
-    HashSet<String> m_inCycleCustomProperties;
+    HashSet<AtomString> m_appliedCustomProperties;
+    HashSet<AtomString> m_inProgressCustomProperties;
+    HashSet<AtomString> m_inCycleCustomProperties;
     WTF::BitSet<numCSSProperties> m_inProgressProperties;
     WTF::BitSet<numCSSProperties> m_inUnitCycleProperties;