Always use matched declarations cache fully when parent inherited style matches
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Nov 2019 00:34:04 +0000 (00:34 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Nov 2019 00:34:04 +0000 (00:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204083

Reviewed by Zalan Bujtas.

* css/StyleResolver.cpp:
(WebCore::StyleResolver::applyMatchedProperties):

We used inheritedDataShared check here since it is always just pointer compare.
However instrumentation shows we miss out from singificant amount of cache benefit
due to this and the full check is not expensive.

* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::inheritedEqual const):
(WebCore::RenderStyle::inheritedNotEqual const): Deleted.

Reverse the logic.

(WebCore::RenderStyle::inheritedDataShared const): Deleted.

Not used anymore.

* rendering/style/RenderStyle.h:
* rendering/style/SVGRenderStyle.cpp:
(WebCore::SVGRenderStyle::inheritedEqual const):
(WebCore::SVGRenderStyle::inheritedNotEqual const): Deleted.
* rendering/style/SVGRenderStyle.h:
* style/StyleChange.cpp:
(WebCore::Style::determineChange):
* style/StyleTreeResolver.cpp:
(WebCore::Style::createInheritedDisplayContentsStyleIfNeeded):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@252344 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/rendering/style/RenderStyle.cpp
Source/WebCore/rendering/style/RenderStyle.h
Source/WebCore/rendering/style/SVGRenderStyle.cpp
Source/WebCore/rendering/style/SVGRenderStyle.h
Source/WebCore/style/StyleChange.cpp
Source/WebCore/style/StyleTreeResolver.cpp

index 964c6b5..3e78225 100644 (file)
@@ -1,5 +1,39 @@
 2019-11-11  Antti Koivisto  <antti@apple.com>
 
+        Always use matched declarations cache fully when parent inherited style matches
+        https://bugs.webkit.org/show_bug.cgi?id=204083
+
+        Reviewed by Zalan Bujtas.
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::applyMatchedProperties):
+
+        We used inheritedDataShared check here since it is always just pointer compare.
+        However instrumentation shows we miss out from singificant amount of cache benefit
+        due to this and the full check is not expensive.
+
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::inheritedEqual const):
+        (WebCore::RenderStyle::inheritedNotEqual const): Deleted.
+
+        Reverse the logic.
+
+        (WebCore::RenderStyle::inheritedDataShared const): Deleted.
+
+        Not used anymore.
+
+        * rendering/style/RenderStyle.h:
+        * rendering/style/SVGRenderStyle.cpp:
+        (WebCore::SVGRenderStyle::inheritedEqual const):
+        (WebCore::SVGRenderStyle::inheritedNotEqual const): Deleted.
+        * rendering/style/SVGRenderStyle.h:
+        * style/StyleChange.cpp:
+        (WebCore::Style::determineChange):
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::createInheritedDisplayContentsStyleIfNeeded):
+
+2019-11-11  Antti Koivisto  <antti@apple.com>
+
         Empty property sets should not mark MatchedProperties uncacheable
         https://bugs.webkit.org/show_bug.cgi?id=204079
 
index 5edbfec..7507bb6 100644 (file)
@@ -553,7 +553,8 @@ void StyleResolver::applyMatchedProperties(State& state, const MatchResult& matc
         // style declarations. We then only need to apply the inherited properties, if any, as their values can depend on the 
         // element context. This is fast and saves memory by reusing the style data structures.
         style.copyNonInheritedFrom(*cacheEntry->renderStyle);
-        if (parentStyle.inheritedDataShared(cacheEntry->parentRenderStyle.get()) && !isAtShadowBoundary(element)) {
+
+        if (parentStyle.inheritedEqual(*cacheEntry->parentRenderStyle) && !isAtShadowBoundary(element)) {
             InsideLink linkStatus = state.style()->insideLink();
             // If the cache item parent style has identical inherited properties to the current parent style then the
             // resulting style will be identical too. We copy the inherited properties over from the cache and are done.
@@ -563,6 +564,7 @@ void StyleResolver::applyMatchedProperties(State& state, const MatchResult& matc
             style.setInsideLink(linkStatus);
             return;
         }
+        
         includedProperties = Style::PropertyCascade::IncludedProperties::InheritedOnly;
     }
 
index 5d38726..87c6ce4 100644 (file)
@@ -428,12 +428,12 @@ void RenderStyle::removeCachedPseudoStyle(PseudoId pid)
     }
 }
 
-bool RenderStyle::inheritedNotEqual(const RenderStyle* other) const
+bool RenderStyle::inheritedEqual(const RenderStyle& other) const
 {
-    return m_inheritedFlags != other->m_inheritedFlags
-        || m_inheritedData != other->m_inheritedData
-        || m_svgStyle->inheritedNotEqual(other->m_svgStyle)
-        || m_rareInheritedData != other->m_rareInheritedData;
+    return m_inheritedFlags == other.m_inheritedFlags
+        && m_inheritedData == other.m_inheritedData
+        && (m_svgStyle.ptr() == other.m_svgStyle.ptr() || m_svgStyle->inheritedEqual(other.m_svgStyle))
+        && m_rareInheritedData == other.m_rareInheritedData;
 }
 
 #if ENABLE(TEXT_AUTOSIZING)
@@ -581,15 +581,6 @@ void RenderStyle::setAutosizeStatus(AutosizeStatus autosizeStatus)
 
 #endif // ENABLE(TEXT_AUTOSIZING)
 
-bool RenderStyle::inheritedDataShared(const RenderStyle* other) const
-{
-    // This is a fast check that only looks if the data structures are shared.
-    return m_inheritedFlags == other->m_inheritedFlags
-        && m_inheritedData.ptr() == other->m_inheritedData.ptr()
-        && m_svgStyle.ptr() == other->m_svgStyle.ptr()
-        && m_rareInheritedData.ptr() == other->m_rareInheritedData.ptr();
-}
-
 static bool positionChangeIsMovementOnly(const LengthBox& a, const LengthBox& b, const Length& width)
 {
     // If any unit types are different, then we can't guarantee
index 42ddd50..3b50e80 100644 (file)
@@ -1396,8 +1396,7 @@ public:
 
     const AtomString& hyphenString() const;
 
-    bool inheritedNotEqual(const RenderStyle*) const;
-    bool inheritedDataShared(const RenderStyle*) const;
+    bool inheritedEqual(const RenderStyle&) const;
 
 #if ENABLE(TEXT_AUTOSIZING)
     uint32_t hashForTextAutosizing() const;
index cd9f364..f5de45f 100644 (file)
@@ -114,13 +114,13 @@ bool SVGRenderStyle::operator==(const SVGRenderStyle& other) const
         && m_nonInheritedFlags == other.m_nonInheritedFlags;
 }
 
-bool SVGRenderStyle::inheritedNotEqual(const SVGRenderStyle& other) const
+bool SVGRenderStyle::inheritedEqual(const SVGRenderStyle& other) const
 {
-    return m_fillData != other.m_fillData
-        || m_strokeData != other.m_strokeData
-        || m_textData != other.m_textData
-        || m_inheritedResourceData != other.m_inheritedResourceData
-        || m_inheritedFlags != other.m_inheritedFlags;
+    return m_fillData == other.m_fillData
+        && m_strokeData == other.m_strokeData
+        && m_textData == other.m_textData
+        && m_inheritedResourceData == other.m_inheritedResourceData
+        && m_inheritedFlags == other.m_inheritedFlags;
 }
 
 void SVGRenderStyle::inheritFrom(const SVGRenderStyle& other)
index fe3f1db..f885e8b 100644 (file)
@@ -37,7 +37,7 @@ public:
     Ref<SVGRenderStyle> copy() const;
     ~SVGRenderStyle();
 
-    bool inheritedNotEqual(const SVGRenderStyle&) const;
+    bool inheritedEqual(const SVGRenderStyle&) const;
     void inheritFrom(const SVGRenderStyle&);
     void copyNonInheritedFrom(const SVGRenderStyle&);
 
index affd773..22adae7 100644 (file)
@@ -50,7 +50,7 @@ Change determineChange(const RenderStyle& s1, const RenderStyle& s2)
         return Detach;
 
     if (s1 != s2) {
-        if (s1.inheritedNotEqual(&s2))
+        if (!s1.inheritedEqual(s2))
             return Inherit;
 
         if (s1.alignItems() != s2.alignItems() || s1.justifyItems() != s2.justifyItems())
index 7d1dc35..41e9f44 100644 (file)
@@ -447,7 +447,7 @@ static std::unique_ptr<RenderStyle> createInheritedDisplayContentsStyleIfNeeded(
 {
     if (parentElementStyle.display() != DisplayType::Contents)
         return nullptr;
-    if (parentBoxStyle && !parentBoxStyle->inheritedNotEqual(&parentElementStyle))
+    if (parentBoxStyle && parentBoxStyle->inheritedEqual(parentElementStyle))
         return nullptr;
     // Compute style for imaginary unstyled <span> around the text node.
     auto style = RenderStyle::createPtr();