Skip to content

[UI-side compositing] Significantly more scrolling sync events #14050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

smfr
Copy link
Contributor

@smfr smfr commented May 19, 2023

c6eb7d8

[UI-side compositing] Significantly more scrolling sync events
https://bugs.webkit.org/show_bug.cgi?id=257001
rdar://109535555

Reviewed by Matt Woodrow.

In the web process scrolling model, we avoid doing some work when
ThreadedScrollingTree::scrollingThreadIsActive() returns false, which means
there are no scrolling animations running, and we haven't received a wheel event in the
last 50ms.

We need to replicate this for UI-process scrolling to optimize power. First, move
scrollingThreadIsActive() to be ScrollingTree::hasRecentActivity().

Second, avoid sending `didRefreshDisplay()` to the scrolling thread if there's not
recent activity.

Third, return early from `RemoteLayerTreeEventDispatcher::mainThreadDisplayDidRefresh()`
if there's no recent activity, avoiding a scrolling thread dispatch.

Finally, return early from `RemoteScrollingTreeMac::didCommitTree()` if there are no
scroll animations to start, avoiding another scrolling thread dispatch.

* Source/WebCore/page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::hasRecentActivity):
* Source/WebCore/page/scrolling/ScrollingTree.h:
* Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp:
(WebCore::ThreadedScrollingTree::willStartRenderingUpdate):
(WebCore::ThreadedScrollingTree::displayDidRefresh):
(WebCore::ThreadedScrollingTree::scrollingThreadIsActive): Deleted.
* Source/WebCore/page/scrolling/ThreadedScrollingTree.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.cpp:
(WebKit::RemoteLayerTreeEventDispatcher::scrollingTreeWasRecentlyActive):
(WebKit::RemoteLayerTreeEventDispatcher::mainThreadDisplayDidRefresh):
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.mm:
(WebKit::RemoteScrollingTreeMac::didCommitTree):

Canonical link: https://commits.webkit.org/264230@main

7b498cc

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 🧪 api-mac ✅ 🛠 gtk
🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 🧪 gtk-wk2
🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv 🧪 mac-AS-debug-wk2
🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@smfr smfr requested a review from cdumez as a code owner May 19, 2023 01:17
@smfr smfr self-assigned this May 19, 2023
@smfr smfr added the WebKit Process Model Bugs related to WebKit's multi-process architecture label May 19, 2023
@@ -123,6 +123,11 @@
void RemoteScrollingTreeMac::didCommitTree()
{
ASSERT(isMainRunLoop());
ASSERT(m_treeLock.isLocked());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably be WTF_REQUIRES_LOCK(m_treeLock) in the header for a compile-time guarantee instead of runtime.

@smfr smfr force-pushed the eng/UI-side-compositing-Significantly-more-scrolling-sync-events branch from 031170f to 7b498cc Compare May 19, 2023 03:44
@smfr smfr added the merge-queue Applied to send a pull request to merge-queue label May 19, 2023
https://bugs.webkit.org/show_bug.cgi?id=257001
rdar://109535555

Reviewed by Matt Woodrow.

In the web process scrolling model, we avoid doing some work when
ThreadedScrollingTree::scrollingThreadIsActive() returns false, which means
there are no scrolling animations running, and we haven't received a wheel event in the
last 50ms.

We need to replicate this for UI-process scrolling to optimize power. First, move
scrollingThreadIsActive() to be ScrollingTree::hasRecentActivity().

Second, avoid sending `didRefreshDisplay()` to the scrolling thread if there's not
recent activity.

Third, return early from `RemoteLayerTreeEventDispatcher::mainThreadDisplayDidRefresh()`
if there's no recent activity, avoiding a scrolling thread dispatch.

Finally, return early from `RemoteScrollingTreeMac::didCommitTree()` if there are no
scroll animations to start, avoiding another scrolling thread dispatch.

* Source/WebCore/page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::hasRecentActivity):
* Source/WebCore/page/scrolling/ScrollingTree.h:
* Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp:
(WebCore::ThreadedScrollingTree::willStartRenderingUpdate):
(WebCore::ThreadedScrollingTree::displayDidRefresh):
(WebCore::ThreadedScrollingTree::scrollingThreadIsActive): Deleted.
* Source/WebCore/page/scrolling/ThreadedScrollingTree.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.cpp:
(WebKit::RemoteLayerTreeEventDispatcher::scrollingTreeWasRecentlyActive):
(WebKit::RemoteLayerTreeEventDispatcher::mainThreadDisplayDidRefresh):
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeEventDispatcher.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.mm:
(WebKit::RemoteScrollingTreeMac::didCommitTree):

Canonical link: https://commits.webkit.org/264230@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/UI-side-compositing-Significantly-more-scrolling-sync-events branch from 7b498cc to c6eb7d8 Compare May 19, 2023 04:47
@webkit-commit-queue
Copy link
Collaborator

Committed 264230@main (c6eb7d8): https://commits.webkit.org/264230@main

Reviewed commits have been landed. Closing PR #14050 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit c6eb7d8 into WebKit:main May 19, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 19, 2023
webkit-commit-queue pushed a commit to smfr/WebKit that referenced this pull request May 23, 2023
https://bugs.webkit.org/show_bug.cgi?id=257167
rdar://109685883

Unreviewed fix.

Respond to review comment on WebKit#14050, using
a WTF_REQUIRES_LOCK() annotation.

* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.mm:
(WebKit::RemoteScrollingTreeMac::didCommitTree):

Canonical link: https://commits.webkit.org/264405@main
mnutt pushed a commit to movableink/webkit that referenced this pull request Jun 28, 2023
https://bugs.webkit.org/show_bug.cgi?id=257167
rdar://109685883

Unreviewed fix.

Respond to review comment on WebKit/WebKit#14050, using
a WTF_REQUIRES_LOCK() annotation.

* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.h:
* Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteScrollingTreeMac.mm:
(WebKit::RemoteScrollingTreeMac::didCommitTree):

Canonical link: https://commits.webkit.org/264405@main
@smfr smfr deleted the eng/UI-side-compositing-Significantly-more-scrolling-sync-events branch October 1, 2023 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKit Process Model Bugs related to WebKit's multi-process architecture
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants