Attachment 440558 Details for Bug 231403 – Patch

[patch] Patch

bug-231403-20211007173008.patch (text/plain), 12.21 KB, created by Myles C. Maxfield on 2021-10-07 17:30:09 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Myles C. Maxfield

Created: 2021-10-07 17:30:09 PDT

Size: 12.21 KB

patch

obsolete

>Subversion Revision: 283756
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 59209eee000698acdeb40034c8fc0b7fae559832..97413357eeff152c7924fb42939a5c970ad8d6e1 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,39 @@
>+2021-10-07  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        [GPU Process] Unique RenderingResourceIdentifiers Part 2: Unroll PendingWakeupInformation::arguments
>+        https://bugs.webkit.org/show_bug.cgi?id=231403
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We're in the process of replacing RenderingResourceIdentifiers in the GPU process with
>+        QualifiedRenderingResourceIdentifier. Unfortunately, PendingWakeupInformation holds
>+        a GPUProcessWakeupMessageArguments which itself holds a RenderingResourceIdentifiers.
>+        PendingWakeupInformation is part of the GPU process, so it needs to migrate to 
>+        QualifiedRenderingResourceIdentifier, but GPUProcessWakeupMessageArguments is the type
>+        of the arguments to the WakeUpAndApplyDisplayList message, so it can't hold a
>+        QualifiedRenderingResourceIdentifier.
>+
>+        So, the solution is to unroll PendingWakeupInformation::arguments, and hoist its fields
>+        directly into the PendingWakeupInformation. That way, PendingWakeupInformation can be
>+        updated to use QualifiedRenderingResourceIdentifier, without affecting
>+        the WakeUpAndApplyDisplayList message.
>+
>+        No new tests because there is no behavior change.
>+
>+        * GPUProcess/graphics/RemoteRenderingBackend.cpp:
>+        (WebKit::RemoteRenderingBackend::createImageBufferWithQualifiedIdentifier):
>+        (WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
>+        (WebKit::RemoteRenderingBackend::wakeUpAndApplyDisplayList):
>+        (WebKit::RemoteRenderingBackend::setNextItemBufferToRead):
>+        (WebKit::RemoteRenderingBackend::cacheNativeImageWithQualifiedIdentifier):
>+        (WebKit::RemoteRenderingBackend::cacheFontWithQualifiedIdentifier):
>+        (WebKit::RemoteRenderingBackend::finalizeRenderingUpdate):
>+        (WebKit::RemoteRenderingBackend::didCreateSharedDisplayListHandleWithQualifiedIdentifier):
>+        (WebKit::RemoteRenderingBackend::resumeFromPendingWakeupInformation):
>+        * GPUProcess/graphics/RemoteRenderingBackend.h:
>+        (WebKit::RemoteRenderingBackend::PendingWakeupInformation::arguments const):
>+        (WebKit::RemoteRenderingBackend::PendingWakeupInformation::shouldPerformWakeup const):
>+
> 2021-10-07  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         [GPU Process] Unique RenderingResourceIdentifiers Part 1: Uniquify at entry points
>diff --git a/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp b/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp
>index 12455b76ed16d170c3f5a6fd7f90606fc50e58b8..b7f0ff86e6f8e14fff8b528b9199051e8a9e40ea 100644
>--- a/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp
>+++ b/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp
>@@ -189,7 +189,7 @@ void RemoteRenderingBackend::createImageBufferWithQualifiedIdentifier(const Floa
>     updateRenderingResourceRequest();
> 
>     if (m_pendingWakeupInfo && m_pendingWakeupInfo->shouldPerformWakeup(imageBufferResourceIdentifier.object()))
>-        wakeUpAndApplyDisplayList(std::exchange(m_pendingWakeupInfo, std::nullopt)->arguments);
>+        resumeFromPendingWakeupInformation();
> }
> 
> RemoteRenderingBackend::ReplayerDelegate::ReplayerDelegate(WebCore::ImageBuffer& destination, RemoteRenderingBackend& remoteRenderingBackend)
>@@ -281,7 +281,10 @@ RefPtr<ImageBuffer> RemoteRenderingBackend::nextDestinationImageBufferAfterApply
>             if (!destination) {
>                 ASSERT(!m_pendingWakeupInfo);
>                 m_pendingWakeupInfo = {{
>-                    { handle.identifier(), offset, *result.nextDestinationImageBuffer, reason },
>+                    handle.identifier(),
>+                    offset,
>+                    *result.nextDestinationImageBuffer,
>+                    reason,
>                     std::nullopt,
>                     RemoteRenderingBackendState::WaitingForDestinationImageBuffer
>                 }};
>@@ -290,7 +293,10 @@ RefPtr<ImageBuffer> RemoteRenderingBackend::nextDestinationImageBufferAfterApply
> 
>         if (result.reasonForStopping == DisplayList::StopReplayReason::MissingCachedResource) {
>             m_pendingWakeupInfo = {{
>-                { handle.identifier(), offset, destination->renderingResourceIdentifier(), reason },
>+                handle.identifier(),
>+                offset,
>+                destination->renderingResourceIdentifier(),
>+                reason,
>                 result.missingCachedResourceIdentifier,
>                 RemoteRenderingBackendState::WaitingForCachedResource
>             }};
>@@ -346,12 +352,12 @@ void RemoteRenderingBackend::wakeUpAndApplyDisplayList(const GPUProcessWakeupMes
>     LOG_WITH_STREAM(SharedDisplayLists, stream << "Waking up to Items[" << arguments.itemBufferIdentifier << "] => Image(" << arguments.destinationImageBufferIdentifier << ") at " << arguments.offset);
>     destinationImageBuffer = nextDestinationImageBufferAfterApplyingDisplayLists(*destinationImageBuffer, arguments.offset, *initialHandle, arguments.reason);
> 
>-    // FIXME: All the callers pass m_pendingWakeupInfo.arguments so the body of this function should just be this loop.
>+    // FIXME: All the callers pass m_pendingWakeupInfo's fields so the body of this function should just be this loop.
>     while (destinationImageBuffer && m_pendingWakeupInfo) {
>         if (m_pendingWakeupInfo->missingCachedResourceIdentifier)
>             break;
> 
>-        auto nextHandle = m_sharedDisplayListHandles.get(m_pendingWakeupInfo->arguments.itemBufferIdentifier);
>+        auto nextHandle = m_sharedDisplayListHandles.get(m_pendingWakeupInfo->itemBufferIdentifier);
>         if (!nextHandle) {
>             // If the handle identifier is currently unknown, wait until the GPU process receives an
>             // IPC message with a shared memory handle to the next item buffer.
>@@ -359,8 +365,10 @@ void RemoteRenderingBackend::wakeUpAndApplyDisplayList(const GPUProcessWakeupMes
>         }
> 
>         // Otherwise, continue reading the next display list item buffer from the start.
>-        auto arguments = std::exchange(m_pendingWakeupInfo, std::nullopt)->arguments;
>-        destinationImageBuffer = nextDestinationImageBufferAfterApplyingDisplayLists(*destinationImageBuffer, arguments.offset, *nextHandle, arguments.reason);
>+        auto offset = m_pendingWakeupInfo->offset;
>+        auto reason = m_pendingWakeupInfo->reason;
>+        m_pendingWakeupInfo = std::nullopt;
>+        destinationImageBuffer = nextDestinationImageBufferAfterApplyingDisplayLists(*destinationImageBuffer, offset, *nextHandle, reason);
>     }
>     LOG_WITH_STREAM(SharedDisplayLists, stream << "Going back to sleep.");
> 
>@@ -373,7 +381,10 @@ void RemoteRenderingBackend::wakeUpAndApplyDisplayList(const GPUProcessWakeupMes
> void RemoteRenderingBackend::setNextItemBufferToRead(DisplayList::ItemBufferIdentifier identifier, WebCore::RenderingResourceIdentifier destinationIdentifier)
> {
>     m_pendingWakeupInfo = {{
>-        { identifier, SharedDisplayListHandle::headerSize(), destinationIdentifier, GPUProcessWakeupReason::Unspecified },
>+        identifier,
>+        SharedDisplayListHandle::headerSize(),
>+        destinationIdentifier,
>+        GPUProcessWakeupReason::Unspecified,
>         std::nullopt,
>         RemoteRenderingBackendState::WaitingForItemBuffer
>     }};
>@@ -529,7 +540,7 @@ void RemoteRenderingBackend::cacheNativeImageWithQualifiedIdentifier(const Share
>     m_remoteResourceCache.cacheNativeImage(*image);
> 
>     if (m_pendingWakeupInfo && m_pendingWakeupInfo->shouldPerformWakeup(nativeImageResourceIdentifier.object()))
>-        wakeUpAndApplyDisplayList(std::exchange(m_pendingWakeupInfo, std::nullopt)->arguments);
>+        resumeFromPendingWakeupInformation();
> }
> 
> void RemoteRenderingBackend::cacheFont(Ref<WebCore::Font>&& font)
>@@ -546,7 +557,7 @@ void RemoteRenderingBackend::cacheFontWithQualifiedIdentifier(Ref<Font>&& font,
> 
>     m_remoteResourceCache.cacheFont(WTFMove(font));
>     if (m_pendingWakeupInfo && m_pendingWakeupInfo->shouldPerformWakeup(fontResourceIdentifier.object()))
>-        wakeUpAndApplyDisplayList(std::exchange(m_pendingWakeupInfo, std::nullopt)->arguments);
>+        resumeFromPendingWakeupInformation();
> }
> 
> void RemoteRenderingBackend::deleteAllFonts()
>@@ -572,12 +583,12 @@ void RemoteRenderingBackend::releaseRemoteResourceWithQualifiedIdentifier(Qualif
> 
> void RemoteRenderingBackend::finalizeRenderingUpdate(RenderingUpdateID renderingUpdateID)
> {
>-    auto shouldPerformWakeup = [&](const GPUProcessWakeupMessageArguments& arguments) {
>-        return m_remoteResourceCache.cachedImageBuffer(arguments.destinationImageBufferIdentifier) && m_sharedDisplayListHandles.contains(arguments.itemBufferIdentifier);
>+    auto shouldPerformWakeup = [&] {
>+        return m_remoteResourceCache.cachedImageBuffer(m_pendingWakeupInfo->destinationImageBufferIdentifier) && m_sharedDisplayListHandles.contains(m_pendingWakeupInfo->itemBufferIdentifier);
>     };
> 
>-    if (m_pendingWakeupInfo && shouldPerformWakeup(m_pendingWakeupInfo->arguments))
>-        wakeUpAndApplyDisplayList(std::exchange(m_pendingWakeupInfo, std::nullopt)->arguments);
>+    if (m_pendingWakeupInfo && shouldPerformWakeup())
>+        resumeFromPendingWakeupInformation();
> 
>     send(Messages::RemoteRenderingBackendProxy::DidFinalizeRenderingUpdate(renderingUpdateID), m_renderingBackendIdentifier);
> }
>@@ -601,7 +612,14 @@ void RemoteRenderingBackend::didCreateSharedDisplayListHandleWithQualifiedIdenti
>     }
> 
>     if (m_pendingWakeupInfo && m_pendingWakeupInfo->shouldPerformWakeup(itemBufferIdentifier))
>-        wakeUpAndApplyDisplayList(std::exchange(m_pendingWakeupInfo, std::nullopt)->arguments);
>+        resumeFromPendingWakeupInformation();
>+}
>+
>+void RemoteRenderingBackend::resumeFromPendingWakeupInformation()
>+{
>+    auto arguments = m_pendingWakeupInfo->arguments();
>+    m_pendingWakeupInfo = std::nullopt;
>+    wakeUpAndApplyDisplayList(arguments);
> }
> 
> void RemoteRenderingBackend::didCreateMaskImageBuffer(ImageBuffer& imageBuffer)
>diff --git a/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h b/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h
>index ca081160b6544b4ba3513aeeb04c0b45be3d130d..744c9414e524991b80eee5f62079f2ab53a6c117 100644
>--- a/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h
>+++ b/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h
>@@ -160,6 +160,8 @@ private:
>     void releaseRemoteResourceWithQualifiedIdentifier(QualifiedRenderingResourceIdentifier, uint64_t useCount);
>     void cacheFontWithQualifiedIdentifier(Ref<WebCore::Font>&&, QualifiedRenderingResourceIdentifier);
> 
>+    void resumeFromPendingWakeupInformation();
>+
>     class ReplayerDelegate : public WebCore::DisplayList::Replayer::Delegate {
>     public:
>         ReplayerDelegate(WebCore::ImageBuffer&, RemoteRenderingBackend&);
>@@ -175,19 +177,27 @@ private:
>     };
> 
>     struct PendingWakeupInformation {
>-        GPUProcessWakeupMessageArguments arguments;
>+        WebCore::DisplayList::ItemBufferIdentifier itemBufferIdentifier;
>+        uint64_t offset { 0 };
>+        WebCore::RenderingResourceIdentifier destinationImageBufferIdentifier;
>+        GPUProcessWakeupReason reason { GPUProcessWakeupReason::Unspecified };
>         std::optional<WebCore::RenderingResourceIdentifier> missingCachedResourceIdentifier;
>         RemoteRenderingBackendState state { RemoteRenderingBackendState::Initialized };
> 
>+        GPUProcessWakeupMessageArguments arguments() const
>+        {
>+            return { itemBufferIdentifier, offset, destinationImageBufferIdentifier, reason };
>+        }
>+
>         bool shouldPerformWakeup(WebCore::RenderingResourceIdentifier identifier) const
>         {
>-            return arguments.destinationImageBufferIdentifier == identifier
>+            return destinationImageBufferIdentifier == identifier
>                 || missingCachedResourceIdentifier == identifier;
>         }
> 
>         bool shouldPerformWakeup(WebCore::DisplayList::ItemBufferIdentifier identifier) const
>         {
>-            return arguments.itemBufferIdentifier == identifier;
>+            return itemBufferIdentifier == identifier;
>         }
>     };
>

Flags:

wenson_hsieh: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 231403: 440558