- Revision
- 290346
- Author
- [email protected]
- Date
- 2022-02-22 20:28:52 -0800 (Tue, 22 Feb 2022)
Log Message
With DOM Rendering in GPU process, every display results in new IOSurface allocation
https://bugs.webkit.org/show_bug.cgi?id=237063
Reviewed by Tim Horton.
We have to avoid ImageBufferRemoteIOSurfaceBackends in the web process from hanging on to
an IOSurface MachSendRight (via ImageBufferBackendHandle) all the time, because doing so
makes the IOSurface appear to be in-use, and therefore not eligible for swapping to,
or making volatile. In particular, swapToValidFrontBuffer() would always think
that the surfaces were in-use, resulting in permanent triple-buffering, and
allocation of a new buffer on every swap.
Fix by clearing the ImageBufferBackendHandles of all the buffers in the WP in
RemoteRenderingBackendProxy::swapToValidFrontBuffer(), so that the GPUP can accurately
gauge in-use-ness; we get back a handle to the new front buffer in the reply,
and this is the only one we need to keep hold of, since it gets transferred to
the UI process.
* GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::markSurfaceNonVolatile):
(WebKit::RemoteRenderingBackend::swapToValidFrontBuffer):
* Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:
(WebKit::RemoteLayerBackingStore::display):
* WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
(WebKit::RemoteRenderingBackendProxy::swapToValidFrontBuffer):
(WebKit::RemoteRenderingBackendProxy::markSurfaceNonVolatile):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (290345 => 290346)
--- trunk/Source/WebKit/ChangeLog 2022-02-23 04:04:05 UTC (rev 290345)
+++ trunk/Source/WebKit/ChangeLog 2022-02-23 04:28:52 UTC (rev 290346)
@@ -1,5 +1,34 @@
2022-02-22 Simon Fraser <[email protected]>
+ With DOM Rendering in GPU process, every display results in new IOSurface allocation
+ https://bugs.webkit.org/show_bug.cgi?id=237063
+
+ Reviewed by Tim Horton.
+
+ We have to avoid ImageBufferRemoteIOSurfaceBackends in the web process from hanging on to
+ an IOSurface MachSendRight (via ImageBufferBackendHandle) all the time, because doing so
+ makes the IOSurface appear to be in-use, and therefore not eligible for swapping to,
+ or making volatile. In particular, swapToValidFrontBuffer() would always think
+ that the surfaces were in-use, resulting in permanent triple-buffering, and
+ allocation of a new buffer on every swap.
+
+ Fix by clearing the ImageBufferBackendHandles of all the buffers in the WP in
+ RemoteRenderingBackendProxy::swapToValidFrontBuffer(), so that the GPUP can accurately
+ gauge in-use-ness; we get back a handle to the new front buffer in the reply,
+ and this is the only one we need to keep hold of, since it gets transferred to
+ the UI process.
+
+ * GPUProcess/graphics/RemoteRenderingBackend.cpp:
+ (WebKit::RemoteRenderingBackend::markSurfaceNonVolatile):
+ (WebKit::RemoteRenderingBackend::swapToValidFrontBuffer):
+ * Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:
+ (WebKit::RemoteLayerBackingStore::display):
+ * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
+ (WebKit::RemoteRenderingBackendProxy::swapToValidFrontBuffer):
+ (WebKit::RemoteRenderingBackendProxy::markSurfaceNonVolatile):
+
+2022-02-22 Simon Fraser <[email protected]>
+
Use non-inline messages for layer volatility-related IPC in RemoteRenderingBackend
https://bugs.webkit.org/show_bug.cgi?id=237061
Modified: trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp (290345 => 290346)
--- trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2022-02-23 04:04:05 UTC (rev 290345)
+++ trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2022-02-23 04:28:52 UTC (rev 290346)
@@ -400,6 +400,8 @@
void RemoteRenderingBackend::markSurfaceNonVolatile(WebCore::RenderingResourceIdentifier identifier, CompletionHandler<void(std::optional<ImageBufferBackendHandle> backendHandle, bool bufferWasEmpty)>&& completionHandler)
{
+ LOG_WITH_STREAM(RemoteRenderingBufferVolatility, stream << "GPU Process: markSurfaceNonVolatile - " << identifier);
+
auto imageBuffer = m_remoteResourceCache.cachedImageBuffer({ identifier, m_gpuConnectionToWebProcess->webProcessIdentifier() });
if (!imageBuffer) {
completionHandler({ }, false);
@@ -422,6 +424,11 @@
auto backBuffer = fetchBuffer(bufferSet.back);
auto secondaryBackBuffer = fetchBuffer(bufferSet.secondaryBack);
+ LOG_WITH_STREAM(RemoteRenderingBufferVolatility, stream << "GPU Process: RemoteRenderingBackend::swapToValidFrontBuffer - front "
+ << bufferSet.front << " (in-use " << (frontBuffer && frontBuffer->isInUse()) << ") "
+ << bufferSet.back << " (in-use " << (backBuffer && backBuffer->isInUse()) << ") "
+ << bufferSet.secondaryBack << " (in-use " << (secondaryBackBuffer && secondaryBackBuffer->isInUse()) << ") ");
+
if (!backBuffer || backBuffer->isInUse()) {
std::swap(backBuffer, secondaryBackBuffer);
@@ -447,8 +454,14 @@
return std::nullopt;
return buffer->renderingResourceIdentifier();
};
+
+ auto resultBufferSet = BufferIdentifierSet { bufferIdentifer(frontBuffer), bufferIdentifer(backBuffer), bufferIdentifer(secondaryBackBuffer) };
- completionHandler({ bufferIdentifer(frontBuffer), bufferIdentifer(backBuffer), bufferIdentifer(secondaryBackBuffer) }, WTFMove(frontBufferHandle), frontBufferWasEmpty);
+ LOG_WITH_STREAM(RemoteRenderingBufferVolatility, stream << "GPU Process: swapToValidFrontBuffer - swapped from ["
+ << bufferSet.front << ", " << bufferSet.back << ", " << bufferSet.secondaryBack << "] to ["
+ << resultBufferSet.front << ", " << resultBufferSet.back << ", " << resultBufferSet.secondaryBack << "]");
+
+ completionHandler(resultBufferSet, WTFMove(frontBufferHandle), frontBufferWasEmpty);
}
void RemoteRenderingBackend::markSurfacesVolatile(const Vector<WebCore::RenderingResourceIdentifier>& identifiers, CompletionHandler<void(const Vector<WebCore::RenderingResourceIdentifier>& inUseBufferIdentifiers)>&& completionHandler)
Modified: trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm (290345 => 290346)
--- trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm 2022-02-23 04:04:05 UTC (rev 290345)
+++ trunk/Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm 2022-02-23 04:28:52 UTC (rev 290346)
@@ -28,6 +28,7 @@
#import "ArgumentCoders.h"
#import "CGDisplayListImageBufferBackend.h"
+#import "Logging.h"
#import "PlatformCALayerRemote.h"
#import "PlatformRemoteImageBufferProxy.h"
#import "RemoteLayerBackingStoreCollection.h"
@@ -45,6 +46,7 @@
#import <WebCore/WebLayer.h>
#import <pal/spi/cocoa/QuartzCoreSPI.h>
#import <wtf/cocoa/TypeCastsCocoa.h>
+#import <wtf/text/TextStream.h>
#if ENABLE(CG_DISPLAY_LIST_BACKED_IMAGE_BUFFER)
#import <WebKitAdditions/CGDisplayListImageBufferAdditions.h>
@@ -304,11 +306,15 @@
return true;
}
+ LOG_WITH_STREAM(RemoteRenderingBufferVolatility, stream << "RemoteLayerBackingStore::display()");
+
// Make the previous front buffer non-volatile early, so that we can dirty the whole layer if it comes back empty.
collection->makeFrontBufferNonVolatile(*this);
- if (m_dirtyRegion.isEmpty() || m_size.isEmpty())
+ if (m_dirtyRegion.isEmpty() || m_size.isEmpty()) {
+ LOG_WITH_STREAM(RemoteRenderingBufferVolatility, stream << " no dirty region");
return needToEncodeBackingStore;
+ }
WebCore::IntRect layerBounds(WebCore::IntPoint(), WebCore::expandedIntSize(m_size));
if (!hasFrontBuffer() || !supportsPartialRepaint())
Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp (290345 => 290346)
--- trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp 2022-02-23 04:04:05 UTC (rev 290345)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp 2022-02-23 04:28:52 UTC (rev 290346)
@@ -274,16 +274,33 @@
return buffer->renderingResourceIdentifier();
};
+ auto clearBackendHandle = [](ImageBuffer* buffer) {
+ if (!buffer)
+ return;
+
+ if (auto* backend = buffer->ensureBackendCreated()) {
+ auto* sharing = backend->toBackendSharing();
+ if (is<ImageBufferBackendHandleSharing>(sharing))
+ downcast<ImageBufferBackendHandleSharing>(*sharing).clearBackendHandle();
+ }
+ };
+
+ // Clear all the buffer's MachSendRights to avoid all the surfaces appearing to be in-use.
+ // We get back the new front buffer's MachSendRight in the reply.
+ clearBackendHandle(buffers.front.get());
+ clearBackendHandle(buffers.back.get());
+ clearBackendHandle(buffers.secondaryBack.get());
+
auto bufferSet = BufferIdentifierSet {
bufferIdentifier(buffers.front.get()),
bufferIdentifier(buffers.back.get()),
bufferIdentifier(buffers.secondaryBack.get())
};
-
+
BufferIdentifierSet swappedBufferSet;
std::optional<ImageBufferBackendHandle> frontBufferHandle;
bool frontBufferWasEmpty = false;
-
+
sendSyncToStream(Messages::RemoteRenderingBackend::SwapToValidFrontBuffer(bufferSet),
Messages::RemoteRenderingBackend::SwapToValidFrontBuffer::Reply(swappedBufferSet, frontBufferHandle, frontBufferWasEmpty));
@@ -318,6 +335,8 @@
VolatilityState RemoteRenderingBackendProxy::markSurfaceNonVolatile(RenderingResourceIdentifier identifier)
{
+ LOG_WITH_STREAM(RemoteRenderingBufferVolatility, stream << "Web Process: RemoteRenderingBackendProxy::markSurfaceNonVolatile " << identifier);
+
bool bufferWasEmpty;
std::optional<ImageBufferBackendHandle> backendHandle;
sendSyncToStream(Messages::RemoteRenderingBackend::MarkSurfaceNonVolatile(identifier), Messages::RemoteRenderingBackend::MarkSurfaceNonVolatile::Reply(backendHandle, bufferWasEmpty));
