Changeset 269969 in webkit

Timestamp:

Nov 18, 2020 10:42:36 AM (3 years ago)

Author:

Wenson Hsieh

Message:

Clean up some code in SharedDisplayListHandle
​https://bugs.webkit.org/show_bug.cgi?id=219089

Reviewed by Geoff Garen.

Currently, reservedCapacityAtStart is defined as a constant 16 bytes, which is enough to encompass the
contents of the header structure in a shared display list handle (i.e. an 8-byte atomic for the lock, and
another 8 bytes for the unread count).

Instead of hard-coding this, we could simply make this a constexpr function that returns the size of
DisplayListSharedMemoryHeader (rounded up to ensure alignment of all display list item data).

No change in behavior.

• GPUProcess/graphics/RemoteRenderingBackend.cpp:

(WebKit::RemoteRenderingBackend::wakeUpAndApplyDisplayList):
(WebKit::RemoteRenderingBackend::didCreateSharedDisplayListHandle):

• Shared/SharedDisplayListHandle.h:

(WebKit::SharedDisplayListHandle::headerSize):

• WebProcess/GPU/graphics/DisplayListWriterHandle.cpp:

(WebKit::DisplayListWriterHandle::resetWritableOffsetIfPossible):

• WebProcess/GPU/graphics/DisplayListWriterHandle.h:

(WebKit::DisplayListWriterHandle::DisplayListWriterHandle):

• WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:

(WebKit::RemoteRenderingBackendProxy::createItemBuffer):

Also add a static assert that the size of a newly allocated buffer is larger than the reserved header capacity.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• GPUProcess/graphics/RemoteRenderingBackend.cpp (modified) (2 diffs)
• Shared/SharedDisplayListHandle.h (modified) (1 diff)
• WebProcess/GPU/graphics/DisplayListWriterHandle.cpp (modified) (2 diffs)
• WebProcess/GPU/graphics/DisplayListWriterHandle.h (modified) (1 diff)
• WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-11-18  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Clean up some code in SharedDisplayListHandle
+        https://bugs.webkit.org/show_bug.cgi?id=219089
+
+        Reviewed by Geoff Garen.
+
+        Currently, `reservedCapacityAtStart` is defined as a constant 16 bytes, which is enough to encompass the
+        contents of the header structure in a shared display list handle (i.e. an 8-byte atomic for the lock, and
+        another 8 bytes for the unread count).
+
+        Instead of hard-coding this, we could simply make this a constexpr function that returns the size of
+        `DisplayListSharedMemoryHeader` (rounded up to ensure alignment of all display list item data).
+
+        No change in behavior.
+
+        * GPUProcess/graphics/RemoteRenderingBackend.cpp:
+        (WebKit::RemoteRenderingBackend::wakeUpAndApplyDisplayList):
+        (WebKit::RemoteRenderingBackend::didCreateSharedDisplayListHandle):
+        * Shared/SharedDisplayListHandle.h:
+        (WebKit::SharedDisplayListHandle::headerSize):
+        * WebProcess/GPU/graphics/DisplayListWriterHandle.cpp:
+        (WebKit::DisplayListWriterHandle::resetWritableOffsetIfPossible):
+        * WebProcess/GPU/graphics/DisplayListWriterHandle.h:
+        (WebKit::DisplayListWriterHandle::DisplayListWriterHandle):
+        * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
+        (WebKit::RemoteRenderingBackendProxy::createItemBuffer):
+
+        Also add a static assert that the size of a newly allocated buffer is larger than the reserved header capacity.
+
 2020-11-18  Per Arne Vollan  <pvollan@apple.com>
 

trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp

@@ -199 +199 @@
         // Otherwise, continue reading the next display list item buffer from the start.
         m_nextItemBufferToRead = { };
-        applyDisplayListsFromHandle(*imageBuffer, *nextHandle, SharedDisplayListHandle::reservedCapacityAtStart);
+        applyDisplayListsFromHandle(*imageBuffer, *nextHandle, SharedDisplayListHandle::headerSize());
     }
 }
@@ -244 +244 @@
     if (m_nextItemBufferToRead == identifier) {
         m_nextItemBufferToRead = { };
-        wakeUpAndApplyDisplayList(identifier, SharedDisplayListHandle::reservedCapacityAtStart, destinationBufferIdentifier);
+        wakeUpAndApplyDisplayList(identifier, SharedDisplayListHandle::headerSize(), destinationBufferIdentifier);
     }
 }

trunk/Source/WebKit/Shared/SharedDisplayListHandle.h

@@ -38 +38 @@
     virtual ~SharedDisplayListHandle() = default;
 
-    static constexpr auto reservedCapacityAtStart = 2 * sizeof(uint64_t);
+    static constexpr size_t headerSize()
+    {
+        return roundUpToMultipleOf<sizeof(std::max_align_t)>(sizeof(DisplayListSharedMemoryHeader));
+    }
 
     SharedMemory& sharedMemory() { return m_sharedMemory.get(); }

trunk/Source/WebKit/WebProcess/GPU/graphics/DisplayListWriterHandle.cpp

@@ -69 +69 @@
 bool DisplayListWriterHandle::resetWritableOffsetIfPossible()
 {
-    if (m_writableOffset <= SharedDisplayListHandle::reservedCapacityAtStart) {
-        RELEASE_ASSERT(m_writableOffset == SharedDisplayListHandle::reservedCapacityAtStart);
+    if (m_writableOffset <= SharedDisplayListHandle::headerSize()) {
+        RELEASE_ASSERT(m_writableOffset == SharedDisplayListHandle::headerSize());
         return true;
     }
@@ -77 +77 @@
         return false;
 
-    m_writableOffset = SharedDisplayListHandle::reservedCapacityAtStart;
+    m_writableOffset = SharedDisplayListHandle::headerSize();
     return true;
 }

trunk/Source/WebKit/WebProcess/GPU/graphics/DisplayListWriterHandle.h

@@ -50 +50 @@
     DisplayListWriterHandle(WebCore::DisplayList::ItemBufferIdentifier identifier, Ref<SharedMemory>&& sharedMemory)
         : SharedDisplayListHandle(identifier, WTFMove(sharedMemory))
-        , m_writableOffset(SharedDisplayListHandle::reservedCapacityAtStart)
+        , m_writableOffset(SharedDisplayListHandle::headerSize())
     {
     }

trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp

@@ -240 +240 @@
 
     static constexpr size_t defaultSharedItemBufferSize = 1 << 16;
-
-    auto sharedMemory = SharedMemory::allocate(std::max(defaultSharedItemBufferSize, capacity + SharedDisplayListHandle::reservedCapacityAtStart));
+    static_assert(defaultSharedItemBufferSize > SharedDisplayListHandle::headerSize());
+
+    auto sharedMemory = SharedMemory::allocate(std::max(defaultSharedItemBufferSize, capacity + SharedDisplayListHandle::headerSize()));
     if (!sharedMemory)
         return { };