Avoid compositing updates after style recalcs which have no compositing implications
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 2 May 2015 02:32:29 +0000 (02:32 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 2 May 2015 02:32:29 +0000 (02:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144502

Reviewed by Darin Adler.

Source/WebCore:

After r183461, we have reliable information about whether a style change with zero
diff can be reliably ignored. Use that information to track whether a given
recalcStyle() does anything which should force a compositing update.

This eliminates up to 40% of the post-recalcStyle compositing updates on some pages.

Add Internals API to test.

Test: compositing/updates/no-style-change-updates.html

* dom/Document.cpp:
(WebCore::Document::recalcStyle): Tell the FrameView we're going to recalc style.
* page/FrameView.cpp:
(WebCore::FrameView::willRecalcStyle): Pass it on to the compositor.
(WebCore::FrameView::updateCompositingLayersAfterStyleChange): Move the code
that was here into RenderLayerCompositor::didRecalcStyleWithNoPendingLayout().
* page/FrameView.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::willRecalcStyle): Reset the m_layerNeedsCompositingUpdate flag.
(WebCore::RenderLayerCompositor::didRecalcStyleWithNoPendingLayout): Bail on the update if
no layers changed.
(WebCore::RenderLayerCompositor::updateCompositingLayers): Logging. Increment m_compositingUpdateCount,
which is used for testing.
(WebCore::RenderLayerCompositor::layerStyleChanged): Set the m_layerNeedsCompositingUpdate flag.
(WebCore::RenderLayerCompositor::startTrackingCompositingUpdates): Reset the counter.
(WebCore::RenderLayerCompositor::compositingUpdateCount):
* rendering/RenderLayerCompositor.h:
* testing/Internals.cpp:
(WebCore::Internals::startTrackingCompositingUpdates):
(WebCore::Internals::compositingUpdateCount):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

Use internals.compositingUpdateCount() to see if various document mutations
cause a compositing update. Doesn't actually detect any behavior change
from this patch, but seems useful in general.

* compositing/updates/no-style-change-updates-expected.txt: Added.
* compositing/updates/no-style-change-updates.html: Added.

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/compositing/updates/no-style-change-updates-expected.txt [new file with mode: 0644]
LayoutTests/compositing/updates/no-style-change-updates.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index 0881217..ea3c83e 100644 (file)
@@ -1,3 +1,17 @@
+2015-05-01  Simon Fraser  <simon.fraser@apple.com>
+
+        Avoid compositing updates after style recalcs which have no compositing implications
+        https://bugs.webkit.org/show_bug.cgi?id=144502
+
+        Reviewed by Darin Adler.
+        
+        Use internals.compositingUpdateCount() to see if various document mutations
+        cause a compositing update. Doesn't actually detect any behavior change
+        from this patch, but seems useful in general.
+
+        * compositing/updates/no-style-change-updates-expected.txt: Added.
+        * compositing/updates/no-style-change-updates.html: Added.
+
 2015-05-01  Ryosuke Niwa  <rniwa@webkit.org>
 
         Class syntax should allow string and numeric identifiers for method names
diff --git a/LayoutTests/compositing/updates/no-style-change-updates-expected.txt b/LayoutTests/compositing/updates/no-style-change-updates-expected.txt
new file mode 100644 (file)
index 0000000..02eef48
--- /dev/null
@@ -0,0 +1,14 @@
+Test that compositing updates do not happen for style changes that do not need them.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS updateCount is 0
+PASS updateCount is 1
+PASS updateCount is 0
+PASS updateCount is 1
+PASS updateCount is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/compositing/updates/no-style-change-updates.html b/LayoutTests/compositing/updates/no-style-change-updates.html
new file mode 100644 (file)
index 0000000..f8bdb18
--- /dev/null
@@ -0,0 +1,109 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+        .box {
+            width: 100px;
+            height: 100px;
+            margin: 10px;
+            background-color: blue;
+            border: 2px solid black;
+            -webkit-transform: translateZ(0);
+        }
+        
+        .colorchange {
+            background-color: green;
+        }
+        
+        .dummy {
+            /* Empty. */
+        }
+        
+    </style>
+    <script src="../../resources/js-test-pre.js"></script>
+    <script>
+        description('Test that compositing updates do not happen for style changes that do not need them.');
+        window.jsTestIsAsync = true;
+
+        var updateCount;
+        
+        function zeroUpdateCount()
+        {
+            if (window.internals) {
+                internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
+                internals.startTrackingCompositingUpdates();
+            }
+        }
+
+        function updateLayoutAndCompositingCount()
+        {
+            if (window.internals) {
+                internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
+                updateCount = internals.compositingUpdateCount();
+            }
+        }
+        
+        function testForCompositingUpdate(callback)
+        {
+            zeroUpdateCount();
+            callback();
+            updateLayoutAndCompositingCount();
+        }
+        
+        function runTest()
+        {
+            updateLayoutAndCompositingCount();
+
+            if (window.internals) {
+                internals.startTrackingCompositingUpdates();
+                updateCount = internals.compositingUpdateCount();
+            }
+
+            shouldBe('updateCount', '0');
+            
+            testForCompositingUpdate(function() {
+                document.getElementById('box').classList.add('colorchange');
+            });
+            
+            shouldBe('updateCount', '1');
+
+            testForCompositingUpdate(function() {
+                document.getElementById('box').classList.add('dummy');
+            });
+
+            shouldBe('updateCount', '0');
+            
+            testForCompositingUpdate(function() {
+                var div = document.createElement('div');
+                div.className = 'box';
+                document.body.appendChild(div);
+            });
+            
+            shouldBe('updateCount', '1');
+            
+            testForCompositingUpdate(function() {
+                var stylesheet = document.createElement('style');
+                stylesheet.type = 'text/css';
+                stylesheet.rel = 'stylesheet';
+                stylesheet.textContent = '.unmatched { border: 5px solid blue; }';
+                document.getElementsByTagName("head")[0].appendChild(stylesheet);
+            });
+            
+            shouldBe('updateCount', '0');
+
+            finishJSTest();
+        }
+        
+        window.addEventListener('load', function() {
+            window.setTimeout(runTest, 200);
+        }, false);
+    </script>
+
+</head>
+<body>
+    <div id="box" class="box"></div>
+
+    <script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index bdee86c..e718226 100644 (file)
@@ -1,3 +1,43 @@
+2015-05-01  Simon Fraser  <simon.fraser@apple.com>
+
+        Avoid compositing updates after style recalcs which have no compositing implications
+        https://bugs.webkit.org/show_bug.cgi?id=144502
+
+        Reviewed by Darin Adler.
+        
+        After r183461, we have reliable information about whether a style change with zero
+        diff can be reliably ignored. Use that information to track whether a given
+        recalcStyle() does anything which should force a compositing update.
+        
+        This eliminates up to 40% of the post-recalcStyle compositing updates on some pages.
+        
+        Add Internals API to test.
+
+        Test: compositing/updates/no-style-change-updates.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::recalcStyle): Tell the FrameView we're going to recalc style.
+        * page/FrameView.cpp:
+        (WebCore::FrameView::willRecalcStyle): Pass it on to the compositor.
+        (WebCore::FrameView::updateCompositingLayersAfterStyleChange): Move the code
+        that was here into RenderLayerCompositor::didRecalcStyleWithNoPendingLayout().
+        * page/FrameView.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::willRecalcStyle): Reset the m_layerNeedsCompositingUpdate flag.
+        (WebCore::RenderLayerCompositor::didRecalcStyleWithNoPendingLayout): Bail on the update if
+        no layers changed.
+        (WebCore::RenderLayerCompositor::updateCompositingLayers): Logging. Increment m_compositingUpdateCount,
+        which is used for testing.
+        (WebCore::RenderLayerCompositor::layerStyleChanged): Set the m_layerNeedsCompositingUpdate flag.
+        (WebCore::RenderLayerCompositor::startTrackingCompositingUpdates): Reset the counter.
+        (WebCore::RenderLayerCompositor::compositingUpdateCount):
+        * rendering/RenderLayerCompositor.h:
+        * testing/Internals.cpp:
+        (WebCore::Internals::startTrackingCompositingUpdates):
+        (WebCore::Internals::compositingUpdateCount):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2015-05-01  Andreas Kling  <akling@apple.com>
 
         Reproducible crash removing name attribute from <img> node
index faed8fd..4396c72 100644 (file)
@@ -1748,6 +1748,8 @@ void Document::recalcStyle(Style::Change change)
 
     m_styleSheetCollection.flushPendingUpdates();
 
+    frameView.willRecalcStyle();
+
     InspectorInstrumentationCookie cookie = InspectorInstrumentation::willRecalculateStyle(*this);
 
     // FIXME: We never reset this flags.
index 94784e7..1774d97 100644 (file)
@@ -728,6 +728,15 @@ void FrameView::calculateScrollbarModesForLayout(ScrollbarMode& hMode, Scrollbar
     }    
 }
 
+void FrameView::willRecalcStyle()
+{
+    RenderView* renderView = this->renderView();
+    if (!renderView)
+        return;
+
+    renderView->compositor().willRecalcStyle();
+}
+
 void FrameView::updateCompositingLayersAfterStyleChange()
 {
     RenderView* renderView = this->renderView();
@@ -738,10 +747,7 @@ void FrameView::updateCompositingLayersAfterStyleChange()
     if (inPreLayoutStyleUpdate() || layoutPending() || renderView->needsLayout())
         return;
 
-    RenderLayerCompositor& compositor = renderView->compositor();
-    // This call will make sure the cached hasAcceleratedCompositing is updated from the pref
-    compositor.cacheAcceleratedCompositingFlags();
-    compositor.updateCompositingLayers(CompositingUpdateAfterStyleChange);
+    renderView->compositor().didRecalcStyleWithNoPendingLayout();
 }
 
 void FrameView::updateCompositingLayersAfterLayout()
index 60a8ded..c5117fd 100644 (file)
@@ -147,6 +147,7 @@ public:
     WEBCORE_EXPORT void serviceScriptedAnimations(double monotonicAnimationStartTime);
 #endif
 
+    void willRecalcStyle();
     void updateCompositingLayersAfterStyleChange();
     void updateCompositingLayersAfterLayout();
     bool flushCompositingStateForThisFrame(Frame* rootFrameForFlush);
index 323aad9..54949b1 100644 (file)
@@ -384,6 +384,20 @@ void RenderLayerCompositor::setCompositingLayersNeedRebuild(bool needRebuild)
         m_compositingLayersNeedRebuild = needRebuild;
 }
 
+void RenderLayerCompositor::willRecalcStyle()
+{
+    m_layerNeedsCompositingUpdate = false;
+}
+
+void RenderLayerCompositor::didRecalcStyleWithNoPendingLayout()
+{
+    if (!m_layerNeedsCompositingUpdate)
+        return;
+    
+    cacheAcceleratedCompositingFlags();
+    updateCompositingLayers(CompositingUpdateAfterStyleChange);
+}
+
 void RenderLayerCompositor::customPositionForVisibleRectComputation(const GraphicsLayer* graphicsLayer, FloatPoint& position) const
 {
     if (graphicsLayer != m_scrollLayer.get())
@@ -656,6 +670,8 @@ void RenderLayerCompositor::cancelCompositingLayerUpdate()
 
 void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType updateType, RenderLayer* updateRoot)
 {
+    LOG(Compositing, "RenderLayerCompositor %p updateCompositingLayers %d %p", this, updateType, updateRoot);
+
     m_updateCompositingLayersTimer.stop();
 
     ASSERT(!m_renderView.document().inPageCache());
@@ -674,6 +690,8 @@ void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType update
     if (!m_reevaluateCompositingAfterLayout && !m_compositing)
         return;
 
+    ++m_compositingUpdateCount;
+
     AnimationUpdateBlock animationUpdateBlock(&m_renderView.frameView().frame().animation());
 
     TemporaryChange<bool> postLayoutChange(m_inPostLayoutUpdate, true);
@@ -710,6 +728,8 @@ void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType update
     if (isFullUpdate && updateType == CompositingUpdateAfterLayout)
         m_reevaluateCompositingAfterLayout = false;
 
+    LOG(Compositing, " checkForHierarchyUpdate %d, needGeometryUpdate %d", checkForHierarchyUpdate, needHierarchyUpdate);
+
 #if !LOG_DISABLED
     double startTime = 0;
     if (compositingLogEnabled()) {
@@ -920,6 +940,8 @@ void RenderLayerCompositor::layerStyleChanged(StyleDifference diff, RenderLayer&
     if (diff == StyleDifferenceEqual)
         return;
 
+    m_layerNeedsCompositingUpdate = true;
+
     const RenderStyle& newStyle = layer.renderer().style();
     if (updateLayerCompositingState(layer) || (oldStyle && styleChangeRequiresLayerRebuild(layer, *oldStyle, newStyle)))
         setCompositingLayersNeedRebuild();
@@ -4186,4 +4208,14 @@ unsigned RenderLayerCompositor::layerFlushCount() const
     return m_layerFlushCount;
 }
 
+void RenderLayerCompositor::startTrackingCompositingUpdates()
+{
+    m_compositingUpdateCount = 0;
+}
+
+unsigned RenderLayerCompositor::compositingUpdateCount() const
+{
+    return m_compositingUpdateCount;
+}
+
 } // namespace WebCore
index bda4cd1..bf051be 100644 (file)
@@ -119,6 +119,9 @@ public:
     // created, destroyed or re-parented).
     void setCompositingLayersNeedRebuild(bool needRebuild = true);
     bool compositingLayersNeedRebuild() const { return m_compositingLayersNeedRebuild; }
+    
+    void willRecalcStyle();
+    void didRecalcStyleWithNoPendingLayout();
 
     // GraphicsLayers buffer state, which gets pushed to the underlying platform layers
     // at specific times.
@@ -311,6 +314,9 @@ public:
     WEBCORE_EXPORT void startTrackingLayerFlushes();
     WEBCORE_EXPORT unsigned layerFlushCount() const;
 
+    WEBCORE_EXPORT void startTrackingCompositingUpdates();
+    WEBCORE_EXPORT unsigned compositingUpdateCount() const;
+
 private:
     class OverlapMap;
     struct CompositingState;
@@ -498,6 +504,7 @@ private:
     int m_compositedLayerCount { 0 };
     unsigned m_layersWithTiledBackingCount { 0 };
     unsigned m_layerFlushCount { 0 };
+    unsigned m_compositingUpdateCount { 0 };
 
     RootLayerAttachment m_rootLayerAttachment;
 
@@ -534,6 +541,7 @@ private:
     bool m_layerFlushThrottlingEnabled;
     bool m_layerFlushThrottlingTemporarilyDisabledForInteraction;
     bool m_hasPendingLayerFlush;
+    bool m_layerNeedsCompositingUpdate { false };
 
     Timer m_paintRelatedMilestonesTimer;
 
index a14c8f4..698f472 100644 (file)
@@ -2114,6 +2114,28 @@ unsigned long Internals::styleRecalcCount(ExceptionCode& ec)
     return document->styleRecalcCount();
 }
 
+void Internals::startTrackingCompositingUpdates(ExceptionCode& ec)
+{
+    Document* document = contextDocument();
+    if (!document || !document->renderView()) {
+        ec = INVALID_ACCESS_ERR;
+        return;
+    }
+
+    document->renderView()->compositor().startTrackingCompositingUpdates();
+}
+
+unsigned long Internals::compositingUpdateCount(ExceptionCode& ec)
+{
+    Document* document = contextDocument();
+    if (!document || !document->renderView()) {
+        ec = INVALID_ACCESS_ERR;
+        return 0;
+    }
+    
+    return document->renderView()->compositor().compositingUpdateCount();
+}
+
 void Internals::updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(ExceptionCode& ec)
 {
     updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(nullptr, ec);
index 04625fe..2c180fe 100644 (file)
@@ -303,6 +303,9 @@ public:
     void startTrackingStyleRecalcs(ExceptionCode&);
     unsigned long styleRecalcCount(ExceptionCode&);
 
+    void startTrackingCompositingUpdates(ExceptionCode&);
+    unsigned long compositingUpdateCount(ExceptionCode&);
+
     void updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(ExceptionCode&);
     void updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks(Node*, ExceptionCode&);
 
index b2443a6..806a82d 100644 (file)
@@ -280,6 +280,9 @@ enum ResourceLoadPriority {
     [RaisesException] void startTrackingStyleRecalcs();
     [RaisesException] unsigned long styleRecalcCount();
 
+    [RaisesException] void startTrackingCompositingUpdates();
+    [RaisesException] unsigned long compositingUpdateCount();
+
     // |node| should be Document, HTMLIFrameElement, or unspecified.
     // If |node| is an HTMLIFrameElement, it assumes node.contentDocument is
     // specified without security checks. Unspecified means this document.