Title: [266717] trunk/Source/WebCore
- Revision
- 266717
- Author
- [email protected]
- Date
- 2020-09-08 01:48:38 -0700 (Tue, 08 Sep 2020)
Log Message
Comparing styles with large but identical custom property maps is slow
https://bugs.webkit.org/show_bug.cgi?id=216241
<rdar://problem/67946605>
Reviewed by Darin Adler.
In a full style rebuild we lose sharing of equal StyleCustomPropertyDatas between the old and the new style.
Custom properties are usually defined in document element and this lack of sharing inherits to all elements.
Without sharing equality comparisons end up requiring deep comparisons, which can be slow if there are thousands of properties.
Fix by deduplicating identical property maps between the old the new style. All the descendants that inherit the properties will
now be cheap to compare.
The patch also contains some other basic custom property optimizations.
Going to full screen and back on youtube, this patch reduces time in the longest style update from ~460ms to ~90ms.
* css/CSSCustomPropertyValue.cpp:
(WebCore::CSSCustomPropertyValue::equals const):
Check for pointer equality first.
* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::deduplicateInheritedCustomProperties):
Deduplicate if the properties are equal but not shared.
(WebCore::RenderStyle::setInheritedCustomPropertyValue):
(WebCore::RenderStyle::setNonInheritedCustomPropertyValue):
Check if the value is already in the map before triggering copy-on-write.
* rendering/style/RenderStyle.h:
(WebCore::RenderStyle::setInheritedCustomPropertyValue): Deleted.
(WebCore::RenderStyle::setNonInheritedCustomPropertyValue): Deleted.
* style/StyleBuilder.cpp:
(WebCore::Style::Builder::applyCustomProperty):
Don't create unnecesary copies of CSSCustomPropertyValue.
* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::createAnimatedElementUpdate):
Try to deduplicate before comparing the styles.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (266716 => 266717)
--- trunk/Source/WebCore/ChangeLog 2020-09-08 07:35:49 UTC (rev 266716)
+++ trunk/Source/WebCore/ChangeLog 2020-09-08 08:48:38 UTC (rev 266717)
@@ -1,3 +1,50 @@
+2020-09-08 Antti Koivisto <[email protected]>
+
+ Comparing styles with large but identical custom property maps is slow
+ https://bugs.webkit.org/show_bug.cgi?id=216241
+ <rdar://problem/67946605>
+
+ Reviewed by Darin Adler.
+
+ In a full style rebuild we lose sharing of equal StyleCustomPropertyDatas between the old and the new style.
+ Custom properties are usually defined in document element and this lack of sharing inherits to all elements.
+ Without sharing equality comparisons end up requiring deep comparisons, which can be slow if there are thousands of properties.
+
+ Fix by deduplicating identical property maps between the old the new style. All the descendants that inherit the properties will
+ now be cheap to compare.
+
+ The patch also contains some other basic custom property optimizations.
+
+ Going to full screen and back on youtube, this patch reduces time in the longest style update from ~460ms to ~90ms.
+
+ * css/CSSCustomPropertyValue.cpp:
+ (WebCore::CSSCustomPropertyValue::equals const):
+
+ Check for pointer equality first.
+
+ * rendering/style/RenderStyle.cpp:
+ (WebCore::RenderStyle::deduplicateInheritedCustomProperties):
+
+ Deduplicate if the properties are equal but not shared.
+
+ (WebCore::RenderStyle::setInheritedCustomPropertyValue):
+ (WebCore::RenderStyle::setNonInheritedCustomPropertyValue):
+
+ Check if the value is already in the map before triggering copy-on-write.
+
+ * rendering/style/RenderStyle.h:
+ (WebCore::RenderStyle::setInheritedCustomPropertyValue): Deleted.
+ (WebCore::RenderStyle::setNonInheritedCustomPropertyValue): Deleted.
+ * style/StyleBuilder.cpp:
+ (WebCore::Style::Builder::applyCustomProperty):
+
+ Don't create unnecesary copies of CSSCustomPropertyValue.
+
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
+
+ Try to deduplicate before comparing the styles.
+
2020-09-07 Sergio Villar Senin <[email protected]>
[css-flex] Don't skip flexboxes with auto height for percentage computations in quirks mode
Modified: trunk/Source/WebCore/css/CSSCustomPropertyValue.cpp (266716 => 266717)
--- trunk/Source/WebCore/css/CSSCustomPropertyValue.cpp 2020-09-08 07:35:49 UTC (rev 266716)
+++ trunk/Source/WebCore/css/CSSCustomPropertyValue.cpp 2020-09-08 08:48:38 UTC (rev 266717)
@@ -31,6 +31,8 @@
bool CSSCustomPropertyValue::equals(const CSSCustomPropertyValue& other) const
{
+ if (this == &other)
+ return true;
if (m_name != other.m_name || m_value.index() != other.m_value.index())
return false;
return WTF::switchOn(m_value, [&](const Ref<CSSVariableReferenceValue>& value) {
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (266716 => 266717)
--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2020-09-08 07:35:49 UTC (rev 266716)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2020-09-08 08:48:38 UTC (rev 266717)
@@ -2341,6 +2341,30 @@
}
}
+void RenderStyle::deduplicateInheritedCustomProperties(const RenderStyle& other)
+{
+ auto& properties = const_cast<DataRef<StyleCustomPropertyData>&>(m_rareInheritedData->customProperties);
+ auto& otherProperties = other.m_rareInheritedData->customProperties;
+ if (properties.ptr() != otherProperties.ptr() && *properties == *otherProperties)
+ properties = otherProperties;
+}
+
+void RenderStyle::setInheritedCustomPropertyValue(const AtomString& name, Ref<CSSCustomPropertyValue>&& value)
+{
+ auto* existingValue = m_rareInheritedData->customProperties->values.get(name);
+ if (existingValue && existingValue->equals(value.get()))
+ return;
+ m_rareInheritedData.access().customProperties.access().setCustomPropertyValue(name, WTFMove(value));
+}
+
+void RenderStyle::setNonInheritedCustomPropertyValue(const AtomString& name, Ref<CSSCustomPropertyValue>&& value)
+{
+ auto* existingValue = m_rareNonInheritedData->customProperties->values.get(name);
+ if (existingValue && existingValue->equals(value.get()))
+ return;
+ m_rareNonInheritedData.access().customProperties.access().setCustomPropertyValue(name, WTFMove(value));
+}
+
#if ENABLE(CSS_SCROLL_SNAP)
ScrollSnapType RenderStyle::initialScrollSnapType()
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (266716 => 266717)
--- trunk/Source/WebCore/rendering/style/RenderStyle.h 2020-09-08 07:35:49 UTC (rev 266716)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h 2020-09-08 08:48:38 UTC (rev 266717)
@@ -183,11 +183,12 @@
const PseudoStyleCache* cachedPseudoStyles() const { return m_cachedPseudoStyles.get(); }
+ void deduplicateInheritedCustomProperties(const RenderStyle&);
const CustomPropertyValueMap& inheritedCustomProperties() const { return m_rareInheritedData->customProperties->values; }
const CustomPropertyValueMap& nonInheritedCustomProperties() const { return m_rareNonInheritedData->customProperties->values; }
const CSSCustomPropertyValue* getCustomProperty(const AtomString&) const;
- void setInheritedCustomPropertyValue(const AtomString& name, Ref<CSSCustomPropertyValue>&& value) { return m_rareInheritedData.access().customProperties.access().setCustomPropertyValue(name, WTFMove(value)); }
- void setNonInheritedCustomPropertyValue(const AtomString& name, Ref<CSSCustomPropertyValue>&& value) { return m_rareNonInheritedData.access().customProperties.access().setCustomPropertyValue(name, WTFMove(value)); }
+ void setInheritedCustomPropertyValue(const AtomString& name, Ref<CSSCustomPropertyValue>&&);
+ void setNonInheritedCustomPropertyValue(const AtomString& name, Ref<CSSCustomPropertyValue>&&);
void setHasViewportUnits(bool v = true) { m_nonInheritedFlags.hasViewportUnits = v; }
bool hasViewportUnits() const { return m_nonInheritedFlags.hasViewportUnits; }
Modified: trunk/Source/WebCore/style/StyleBuilder.cpp (266716 => 266717)
--- trunk/Source/WebCore/style/StyleBuilder.cpp 2020-09-08 07:35:49 UTC (rev 266716)
+++ trunk/Source/WebCore/style/StyleBuilder.cpp 2020-09-08 08:48:38 UTC (rev 266717)
@@ -195,7 +195,7 @@
if (index != SelectorChecker::MatchDefault && m_state.style().insideLink() == InsideLink::NotInside)
continue;
- Ref<CSSCustomPropertyValue> valueToApply = CSSCustomPropertyValue::create(downcast<CSSCustomPropertyValue>(*property.cssValue[index]));
+ auto valueToApply = makeRef(downcast<CSSCustomPropertyValue>(*property.cssValue[index]));
if (inCycle) {
m_state.m_appliedCustomProperties.add(name); // Make sure we do not try to apply this property again while resolving it.
@@ -231,7 +231,7 @@
if (index != SelectorChecker::MatchDefault && m_state.style().insideLink() == InsideLink::NotInside)
continue;
- Ref<CSSCustomPropertyValue> valueToApply = CSSCustomPropertyValue::create(downcast<CSSCustomPropertyValue>(*property.cssValue[index]));
+ auto valueToApply = makeRef(downcast<CSSCustomPropertyValue>(*property.cssValue[index]));
if (inCycle && WTF::holds_alternative<Ref<CSSVariableReferenceValue>>(valueToApply->value())) {
// Resolve this value so that we reset its dependencies.
Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (266716 => 266717)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2020-09-08 07:35:49 UTC (rev 266716)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2020-09-08 08:48:38 UTC (rev 266717)
@@ -361,6 +361,11 @@
if (animationImpact)
Adjuster::adjustAnimatedStyle(*newStyle, parentBoxStyle(), animationImpact);
+ // Deduplication speeds up equality comparisons as the properties inherit to descendants.
+ // FIXME: There should be a more general mechanism for this.
+ if (oldStyle)
+ newStyle->deduplicateInheritedCustomProperties(*oldStyle);
+
auto change = oldStyle ? determineChange(*oldStyle, *newStyle) : Detach;
auto validity = element.styleValidity();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
