[Curl] Reduce unnecessary SharedBuffer creation by making CurlMultipa…

…rtHandle::didReceiveMessage take std::span<uint8_t> https://bugs.webkit.org/show_bug.cgi?id=262472 Reviewed by Fujii Hironori. Currently, a SharedBuffer is generated in CurlRequest::didReceiveDataCallback when receiving data from libcurl. However, if CurlRequest::didReceiveData performs CURL_WRITEFUNC_PAUSE processing, this SharedBuffer will be discarded without being used. Also, CurlMultipartHandle::didReceiveMessage performs a SharedBuffer to Vector conversion. This is a wasteful data conversion. For this reason, avoid converting to SharedBuffer at the time of CurlRequest::didReceiveDataCallback. * Source/WebCore/platform/network/curl/CurlMultipartHandle.cpp: (WebCore::CurlMultipartHandle::didReceiveMessage): (WebCore::CurlMultipartHandle::processContent): * Source/WebCore/platform/network/curl/CurlMultipartHandle.h: * Source/WebCore/platform/network/curl/CurlMultipartHandleClient.h: * Source/WebCore/platform/network/curl/CurlRequest.cpp: (WebCore::CurlRequest::didReceiveData): (WebCore::CurlRequest::didReceiveDataFromMultipart): (WebCore::CurlRequest::didReceiveDataCallback): * Source/WebCore/platform/network/curl/CurlRequest.h: * Source/WebCore/platform/network/curl/CurlRequestClient.h: * Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp: (WebKit::NetworkDataTaskCurl::curlDidReceiveData): * Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h: * Tools/TestWebKitAPI/Tests/WebCore/curl/CurlMultipartHandleTests.cpp: (TestWebKitAPI::Curl::toSpan): (TestWebKitAPI::Curl::TEST): Canonical link: https://commits.webkit.org/268801@main
WebKit · Oct 3, 2023 · 7ab6aab · 7ab6aab
1 parent bf33b61
commit 7ab6aab
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 58 deletions.
diff --git a/Source/WebCore/platform/network/curl/CurlMultipartHandle.cpp b/Source/WebCore/platform/network/curl/CurlMultipartHandle.cpp
@@ -79,12 +79,12 @@ CurlMultipartHandle::CurlMultipartHandle(CurlMultipartHandleClient& client, CStr
 {
 }
 
-void CurlMultipartHandle::didReceiveMessage(const SharedBuffer& buffer)
+void CurlMultipartHandle::didReceiveMessage(std::span<const uint8_t> receivedData)
 {
     if (m_state == State::WaitingForTerminate || m_state == State::End || m_didCompleteMessage)
         return; // The handler is closed down so ignore everything.
 
-    m_buffer.append(buffer.data(), buffer.size());
+    m_buffer.append(receivedData);
 
     while (processContent()) { }
 }
@@ -128,7 +128,7 @@ bool CurlMultipartHandle::processContent()
         }
 
         if (m_state == State::InBody && result.dataEnd)
-            m_client->didReceiveDataFromMultipart(SharedBuffer::create(m_buffer.data(), result.dataEnd));
+            m_client->didReceiveDataFromMultipart({ m_buffer.data(), result.dataEnd });
 
         if (result.processed)
             m_buffer.remove(0, result.processed);

diff --git a/Source/WebCore/platform/network/curl/CurlMultipartHandle.h b/Source/WebCore/platform/network/curl/CurlMultipartHandle.h
@@ -45,7 +45,7 @@ class CurlMultipartHandle {
     CurlMultipartHandle(CurlMultipartHandleClient&, CString&&);
     ~CurlMultipartHandle() { }
 
-    WEBCORE_EXPORT void didReceiveMessage(const SharedBuffer&);
+    WEBCORE_EXPORT void didReceiveMessage(std::span<const uint8_t>);
     WEBCORE_EXPORT void didCompleteMessage();
 
     WEBCORE_EXPORT void completeHeaderProcessing();

diff --git a/Source/WebCore/platform/network/curl/CurlMultipartHandleClient.h b/Source/WebCore/platform/network/curl/CurlMultipartHandleClient.h
@@ -32,7 +32,7 @@ namespace WebCore {
 class CurlMultipartHandleClient : public CanMakeThreadSafeCheckedPtr {
 public:
     virtual void didReceiveHeaderFromMultipart(Vector<String>&&) = 0;
-    virtual void didReceiveDataFromMultipart(Ref<SharedBuffer>&&) = 0;
+    virtual void didReceiveDataFromMultipart(std::span<const uint8_t>) = 0;
     virtual void didCompleteFromMultipart() = 0;
 
 protected:

diff --git a/Source/WebCore/platform/network/curl/CurlRequest.cpp b/Source/WebCore/platform/network/curl/CurlRequest.cpp
@@ -346,7 +346,7 @@ size_t CurlRequest::didReceiveHeader(String&& header)
 
 // called with data after all headers have been processed via headerCallback
 
-size_t CurlRequest::didReceiveData(const SharedBuffer& buffer)
+size_t CurlRequest::didReceiveData(std::span<const uint8_t> receivedData)
 {
     if (isCompletedOrCancelled())
         return CURL_WRITEFUNC_ERROR;
@@ -366,22 +366,21 @@ size_t CurlRequest::didReceiveData(const SharedBuffer& buffer)
         return CURL_WRITEFUNC_PAUSE;
     }
 
-    auto receiveBytes = buffer.size();
-    m_totalReceivedSize += receiveBytes;
+    m_totalReceivedSize += receivedData.size();
 
-    if (receiveBytes) {
+    if (receivedData.size()) {
         if (m_multipartHandle) {
-            m_multipartHandle->didReceiveMessage(buffer);
+            m_multipartHandle->didReceiveMessage(receivedData);
             if (m_multipartHandle->hasError())
                 return CURL_WRITEFUNC_ERROR;
         } else {
-            callClient([buffer = Ref { buffer }](CurlRequest& request, CurlRequestClient& client) {
-                client.curlDidReceiveData(request, buffer);
+            callClient([buffer = SharedBuffer::create(receivedData)](CurlRequest& request, CurlRequestClient& client) mutable {
+                client.curlDidReceiveData(request, WTFMove(buffer));
             });
         }
     }
 
-    return receiveBytes;
+    return receivedData.size();
 }
 
 void CurlRequest::didReceiveHeaderFromMultipart(Vector<String>&& headers)
@@ -406,16 +405,14 @@ void CurlRequest::didReceiveHeaderFromMultipart(Vector<String>&& headers)
     });
 }
 
-void CurlRequest::didReceiveDataFromMultipart(Ref<SharedBuffer>&& buffer)
+void CurlRequest::didReceiveDataFromMultipart(std::span<const uint8_t> receivedData)
 {
     if (isCompletedOrCancelled())
         return;
 
-    auto receiveBytes = buffer->size();
-
-    if (receiveBytes) {
-        callClient([buffer = WTFMove(buffer)](CurlRequest& request, CurlRequestClient& client) {
-            client.curlDidReceiveData(request, buffer.get());
+    if (receivedData.size()) {
+        callClient([buffer = SharedBuffer::create(receivedData)](CurlRequest& request, CurlRequestClient& client) mutable {
+            client.curlDidReceiveData(request, WTFMove(buffer));
         });
     }
 }
@@ -639,7 +636,7 @@ size_t CurlRequest::didReceiveHeaderCallback(char* ptr, size_t blockSize, size_t
 
 size_t CurlRequest::didReceiveDataCallback(char* ptr, size_t blockSize, size_t numberOfBlocks, void* userData)
 {
-    return static_cast<CurlRequest*>(userData)->didReceiveData(SharedBuffer::create(ptr, blockSize * numberOfBlocks));
+    return static_cast<CurlRequest*>(userData)->didReceiveData({ reinterpret_cast<const uint8_t*>(ptr), blockSize * numberOfBlocks });
 }
 
 int CurlRequest::didReceiveDebugInfoCallback(CURL*, curl_infotype type, char* data, size_t size, void* userData)

diff --git a/Source/WebCore/platform/network/curl/CurlRequest.h b/Source/WebCore/platform/network/curl/CurlRequest.h
@@ -96,9 +96,9 @@ class CurlRequest : public ThreadSafeRefCounted<CurlRequest>, public CurlRequest
     CURL* setupTransfer() override;
     size_t willSendData(char*, size_t, size_t);
     size_t didReceiveHeader(String&&);
-    size_t didReceiveData(const SharedBuffer&);
+    size_t didReceiveData(std::span<const uint8_t>);
     void didReceiveHeaderFromMultipart(Vector<String>&&) override;
-    void didReceiveDataFromMultipart(Ref<SharedBuffer>&&) override;
+    void didReceiveDataFromMultipart(std::span<const uint8_t>) override;
     void didCompleteFromMultipart() override;
     void didCompleteTransfer(CURLcode) override;
     void didCancelTransfer() override;

diff --git a/Source/WebCore/platform/network/curl/CurlRequestClient.h b/Source/WebCore/platform/network/curl/CurlRequestClient.h
@@ -43,7 +43,7 @@ class CurlRequestClient {
 
     virtual void curlDidSendData(CurlRequest&, unsigned long long bytesSent, unsigned long long totalBytesToBeSent) = 0;
     virtual void curlDidReceiveResponse(CurlRequest&, CurlResponse&&) = 0;
-    virtual void curlDidReceiveData(CurlRequest&, const SharedBuffer&) = 0;
+    virtual void curlDidReceiveData(CurlRequest&, Ref<SharedBuffer>&&) = 0;
     virtual void curlDidComplete(CurlRequest&, NetworkLoadMetrics&&) = 0;
     virtual void curlDidFailWithError(CurlRequest&, ResourceError&&, CertificateInfo&&) = 0;
 };

diff --git a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
@@ -185,7 +185,7 @@ void NetworkDataTaskCurl::curlDidReceiveResponse(CurlRequest& request, CurlRespo
     invokeDidReceiveResponse();
 }
 
-void NetworkDataTaskCurl::curlDidReceiveData(CurlRequest&, const SharedBuffer& buffer)
+void NetworkDataTaskCurl::curlDidReceiveData(CurlRequest&, Ref<SharedBuffer>&& buffer)
 {
     Ref protectedThis { *this };
     if (state() == State::Canceling || state() == State::Completed || (!m_client && !isDownload()))
@@ -195,7 +195,7 @@ void NetworkDataTaskCurl::curlDidReceiveData(CurlRequest&, const SharedBuffer& b
         auto* download = m_session->networkProcess().downloadManager().download(m_pendingDownloadID);
         RELEASE_ASSERT(download);
         uint64_t bytesWritten = 0;
-        for (auto& segment : buffer) {
+        for (auto& segment : buffer.get()) {
             if (-1 == FileSystem::writeToFile(m_downloadDestinationFile, segment.segment->data(), segment.segment->size())) {
                 download->didFail(ResourceError(CURLE_WRITE_ERROR, m_response.url()), IPC::DataReference());
                 invalidateAndCancel();
@@ -208,7 +208,7 @@ void NetworkDataTaskCurl::curlDidReceiveData(CurlRequest&, const SharedBuffer& b
         return;
     }
 
-    m_client->didReceiveData(buffer);
+    m_client->didReceiveData(buffer.get());
 }
 
 void NetworkDataTaskCurl::curlDidComplete(CurlRequest&, NetworkLoadMetrics&& networkLoadMetrics)

diff --git a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h
@@ -71,7 +71,7 @@ class NetworkDataTaskCurl final : public NetworkDataTask, public WebCore::CurlRe
     Ref<WebCore::CurlRequest> createCurlRequest(WebCore::ResourceRequest&&, RequestStatus = RequestStatus::NewRequest);
     void curlDidSendData(WebCore::CurlRequest&, unsigned long long, unsigned long long) override;
     void curlDidReceiveResponse(WebCore::CurlRequest&, WebCore::CurlResponse&&) override;
-    void curlDidReceiveData(WebCore::CurlRequest&, const WebCore::SharedBuffer&) override;
+    void curlDidReceiveData(WebCore::CurlRequest&, Ref<WebCore::SharedBuffer>&&) override;
     void curlDidComplete(WebCore::CurlRequest&, WebCore::NetworkLoadMetrics&&) override;
     void curlDidFailWithError(WebCore::CurlRequest&, WebCore::ResourceError&&, WebCore::CertificateInfo&&) override;