Skip to content

Commit

Permalink
Reduce memory usage when sending SharedBuffer across IPC
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=248301
rdar://102201837

Pack small SharedBuffers across a single allocated SharedMemory.
A threshold of 4kB has been chosen through educated guesses.
Have URLSchemeTaskDidReceiveData use SharedBuffer when sending data to the content
process rather than a SharedBufferReference. The primary use of a SharedBufferReference
is to keep ownership of the allocated memory to the sending process. There's no
such requirement here.

Fly-by improvement, only perform a single memory allocation when passing a FragmentedShared made of multiple segments
on GTK and WPE platfom.

Reviewed by Kimmo Kinnunen.

* Source/WebKit/Shared/WebCoreArgumentCoders.cpp:
(IPC::useUnixDomainSockets):
(IPC::ArgumentCoder<WebCore::FragmentedSharedBuffer>::encode):
(IPC::ArgumentCoder<WebCore::FragmentedSharedBuffer>::decode):
* Source/WebKit/UIProcess/WebURLSchemeTask.cpp:
(WebKit::WebURLSchemeTask::didReceiveData):
* Source/WebKit/UIProcess/WebURLSchemeTask.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::urlSchemeTaskDidReceiveData):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:
* Source/WebKit/WebProcess/WebPage/WebURLSchemeHandlerProxy.cpp:
(WebKit::WebURLSchemeHandlerProxy::taskDidReceiveData):
* Source/WebKit/WebProcess/WebPage/WebURLSchemeHandlerProxy.h:

Canonical link: https://commits.webkit.org/256999@main
  • Loading branch information
jyavenard committed Nov 24, 2022
1 parent a9510ae commit 80bf5c2
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 35 deletions.
62 changes: 37 additions & 25 deletions Source/WebKit/Shared/WebCoreArgumentCoders.cpp
Expand Up @@ -1806,29 +1806,41 @@ std::optional<SerializedPlatformDataCueValue> ArgumentCoder<WebCore::SerializedP
}
#endif

constexpr bool useUnixDomainSockets()
{
#if USE(UNIX_DOMAIN_SOCKETS)
return true;
#else
return false;
#endif
}

static constexpr size_t minimumPageSize = 4096;

void ArgumentCoder<WebCore::FragmentedSharedBuffer>::encode(Encoder& encoder, const WebCore::FragmentedSharedBuffer& buffer)
{
uint64_t bufferSize = buffer.size();
encoder << bufferSize;
if (!bufferSize)
return;

#if USE(UNIX_DOMAIN_SOCKETS)
// Do not use shared memory for FragmentedSharedBuffer encoding in Unix, because it's easy to reach the
// maximum number of file descriptors open per process when sending large data in small chunks
// over the IPC. ConnectionUnix.cpp already uses shared memory to send any IPC message that is
// too large. See https://bugs.webkit.org/show_bug.cgi?id=208571.
for (const auto& element : buffer)
encoder.encodeFixedLengthData(element.segment->data(), element.segment->size(), 1);
#else
SharedMemory::Handle handle;
{
auto sharedMemoryBuffer = SharedMemory::copyBuffer(buffer);
if (auto memoryHandle = sharedMemoryBuffer->createHandle(SharedMemory::Protection::ReadOnly))
handle = WTFMove(*memoryHandle);
if (useUnixDomainSockets() || bufferSize < minimumPageSize) {
encoder.reserve(encoder.bufferSize() + bufferSize);
// Do not use shared memory for FragmentedSharedBuffer encoding in Unix, because it's easy to reach the
// maximum number of file descriptors open per process when sending large data in small chunks
// over the IPC. ConnectionUnix.cpp already uses shared memory to send any IPC message that is
// too large. See https://bugs.webkit.org/show_bug.cgi?id=208571.
for (const auto& element : buffer)
encoder.encodeFixedLengthData(element.segment->data(), element.segment->size(), 1);
} else {
SharedMemory::Handle handle;
{
auto sharedMemoryBuffer = SharedMemory::copyBuffer(buffer);
if (auto memoryHandle = sharedMemoryBuffer->createHandle(SharedMemory::Protection::ReadOnly))
handle = WTFMove(*memoryHandle);
}
encoder << WTFMove(handle);
}
encoder << WTFMove(handle);
#endif
}

std::optional<Ref<WebCore::FragmentedSharedBuffer>> ArgumentCoder<WebCore::FragmentedSharedBuffer>::decode(Decoder& decoder)
Expand All @@ -1840,17 +1852,18 @@ std::optional<Ref<WebCore::FragmentedSharedBuffer>> ArgumentCoder<WebCore::Fragm
if (!bufferSize)
return SharedBuffer::create();

#if USE(UNIX_DOMAIN_SOCKETS)
if (!decoder.bufferIsLargeEnoughToContain<uint8_t>(bufferSize))
return std::nullopt;
if (useUnixDomainSockets() || bufferSize < minimumPageSize) {
if (!decoder.bufferIsLargeEnoughToContain<uint8_t>(bufferSize))
return std::nullopt;

Vector<uint8_t> data;
data.grow(bufferSize);
if (!decoder.decodeFixedLengthData(data.data(), data.size(), 1))
return std::nullopt;
Vector<uint8_t> data;
data.grow(bufferSize);
if (!decoder.decodeFixedLengthData(data.data(), data.size(), 1))
return std::nullopt;

return SharedBuffer::create(WTFMove(data));
}

return SharedBuffer::create(WTFMove(data));
#else
SharedMemory::Handle handle;
if (!decoder.decode(handle))
return std::nullopt;
Expand All @@ -1863,7 +1876,6 @@ std::optional<Ref<WebCore::FragmentedSharedBuffer>> ArgumentCoder<WebCore::Fragm
return std::nullopt;

return SharedBuffer::create(static_cast<unsigned char*>(sharedMemoryBuffer->data()), bufferSize);
#endif
}

void ArgumentCoder<WebCore::SharedBuffer>::encode(Encoder& encoder, const WebCore::SharedBuffer& buffer)
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/WebURLSchemeTask.cpp
Expand Up @@ -167,7 +167,7 @@ auto WebURLSchemeTask::didReceiveResponse(const ResourceResponse& response) -> E
return ExceptionType::None;
}

auto WebURLSchemeTask::didReceiveData(const SharedBuffer& buffer) -> ExceptionType
auto WebURLSchemeTask::didReceiveData(Ref<SharedBuffer>&& buffer) -> ExceptionType
{
ASSERT(RunLoop::isMain());

Expand All @@ -190,7 +190,7 @@ auto WebURLSchemeTask::didReceiveData(const SharedBuffer& buffer) -> ExceptionTy
return ExceptionType::None;
}

m_process->send(Messages::WebPage::URLSchemeTaskDidReceiveData(m_urlSchemeHandler->identifier(), m_resourceLoaderID, IPC::SharedBufferReference(buffer)), m_webPageID);
m_process->send(Messages::WebPage::URLSchemeTaskDidReceiveData(m_urlSchemeHandler->identifier(), m_resourceLoaderID, WTFMove(buffer)), m_webPageID);
return ExceptionType::None;
}

Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/WebURLSchemeTask.h
Expand Up @@ -83,7 +83,7 @@ class WebURLSchemeTask : public API::ObjectImpl<API::Object::Type::URLSchemeTask
ExceptionType willPerformRedirection(WebCore::ResourceResponse&&, WebCore::ResourceRequest&&, Function<void(WebCore::ResourceRequest&&)>&&);
ExceptionType didPerformRedirection(WebCore::ResourceResponse&&, WebCore::ResourceRequest&&);
ExceptionType didReceiveResponse(const WebCore::ResourceResponse&);
ExceptionType didReceiveData(const WebCore::SharedBuffer&);
ExceptionType didReceiveData(Ref<WebCore::SharedBuffer>&&);
ExceptionType didComplete(const WebCore::ResourceError&);

void stop();
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/WebProcess/WebPage/WebPage.cpp
Expand Up @@ -7406,12 +7406,12 @@ void WebPage::urlSchemeTaskDidReceiveResponse(WebURLSchemeHandlerIdentifier hand
handler->taskDidReceiveResponse(taskIdentifier, response);
}

void WebPage::urlSchemeTaskDidReceiveData(WebURLSchemeHandlerIdentifier handlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, const IPC::SharedBufferReference& data)
void WebPage::urlSchemeTaskDidReceiveData(WebURLSchemeHandlerIdentifier handlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, Ref<WebCore::SharedBuffer>&& data)
{
auto* handler = m_identifierToURLSchemeHandlerProxyMap.get(handlerIdentifier);
ASSERT(handler);

handler->taskDidReceiveData(taskIdentifier, data.isNull() ? WebCore::SharedBuffer::create() : data.unsafeBuffer().releaseNonNull());
handler->taskDidReceiveData(taskIdentifier, WTFMove(data));
}

void WebPage::urlSchemeTaskDidComplete(WebURLSchemeHandlerIdentifier handlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, const ResourceError& error)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/WebPage/WebPage.h
Expand Up @@ -2017,7 +2017,7 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
void urlSchemeTaskWillPerformRedirection(WebURLSchemeHandlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, WebCore::ResourceResponse&&, WebCore::ResourceRequest&&, CompletionHandler<void(WebCore::ResourceRequest&&)>&&);
void urlSchemeTaskDidPerformRedirection(WebURLSchemeHandlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, WebCore::ResourceResponse&&, WebCore::ResourceRequest&&);
void urlSchemeTaskDidReceiveResponse(WebURLSchemeHandlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, const WebCore::ResourceResponse&);
void urlSchemeTaskDidReceiveData(WebURLSchemeHandlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, const IPC::SharedBufferReference&);
void urlSchemeTaskDidReceiveData(WebURLSchemeHandlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, Ref<WebCore::SharedBuffer>&&);
void urlSchemeTaskDidComplete(WebURLSchemeHandlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, const WebCore::ResourceError&);

void setIsTakingSnapshotsForApplicationSuspension(bool);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Expand Up @@ -605,7 +605,7 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
URLSchemeTaskWillPerformRedirection(WebKit::WebURLSchemeHandlerIdentifier handlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, WebCore::ResourceResponse response, WebCore::ResourceRequest proposedRequest) -> (WebCore::ResourceRequest actualRequest)
URLSchemeTaskDidPerformRedirection(WebKit::WebURLSchemeHandlerIdentifier handlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, WebCore::ResourceResponse response, WebCore::ResourceRequest proposedRequest)
URLSchemeTaskDidReceiveResponse(WebKit::WebURLSchemeHandlerIdentifier handlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, WebCore::ResourceResponse response)
URLSchemeTaskDidReceiveData(WebKit::WebURLSchemeHandlerIdentifier handlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, IPC::SharedBufferReference data)
URLSchemeTaskDidReceiveData(WebKit::WebURLSchemeHandlerIdentifier handlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, Ref<WebCore::SharedBuffer> data)
URLSchemeTaskDidComplete(WebKit::WebURLSchemeHandlerIdentifier handlerIdentifier, WebCore::ResourceLoaderIdentifier taskIdentifier, WebCore::ResourceError error)

SetIsSuspended(bool suspended)
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/WebProcess/WebPage/WebURLSchemeHandlerProxy.cpp
Expand Up @@ -97,13 +97,13 @@ void WebURLSchemeHandlerProxy::taskDidReceiveResponse(WebCore::ResourceLoaderIde
task->didReceiveResponse(response);
}

void WebURLSchemeHandlerProxy::taskDidReceiveData(WebCore::ResourceLoaderIdentifier taskIdentifier, const SharedBuffer& data)
void WebURLSchemeHandlerProxy::taskDidReceiveData(WebCore::ResourceLoaderIdentifier taskIdentifier, Ref<WebCore::SharedBuffer>&& data)
{
auto* task = m_tasks.get(taskIdentifier);
if (!task)
return;

task->didReceiveData(data);
task->didReceiveData(WTFMove(data));
}

void WebURLSchemeHandlerProxy::taskDidComplete(WebCore::ResourceLoaderIdentifier taskIdentifier, const ResourceError& error)
Expand Down
Expand Up @@ -60,7 +60,7 @@ class WebURLSchemeHandlerProxy : public RefCounted<WebURLSchemeHandlerProxy> {

void taskDidPerformRedirection(WebCore::ResourceLoaderIdentifier, WebCore::ResourceResponse&&, WebCore::ResourceRequest&&, CompletionHandler<void(WebCore::ResourceRequest&&)>&&);
void taskDidReceiveResponse(WebCore::ResourceLoaderIdentifier, const WebCore::ResourceResponse&);
void taskDidReceiveData(WebCore::ResourceLoaderIdentifier, const WebCore::SharedBuffer&);
void taskDidReceiveData(WebCore::ResourceLoaderIdentifier, Ref<WebCore::SharedBuffer>&&);
void taskDidComplete(WebCore::ResourceLoaderIdentifier, const WebCore::ResourceError&);
void taskDidStopLoading(WebURLSchemeTaskProxy&);

Expand Down

0 comments on commit 80bf5c2

Please sign in to comment.