Changeset 260716 in webkit


Ignore:
Timestamp:
Apr 25, 2020 6:24:03 PM (4 years ago)
Author:
Simon Fraser
Message:

Commit the scrolling tree from the main thread
​https://bugs.webkit.org/show_bug.cgi?id=211026
<rdar://problem/62374855>

Reviewed by Darin Adler.

ScrollingCoordinatorMac::commitTreeStateIfNeeded() passed the new state tree to
the scrolling thread which then did the commit (which updates the scrolling tree
from the state tree). However, applyLayerPositions() immediately waited for that
commit to complete, blocking the main thread anyway.

We might as well just commit the scrolling tree on the main thread. ScrollingTree::commitTreeState()
locks m_treeMutex, so this is still safe. Lock contention with the scrolling or event dispatcher
threads should be rare; those threads are both mostly responsive.

  • page/scrolling/AsyncScrollingCoordinator.cpp:

(WebCore::AsyncScrollingCoordinator::scrollingTreeAsText const):

  • page/scrolling/ScrollingTree.cpp:

(WebCore::ScrollingTree::commitTreeState):

  • page/scrolling/ScrollingTree.h:

(WebCore::ScrollingTree::waitForScrollingTreeCommit): Deleted.

  • page/scrolling/ThreadedScrollingTree.cpp:

(WebCore::ThreadedScrollingTree::commitTreeState): Deleted.
(WebCore::ThreadedScrollingTree::incrementPendingCommitCount): Deleted.
(WebCore::ThreadedScrollingTree::decrementPendingCommitCount): Deleted.
(WebCore::ThreadedScrollingTree::waitForPendingCommits): Deleted.
(WebCore::ThreadedScrollingTree::waitForScrollingTreeCommit): Deleted.
(WebCore::ThreadedScrollingTree::applyLayerPositions): Deleted.

  • page/scrolling/ThreadedScrollingTree.h:
  • page/scrolling/mac/ScrollingCoordinatorMac.mm:

(WebCore::ScrollingCoordinatorMac::commitTreeStateIfNeeded):

  • page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp:

(WebCore::ScrollingCoordinatorNicosia::commitTreeState):

Location:
trunk/Source/WebCore
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r260714 r260716  
     12020-04-25  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Commit the scrolling tree from the main thread
     4        https://bugs.webkit.org/show_bug.cgi?id=211026
     5        <rdar://problem/62374855>
     6
     7        Reviewed by Darin Adler.
     8
     9        ScrollingCoordinatorMac::commitTreeStateIfNeeded() passed the new state tree to
     10        the scrolling thread which then did the commit (which updates the scrolling tree
     11        from the state tree). However, applyLayerPositions() immediately waited for that
     12        commit to complete, blocking the main thread anyway.
     13
     14        We might as well just commit the scrolling tree on the main thread. ScrollingTree::commitTreeState()
     15        locks m_treeMutex, so this is still safe. Lock contention with the scrolling or event dispatcher
     16        threads should be rare; those threads are both mostly responsive.
     17
     18        * page/scrolling/AsyncScrollingCoordinator.cpp:
     19        (WebCore::AsyncScrollingCoordinator::scrollingTreeAsText const):
     20        * page/scrolling/ScrollingTree.cpp:
     21        (WebCore::ScrollingTree::commitTreeState):
     22        * page/scrolling/ScrollingTree.h:
     23        (WebCore::ScrollingTree::waitForScrollingTreeCommit): Deleted.
     24        * page/scrolling/ThreadedScrollingTree.cpp:
     25        (WebCore::ThreadedScrollingTree::commitTreeState): Deleted.
     26        (WebCore::ThreadedScrollingTree::incrementPendingCommitCount): Deleted.
     27        (WebCore::ThreadedScrollingTree::decrementPendingCommitCount): Deleted.
     28        (WebCore::ThreadedScrollingTree::waitForPendingCommits): Deleted.
     29        (WebCore::ThreadedScrollingTree::waitForScrollingTreeCommit): Deleted.
     30        (WebCore::ThreadedScrollingTree::applyLayerPositions): Deleted.
     31        * page/scrolling/ThreadedScrollingTree.h:
     32        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
     33        (WebCore::ScrollingCoordinatorMac::commitTreeStateIfNeeded):
     34        * page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp:
     35        (WebCore::ScrollingCoordinatorNicosia::commitTreeState):
     36
    1372020-04-25  Yusuke Suzuki  <ysuzuki@apple.com>
    238

  • trunk/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp

    r260557 r260716  
    854854        return emptyString();
    855855
    856     m_scrollingTree->waitForScrollingTreeCommit();
    857856    return m_scrollingTree->scrollingTreeAsText(behavior);
    858857}

  • trunk/Source/WebCore/page/scrolling/ScrollingTree.cpp

    r260571 r260716  
    192192}
    193193
    194 void ScrollingTree::commitTreeState(std::unique_ptr<ScrollingStateTree> scrollingStateTree)
     194void ScrollingTree::commitTreeState(std::unique_ptr<ScrollingStateTree>&& scrollingStateTree)
    195195{
    196196    SetForScope<bool> inCommitTreeState(m_inCommitTreeState, true);

  • trunk/Source/WebCore/page/scrolling/ScrollingTree.h

    r260571 r260716  
    7979
    8080    virtual void invalidate() { }
    81     WEBCORE_EXPORT virtual void commitTreeState(std::unique_ptr<ScrollingStateTree>);
     81    WEBCORE_EXPORT virtual void commitTreeState(std::unique_ptr<ScrollingStateTree>&&);
    8282   
    8383    WEBCORE_EXPORT virtual void applyLayerPositions();
    … …  
    174174    virtual void lockLayersForHitTesting() { }
    175175    virtual void unlockLayersForHitTesting() { }
    176 
    177     virtual void waitForScrollingTreeCommit() { }
    178176
    179177protected:

  • trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp

    r260199 r260716  
    8686}
    8787
    88 void ThreadedScrollingTree::commitTreeState(std::unique_ptr<ScrollingStateTree> scrollingStateTree)
    89 {
    90     ASSERT(ScrollingThread::isCurrentThread());
    91     ScrollingTree::commitTreeState(WTFMove(scrollingStateTree));
    92    
    93     decrementPendingCommitCount();
    94 }
    95 
    9688void ThreadedScrollingTree::propagateSynchronousScrollingReasons(const HashSet<ScrollingNodeID>& synchronousScrollingNodes)
    9789{
    … …  
    161153}
    162154
    163 void ThreadedScrollingTree::incrementPendingCommitCount()
    164 {
    165     LockHolder commitLocker(m_pendingCommitCountMutex);
    166     ++m_pendingCommitCount;
    167 }
    168 
    169 void ThreadedScrollingTree::decrementPendingCommitCount()
    170 {
    171     LockHolder commitLocker(m_pendingCommitCountMutex);
    172     ASSERT(m_pendingCommitCount > 0);
    173     if (!--m_pendingCommitCount)
    174         m_commitCondition.notifyOne();
    175 }
    176 
    177 void ThreadedScrollingTree::waitForPendingCommits()
    178 {
    179     ASSERT(isMainThread());
    180 
    181     LockHolder commitLocker(m_pendingCommitCountMutex);
    182     while (m_pendingCommitCount)
    183         m_commitCondition.wait(m_pendingCommitCountMutex);
    184 }
    185 
    186 void ThreadedScrollingTree::waitForScrollingTreeCommit()
    187 {
    188     waitForPendingCommits();
    189 }
    190 
    191 void ThreadedScrollingTree::applyLayerPositions()
    192 {
    193     waitForPendingCommits();
    194     ScrollingTree::applyLayerPositions();
    195 }
    196 
    197155#if PLATFORM(COCOA)
    198156void ThreadedScrollingTree::currentSnapPointIndicesDidChange(ScrollingNodeID nodeID, unsigned horizontal, unsigned vertical)

  • trunk/Source/WebCore/page/scrolling/ThreadedScrollingTree.h

    r260190 r260716  
    4545    virtual ~ThreadedScrollingTree();
    4646
    47     void commitTreeState(std::unique_ptr<ScrollingStateTree>) override;
    48 
    4947    ScrollingEventResult handleWheelEvent(const PlatformWheelEvent&) override;
    5048
    … …  
    5553
    5654    void invalidate() override;
    57 
    58     void incrementPendingCommitCount();
    59     void decrementPendingCommitCount();
    6055
    6156protected:
    … …  
    7873private:
    7974    bool isThreadedScrollingTree() const override { return true; }
    80     void applyLayerPositions() override;
    81     void waitForScrollingTreeCommit() override;
    8275    void propagateSynchronousScrollingReasons(const HashSet<ScrollingNodeID>&) override;
    8376
    8477    RefPtr<AsyncScrollingCoordinator> m_scrollingCoordinator;
    85 
    86     void waitForPendingCommits();
    87 
    88     Lock m_pendingCommitCountMutex;
    89     unsigned m_pendingCommitCount { 0 };
    90     Condition m_commitCondition;
    9178};
    9279

  • trunk/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm

    r260366 r260716  
    9898    willCommitTree();
    9999
    100     LOG(Scrolling, "ScrollingCoordinatorMac::commitTreeState, has changes %d", scrollingStateTree()->hasChangedProperties());
     100    LOG_WITH_STREAM(Scrolling, stream << "ScrollingCoordinatorMac::commitTreeState, has changes " << scrollingStateTree()->hasChangedProperties());
    101101
    102102    if (!scrollingStateTree()->hasChangedProperties())
    103103        return;
    104104
    105     LOG(Scrolling, "%s", scrollingStateTreeAsText(ScrollingStateTreeAsTextBehaviorDebug).utf8().data());
     105    LOG_WITH_STREAM(Scrolling, stream << scrollingStateTreeAsText(ScrollingStateTreeAsTextBehaviorDebug));
    106106
    107     RefPtr<ThreadedScrollingTree> threadedScrollingTree = downcast<ThreadedScrollingTree>(scrollingTree());
    108     ScrollingStateTree* unprotectedTreeState = scrollingStateTree()->commit(LayerRepresentation::PlatformLayerRepresentation).release();
    109 
    110     threadedScrollingTree->incrementPendingCommitCount();
    111 
    112     ScrollingThread::dispatch([threadedScrollingTree, unprotectedTreeState] {
    113         std::unique_ptr<ScrollingStateTree> treeState(unprotectedTreeState);
    114         threadedScrollingTree->commitTreeState(WTFMove(treeState));
    115     });
     107    auto stateTree = scrollingStateTree()->commit(LayerRepresentation::PlatformLayerRepresentation);
     108    scrollingTree()->commitTreeState(WTFMove(stateTree));
    116109
    117110    updateTiledScrollingIndicator();

  • trunk/Source/WebCore/page/scrolling/nicosia/ScrollingCoordinatorNicosia.cpp

    r250419 r260716  
    9595        return;
    9696
    97     RefPtr<ThreadedScrollingTree> threadedScrollingTree = downcast<ThreadedScrollingTree>(scrollingTree());
    98     threadedScrollingTree->incrementPendingCommitCount();
    99 
    100     auto treeState = scrollingStateTree()->commit(LayerRepresentation::PlatformLayerRepresentation);
    101     ScrollingThread::dispatch([threadedScrollingTree, treeState = WTFMove(treeState)]() mutable {
    102         threadedScrollingTree->commitTreeState(WTFMove(treeState));
    103     });
     97    auto stateTree = scrollingStateTree()->commit(LayerRepresentation::PlatformLayerRepresentation);
     98    scrollingTree()->commitTreeState(WTFMove(stateTree));
    10499}
    105100
Note: See TracChangeset for help on using the changeset viewer.