Skip to content
Permalink
Browse files
[GPUProcess] Stop using a synchronous IPC for marking layers as volatile
https://bugs.webkit.org/show_bug.cgi?id=239901
<rdar://91663593>

Reviewed by Geoffrey Garen.

Stop using a synchronous IPC to the GPUProcess for marking layers as volatile. Instead,
use an asynchronous IPC for the request and another asynchronous one for the response.
Ideally, we'd be using a sendWithAsyncReply(), like the FIXME comment in the code
suggested. However, this is not supported yet by our new "streaming" IPC.

* Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::markSurfacesVolatile):
* Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h:
* Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.messages.in:
* Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
(WebKit::RemoteRenderingBackendProxy::~RemoteRenderingBackendProxy):
(WebKit::RemoteRenderingBackendProxy::markSurfacesVolatile):
(WebKit::RemoteRenderingBackendProxy::didMarkLayersAsVolatile):
* Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.h:
* Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.messages.in:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::tryMarkLayersVolatileCompletionHandler):

Canonical link: https://commits.webkit.org/250140@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293636 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
cdumez committed Apr 29, 2022
1 parent aca90d5 commit 9af21f722a464f3d23107a6aeb8efdb27d47c28e
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 11 deletions.
@@ -496,7 +496,7 @@ void RemoteRenderingBackend::prepareLayerBuffersForDisplay(const PrepareBackingS
outputData.displayRequirement = needsFullDisplay ? SwapBuffersDisplayRequirement::NeedsFullDisplay : SwapBuffersDisplayRequirement::NeedsNormalDisplay;
}

void RemoteRenderingBackend::markSurfacesVolatile(const Vector<RenderingResourceIdentifier>& identifiers, CompletionHandler<void(const Vector<RenderingResourceIdentifier>& markedVolatileBufferIdentifiers)>&& completionHandler)
void RemoteRenderingBackend::markSurfacesVolatile(uint64_t requestIdentifier, const Vector<RenderingResourceIdentifier>& identifiers)
{
LOG_WITH_STREAM(RemoteRenderingBufferVolatility, stream << "GPU Process: RemoteRenderingBackend::markSurfacesVolatile " << identifiers);

@@ -517,7 +517,8 @@ void RemoteRenderingBackend::markSurfacesVolatile(const Vector<RenderingResource

LOG_WITH_STREAM(RemoteRenderingBufferVolatility, stream << "GPU Process: markSurfacesVolatile - surfaces marked volatile " << markedVolatileBufferIdentifiers);

completionHandler(markedVolatileBufferIdentifiers);
bool didMarkAllLayerAsVolatile = identifiers.size() == markedVolatileBufferIdentifiers.size();
send(Messages::RemoteRenderingBackendProxy::DidMarkLayersAsVolatile(requestIdentifier, markedVolatileBufferIdentifiers, didMarkAllLayerAsVolatile), m_renderingBackendIdentifier);
}

void RemoteRenderingBackend::finalizeRenderingUpdate(RenderingUpdateID renderingUpdateID)
@@ -128,7 +128,7 @@ class RemoteRenderingBackend : private IPC::MessageSender, public IPC::StreamMes
void deleteAllFonts();
void releaseRemoteResource(WebCore::RenderingResourceIdentifier);
void finalizeRenderingUpdate(RenderingUpdateID);
void markSurfacesVolatile(const Vector<WebCore::RenderingResourceIdentifier>&, CompletionHandler<void(const Vector<WebCore::RenderingResourceIdentifier>& markedVolatileBufferIdentifiers)>&&);
void markSurfacesVolatile(uint64_t requestIdentifier, const Vector<WebCore::RenderingResourceIdentifier>&);
Copy link
@smfr

smfr May 2, 2022

Contributor

I would have preferred an ObjectIdentifier<> here. There are so many uint64_t identifiers, they are easy to get mixed up.

Copy link
@cdumez

cdumez May 2, 2022

Author Contributor

Ok, I'll follow up.

void prepareBuffersForDisplay(Vector<PrepareBackingStoreBuffersInputData> swapBuffersInput, CompletionHandler<void(const Vector<PrepareBackingStoreBuffersOutputData>&)>&&);

// Received messages translated to use QualifiedRenderingResourceIdentifier.
@@ -39,8 +39,7 @@ messages -> RemoteRenderingBackend NotRefCounted Stream {

PrepareBuffersForDisplay(Vector<WebKit::PrepareBackingStoreBuffersInputData> swapBuffersInput) -> (Vector<WebKit::PrepareBackingStoreBuffersOutputData> swapBuffersOutput) Synchronous NotStreamEncodable NotStreamEncodableReply

MarkSurfacesVolatile(Vector<WebCore::RenderingResourceIdentifier> renderingResourceIdentifiers) -> (Vector<WebCore::RenderingResourceIdentifier> markedVolatileBufferIdentifiers) Synchronous

MarkSurfacesVolatile(uint64_t requestIdentifier, Vector<WebCore::RenderingResourceIdentifier> renderingResourceIdentifiers)
FinalizeRenderingUpdate(WebKit::RenderingUpdateID renderingUpdateID)
}

@@ -61,6 +61,9 @@ RemoteRenderingBackendProxy::RemoteRenderingBackendProxy(WebPage& webPage)

RemoteRenderingBackendProxy::~RemoteRenderingBackendProxy()
{
for (auto& markAsVolatileHandlers : m_markAsVolatileRequests.values())
markAsVolatileHandlers(false);

if (!m_gpuProcessConnection)
return;
m_gpuProcessConnection->connection().send(Messages::GPUConnectionToWebProcess::ReleaseRenderingBackend(renderingBackendIdentifier()), 0, IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
@@ -356,18 +359,26 @@ auto RemoteRenderingBackendProxy::prepareBuffersForDisplay(const Vector<LayerPre

void RemoteRenderingBackendProxy::markSurfacesVolatile(Vector<WebCore::RenderingResourceIdentifier>&& identifiers, CompletionHandler<void(bool)>&& completionHandler)
{
Vector<WebCore::RenderingResourceIdentifier> markedVolatileBufferIdentifiers;
// FIXME: This could become async when webkit.org/b/235965 is fixed (but be careful with buffer volatility state).
sendSyncToStream(Messages::RemoteRenderingBackend::MarkSurfacesVolatile(identifiers), Messages::RemoteRenderingBackend::MarkSurfacesVolatile::Reply(markedVolatileBufferIdentifiers));
static uint64_t lastRequestIdentifier = 0;
auto requestIdentifier = ++lastRequestIdentifier;
m_markAsVolatileRequests.add(requestIdentifier, WTFMove(completionHandler));

sendToStream(Messages::RemoteRenderingBackend::MarkSurfacesVolatile(requestIdentifier, identifiers));
}

void RemoteRenderingBackendProxy::didMarkLayersAsVolatile(uint64_t requestIdentifier, const Vector<WebCore::RenderingResourceIdentifier>& markedVolatileBufferIdentifiers, bool didMarkAllLayersAsVolatile)
{
ASSERT(requestIdentifier);
auto completionHandler = m_markAsVolatileRequests.take(requestIdentifier);
if (!completionHandler)
return;

for (auto identifier : markedVolatileBufferIdentifiers) {
auto imageBuffer = m_remoteResourceCacheProxy.cachedImageBuffer(identifier);
if (imageBuffer)
imageBuffer->setVolatilityState(WebCore::VolatilityState::Volatile);
}

bool markedAllVolatile = identifiers.size() == markedVolatileBufferIdentifiers.size();
completionHandler(markedAllVolatile);
completionHandler(didMarkAllLayersAsVolatile);
}

void RemoteRenderingBackendProxy::finalizeRenderingUpdate()
@@ -43,6 +43,7 @@
#include <WebCore/RenderingResourceIdentifier.h>
#include <WebCore/Timer.h>
#include <wtf/Deque.h>
#include <wtf/HashMap.h>
#include <wtf/Span.h>
#include <wtf/WeakPtr.h>

@@ -171,6 +172,7 @@ class RemoteRenderingBackendProxy
void didCreateImageBufferBackend(ImageBufferBackendHandle, WebCore::RenderingResourceIdentifier);
void didFlush(WebCore::GraphicsContextFlushIdentifier, WebCore::RenderingResourceIdentifier);
void didFinalizeRenderingUpdate(RenderingUpdateID didRenderingUpdateID);
void didMarkLayersAsVolatile(uint64_t requestIdentifier, const Vector<WebCore::RenderingResourceIdentifier>& markedVolatileBufferIdentifiers, bool didMarkAllLayerAsVolatile);

GPUProcessConnection* m_gpuProcessConnection { nullptr };
RefPtr<IPC::Connection> m_connection;
@@ -179,6 +181,7 @@ class RemoteRenderingBackendProxy
RemoteResourceCacheProxy m_remoteResourceCacheProxy { *this };
RefPtr<SharedMemory> m_getPixelBufferSharedMemory;
WebCore::Timer m_destroyGetPixelBufferSharedMemoryTimer { *this, &RemoteRenderingBackendProxy::destroyGetPixelBufferSharedMemory };
HashMap<uint64_t, CompletionHandler<void(bool)>> m_markAsVolatileRequests;

RenderingUpdateID m_renderingUpdateID;
RenderingUpdateID m_didRenderingUpdateID;
@@ -27,6 +27,7 @@ messages -> RemoteRenderingBackendProxy NotRefCounted {
DidFlush(WebCore::GraphicsContextFlushIdentifier flushIdentifier, WebCore::RenderingResourceIdentifier renderingResourceIdentifier)
DidFinalizeRenderingUpdate(WebKit::RenderingUpdateID didRenderingUpdateID)
DidCreateWakeUpSemaphoreForDisplayListStream(IPC::Semaphore wakeUpSemaphore)
DidMarkLayersAsVolatile(uint64_t requestIdentifier, Vector<WebCore::RenderingResourceIdentifier> markedVolatileBufferIdentifiers, bool didMarkAllLayersAsVolatile)
}

#endif // ENABLE(GPU_PROCESS)
@@ -2962,6 +2962,11 @@ void WebPage::tryMarkLayersVolatileCompletionHandler(MarkLayersVolatileDontRetry
return;
}

if (m_markLayersAsVolatileCompletionHandlers.isEmpty()) {
WEBPAGE_RELEASE_LOG(Layers, "markLayersVolatile: Failed to mark all layers as volatile, but will not retry because the operation was cancelled");
return;
}

WEBPAGE_RELEASE_LOG(Layers, "markLayersVolatile: Failed to mark all layers as volatile, will retry in %g ms", m_layerVolatilityTimerInterval.milliseconds());
m_layerVolatilityTimer.startOneShot(m_layerVolatilityTimerInterval);
}

0 comments on commit 9af21f7

Please sign in to comment.