Skip to content

Conversation

carlosgcampos
Copy link
Contributor

@carlosgcampos carlosgcampos commented Jul 22, 2024

18a0d32

[GTK][WPE] Constant high idle CPU usage in front of any logged in page on Trello.com
https://bugs.webkit.org/show_bug.cgi?id=252545

Reviewed by Miguel Gomez.

Do not schedule a composition if layers haven't changed and it's not a
force repaint. To know whether platform layer changed, we need to also
track the updates during layer flush and add a way to check the status.
This patch merges updateContentBuffersIncludingSubLayers() and
checkPendingStateChanges() into finalizeCompositingStateFlush() to
upadate the backing stores and check the frame sync and platform updates
in a single layer iteration.

* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
(WebCore::CoordinatedGraphicsLayer::updatePlatformLayer):
(WebCore::CoordinatedGraphicsLayer::checkContentLayerUpdated):
(WebCore::CoordinatedGraphicsLayer::checkPendingStateChanges):
(WebCore::CoordinatedGraphicsLayer::finalizeCompositingStateFlush):
(WebCore::CoordinatedGraphicsLayer::checkPendingStateChangesIncludingSubLayers): Deleted.
(WebCore::CoordinatedGraphicsLayer::updateContentBuffersIncludingSubLayers): Deleted.
* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:
* Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp:
(WebKit::CompositingCoordinator::flushPendingLayerChanges):
* Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h:
* Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/LayerTreeHost.cpp:
(WebKit::LayerTreeHost::layerFlushTimerFired):
(WebKit::LayerTreeHost::forceRepaint):
(WebKit::LayerTreeHost::renderNextFrame):

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

bfcb16f

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

@carlosgcampos carlosgcampos self-assigned this Jul 22, 2024
@carlosgcampos carlosgcampos added the WebKitGTK Bugs related to the Gtk API layer. label Jul 22, 2024
@magomez
Copy link
Contributor

magomez commented Jul 24, 2024

The change for swapBuffersIfNeeded to return a bool is not really needed, and it makes the patch more complicated than it should. Can you keep it returning void?
In general we assume that a call to swapBuffersIfNeeded is going to push a buffer, which is going to be true in most of the cases. For the cases that it's not true, worst case scenario we're requesting a composition that is not needed, which is not a big deal.

@carlosgcampos carlosgcampos added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 24, 2024
…e on Trello.com

https://bugs.webkit.org/show_bug.cgi?id=252545

Reviewed by Miguel Gomez.

Do not schedule a composition if layers haven't changed and it's not a
force repaint. To know whether platform layer changed, we need to also
track the updates during layer flush and add a way to check the status.
This patch merges updateContentBuffersIncludingSubLayers() and
checkPendingStateChanges() into finalizeCompositingStateFlush() to
upadate the backing stores and check the frame sync and platform updates
in a single layer iteration.

* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
(WebCore::CoordinatedGraphicsLayer::updatePlatformLayer):
(WebCore::CoordinatedGraphicsLayer::checkContentLayerUpdated):
(WebCore::CoordinatedGraphicsLayer::checkPendingStateChanges):
(WebCore::CoordinatedGraphicsLayer::finalizeCompositingStateFlush):
(WebCore::CoordinatedGraphicsLayer::checkPendingStateChangesIncludingSubLayers): Deleted.
(WebCore::CoordinatedGraphicsLayer::updateContentBuffersIncludingSubLayers): Deleted.
* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:
* Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp:
(WebKit::CompositingCoordinator::flushPendingLayerChanges):
* Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h:
* Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/LayerTreeHost.cpp:
(WebKit::LayerTreeHost::layerFlushTimerFired):
(WebKit::LayerTreeHost::forceRepaint):
(WebKit::LayerTreeHost::renderNextFrame):

Canonical link: https://commits.webkit.org/281288@main
@webkit-commit-queue
Copy link
Collaborator

Committed 281288@main (18a0d32): https://commits.webkit.org/281288@main

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

@webkit-commit-queue webkit-commit-queue merged commit 18a0d32 into WebKit:main Jul 24, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 24, 2024
@carlosgcampos carlosgcampos deleted the wk-no-frame-sync branch July 24, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebKitGTK Bugs related to the Gtk API layer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants