Changeset 220383 in webkit

Timestamp:

Aug 7, 2017 7:52:10 PM (7 years ago)

Author:

Simon Fraser

Message:

RenderStyle:diff() was inadvertently doing deep compares of StyleRareNonInheritedData etc
​https://bugs.webkit.org/show_bug.cgi?id=175304

Reviewed by Tim Horton.

r210758 changed DataRef::get() from returning a pointer to a reference. This caused all the places
in RenderStyle::diff() and related functions, where we intended to do a quick pointer comparison,
to doing deep compares via operator!=. This made the code slightly slower.

Fix by exposing ptr() on DataRef and using it wherever we wish to do pointer comparison.

• rendering/style/DataRef.h:

(WebCore::DataRef::ptr const):

• rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::inheritedDataShared const):
(WebCore::RenderStyle::changeAffectsVisualOverflow const):
(WebCore::RenderStyle::changeRequiresLayout const):
(WebCore::RenderStyle::changeRequiresRecompositeLayer const):
(WebCore::RenderStyle::listStyleImage const): Expand the function onto multiple lines.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/style/DataRef.h (modified) (1 diff)
• rendering/style/RenderStyle.cpp (modified) (6 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-08-07  Simon Fraser  <simon.fraser@apple.com>
+
+        RenderStyle:diff() was inadvertently doing deep compares of StyleRareNonInheritedData etc
+        https://bugs.webkit.org/show_bug.cgi?id=175304
+
+        Reviewed by Tim Horton.
+
+        r210758 changed DataRef::get() from returning a pointer to a reference. This caused all the places
+        in RenderStyle::diff() and related functions, where we intended to do a quick pointer comparison,
+        to doing deep compares via operator!=. This made the code slightly slower.
+
+        Fix by exposing ptr() on DataRef and using it wherever we wish to do pointer comparison.
+
+        * rendering/style/DataRef.h:
+        (WebCore::DataRef::ptr const):
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::inheritedDataShared const):
+        (WebCore::RenderStyle::changeAffectsVisualOverflow const):
+        (WebCore::RenderStyle::changeRequiresLayout const):
+        (WebCore::RenderStyle::changeRequiresRecompositeLayer const):
+        (WebCore::RenderStyle::listStyleImage const): Expand the function onto multiple lines.
+
 2017-08-07  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/rendering/style/DataRef.h

@@ -56 +56 @@
     }
 
+    const T* ptr() const
+    {
+        return m_data.ptr();
+    }
+
     const T& get() const
     {

trunk/Source/WebCore/rendering/style/RenderStyle.cpp

@@ -453 +453 @@
     // This is a fast check that only looks if the data structures are shared.
     return m_inheritedFlags == other->m_inheritedFlags
-        && m_inheritedData.get() == other->m_inheritedData.get()
-        && m_svgStyle.get() == other->m_svgStyle.get()
-        && m_rareInheritedData.get() == other->m_rareInheritedData.get();
+        && m_inheritedData.ptr() == other->m_inheritedData.ptr()
+        && m_svgStyle.ptr() == other->m_svgStyle.ptr()
+        && m_rareInheritedData.ptr() == other->m_rareInheritedData.ptr();
 }
 
@@ -487 +487 @@
 inline bool RenderStyle::changeAffectsVisualOverflow(const RenderStyle& other) const
 {
-    if (m_rareNonInheritedData.get() != other.m_rareNonInheritedData.get()
+    if (m_rareNonInheritedData.ptr() != other.m_rareNonInheritedData.ptr()
         && !arePointingToEqualData(m_rareNonInheritedData->boxShadow, other.m_rareNonInheritedData->boxShadow))
         return true;
 
-    if (m_rareInheritedData.get() != other.m_rareInheritedData.get()
+    if (m_rareInheritedData.ptr() != other.m_rareInheritedData.ptr()
         && !arePointingToEqualData(m_rareInheritedData->textShadow, other.m_rareInheritedData->textShadow))
         return true;
@@ -537 +537 @@
         return true;
 
-    if (m_rareNonInheritedData.get() != other.m_rareNonInheritedData.get()) {
+    if (m_rareNonInheritedData.ptr() != other.m_rareNonInheritedData.ptr()) {
         if (m_rareNonInheritedData->appearance != other.m_rareNonInheritedData->appearance
             || m_rareNonInheritedData->marginBeforeCollapse != other.m_rareNonInheritedData->marginBeforeCollapse
@@ -598 +598 @@
     }
 
-    if (m_rareInheritedData.get() != other.m_rareInheritedData.get()) {
+    if (m_rareInheritedData.ptr() != other.m_rareInheritedData.ptr()) {
         if (m_rareInheritedData->indent != other.m_rareInheritedData->indent
 #if ENABLE(CSS3_TEXT)
@@ -911 +911 @@
 bool RenderStyle::changeRequiresRecompositeLayer(const RenderStyle& other, unsigned&) const
 {
-    if (m_rareNonInheritedData.get() != other.m_rareNonInheritedData.get()) {
+    if (m_rareNonInheritedData.ptr() != other.m_rareNonInheritedData.ptr()) {
         if (m_rareNonInheritedData->transformStyle3D != other.m_rareNonInheritedData->transformStyle3D
             || m_rareNonInheritedData->backfaceVisibility != other.m_rareNonInheritedData->backfaceVisibility
@@ -1197 +1197 @@
 }
 
-StyleImage* RenderStyle::listStyleImage() const { return m_rareInheritedData->listStyleImage.get(); }
+StyleImage* RenderStyle::listStyleImage() const
+{
+    return m_rareInheritedData->listStyleImage.get();
+}
 
 void RenderStyle::setListStyleImage(RefPtr<StyleImage>&& v)