Title: [185365] trunk/Source/WebKit2
Revision
185365
Author
[email protected]
Date
2015-06-09 10:07:19 -0700 (Tue, 09 Jun 2015)

Log Message

[SOUP] Network Cache: run the IO completion handler in the given queue instead of the whole operation
https://bugs.webkit.org/show_bug.cgi?id=145797

Reviewed by Žan Doberšek.

I misunderstood what the WorkQueue parameter meant in the IO
channel operations. It's the queue where the completion handler
should be run, not the whole operation. Since our operations are
already non-blocking, we can just run the read/writes in the main
thread, and schedule the completion handler in the given work
queue when the operation finishes.

* NetworkProcess/cache/NetworkCacheIOChannel.h:
* NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:
(WebKit::NetworkCache::inputStreamReadReadyCallback):
(WebKit::NetworkCache::IOChannel::read):
(WebKit::NetworkCache::IOChannel::readSync):
(WebKit::NetworkCache::outputStreamWriteReadyCallback):
(WebKit::NetworkCache::IOChannel::write):

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (185364 => 185365)


--- trunk/Source/WebKit2/ChangeLog	2015-06-09 16:56:37 UTC (rev 185364)
+++ trunk/Source/WebKit2/ChangeLog	2015-06-09 17:07:19 UTC (rev 185365)
@@ -1,3 +1,25 @@
+2015-06-09  Carlos Garcia Campos  <[email protected]>
+
+        [SOUP] Network Cache: run the IO completion handler in the given queue instead of the whole operation
+        https://bugs.webkit.org/show_bug.cgi?id=145797
+
+        Reviewed by Žan Doberšek.
+
+        I misunderstood what the WorkQueue parameter meant in the IO
+        channel operations. It's the queue where the completion handler
+        should be run, not the whole operation. Since our operations are
+        already non-blocking, we can just run the read/writes in the main
+        thread, and schedule the completion handler in the given work
+        queue when the operation finishes.
+
+        * NetworkProcess/cache/NetworkCacheIOChannel.h:
+        * NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:
+        (WebKit::NetworkCache::inputStreamReadReadyCallback):
+        (WebKit::NetworkCache::IOChannel::read):
+        (WebKit::NetworkCache::IOChannel::readSync):
+        (WebKit::NetworkCache::outputStreamWriteReadyCallback):
+        (WebKit::NetworkCache::IOChannel::write):
+
 2015-06-09  David Kilzer  <[email protected]>
 
         REGRESSION (r185357): Fix build for iOS 8.x

Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannel.h (185364 => 185365)


--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannel.h	2015-06-09 16:56:37 UTC (rev 185364)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannel.h	2015-06-09 17:07:19 UTC (rev 185365)
@@ -60,12 +60,6 @@
 private:
     IOChannel(const String& filePath, IOChannel::Type);
 
-#if USE(SOUP)
-    void read(size_t offset, size_t, std::function<void (Data&, int error)>);
-    void readSync(size_t offset, size_t, std::function<void (Data&, int error)>);
-    void write(size_t offset, const Data&, std::function<void (int error)>);
-#endif
-
     String m_path;
     Type m_type;
 

Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp (185364 => 185365)


--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp	2015-06-09 16:56:37 UTC (rev 185364)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp	2015-06-09 17:07:19 UTC (rev 185365)
@@ -29,6 +29,7 @@
 #if ENABLE(NETWORK_CACHE)
 
 #include "NetworkCacheFileSystemPosix.h"
+#include <wtf/MainThread.h>
 #include <wtf/gobject/GMainLoopSource.h>
 #include <wtf/gobject/GMutexLocker.h>
 #include <wtf/gobject/GUniquePtr.h>
@@ -102,6 +103,7 @@
 struct ReadAsyncData {
     RefPtr<IOChannel> channel;
     GRefPtr<SoupBuffer> buffer;
+    RefPtr<WorkQueue> queue;
     size_t bytesToRead;
     std::function<void (Data&, int error)> completionHandler;
     Data data;
@@ -112,12 +114,22 @@
     std::unique_ptr<ReadAsyncData> asyncData(static_cast<ReadAsyncData*>(userData));
     gssize bytesRead = g_input_stream_read_finish(stream, result, nullptr);
     if (bytesRead == -1) {
-        asyncData->completionHandler(asyncData->data, -1);
+        WorkQueue* queue = asyncData->queue.get();
+        auto* asyncDataPtr = asyncData.release();
+        runTaskInQueue([asyncDataPtr] {
+            std::unique_ptr<ReadAsyncData> asyncData(asyncDataPtr);
+            asyncData->completionHandler(asyncData->data, -1);
+        }, queue);
         return;
     }
 
     if (!bytesRead) {
-        asyncData->completionHandler(asyncData->data, 0);
+        WorkQueue* queue = asyncData->queue.get();
+        auto* asyncDataPtr = asyncData.release();
+        runTaskInQueue([asyncDataPtr] {
+            std::unique_ptr<ReadAsyncData> asyncData(asyncDataPtr);
+            asyncData->completionHandler(asyncData->data, 0);
+        }, queue);
         return;
     }
 
@@ -126,7 +138,12 @@
 
     size_t pendingBytesToRead = asyncData->bytesToRead - asyncData->data.size();
     if (!pendingBytesToRead) {
-        asyncData->completionHandler(asyncData->data, 0);
+        WorkQueue* queue = asyncData->queue.get();
+        auto* asyncDataPtr = asyncData.release();
+        runTaskInQueue([asyncDataPtr] {
+            std::unique_ptr<ReadAsyncData> asyncData(asyncDataPtr);
+            asyncData->completionHandler(asyncData->data, 0);
+        }, queue);
         return;
     }
 
@@ -137,38 +154,44 @@
         reinterpret_cast<GAsyncReadyCallback>(inputStreamReadReadyCallback), asyncData.release());
 }
 
-void IOChannel::read(size_t offset, size_t size, std::function<void (Data&, int error)> completionHandler)
+void IOChannel::read(size_t offset, size_t size, WorkQueue* queue, std::function<void (Data&, int error)> completionHandler)
 {
+    RefPtr<IOChannel> channel(this);
     if (!m_inputStream) {
-        Data data;
-        completionHandler(data, -1);
+        runTaskInQueue([channel, completionHandler] {
+            Data data;
+            completionHandler(data, -1);
+        }, queue);
         return;
     }
 
     size_t bufferSize = std::min(size, gDefaultReadBufferSize);
     uint8_t* bufferData = static_cast<uint8_t*>(fastMalloc(bufferSize));
     GRefPtr<SoupBuffer> buffer = adoptGRef(soup_buffer_new_with_owner(bufferData, bufferSize, bufferData, fastFree));
-    ReadAsyncData* asyncData = new ReadAsyncData { this, buffer.get(), size, completionHandler, { } };
+    ReadAsyncData* asyncData = new ReadAsyncData { this, buffer.get(), queue, size, completionHandler, { } };
 
     // FIXME: implement offset.
     g_input_stream_read_async(m_inputStream.get(), const_cast<char*>(buffer->data), bufferSize, G_PRIORITY_DEFAULT, nullptr,
         reinterpret_cast<GAsyncReadyCallback>(inputStreamReadReadyCallback), asyncData);
 }
 
-void IOChannel::read(size_t offset, size_t size, WorkQueue* queue, std::function<void (Data&, int error)> completionHandler)
+// FIXME: It would be better to do without this.
+void IOChannel::readSync(size_t offset, size_t size, WorkQueue* queue, std::function<void (Data&, int error)> completionHandler)
 {
+    ASSERT(!isMainThread());
+
+    static GMutex mutex;
+    static GCond condition;
+    WTF::GMutexLocker<GMutex> lock(mutex);
     RefPtr<IOChannel> channel(this);
-    runTaskInQueue([channel, offset, size, completionHandler] {
-        channel->read(offset, size, completionHandler);
-    }, queue);
-}
 
-// FIXME: It would be better to do without this.
-void IOChannel::readSync(size_t offset, size_t size, std::function<void (Data&, int error)> completionHandler)
-{
     if (!m_inputStream) {
-        Data data;
-        completionHandler(data, -1);
+        runTaskInQueue([channel, completionHandler] {
+            Data data;
+            completionHandler(data, -1);
+            g_cond_signal(&condition);
+        }, queue);
+        g_cond_wait(&condition, &mutex);
         return;
     }
 
@@ -196,18 +219,8 @@
         bytesToRead = std::min(pendingBytesToRead, readBuffer->length);
     } while (pendingBytesToRead);
 
-    completionHandler(data, 0);
-}
-
-void IOChannel::readSync(size_t offset, size_t size, WorkQueue* queue, std::function<void (Data&, int error)> completionHandler)
-{
-    static GMutex mutex;
-    static GCond condition;
-
-    WTF::GMutexLocker<GMutex> lock(mutex);
-    RefPtr<IOChannel> channel(this);
-    runTaskInQueue([channel, offset, size, completionHandler] {
-        channel->readSync(offset, size, completionHandler);
+    runTaskInQueue([channel, &data, completionHandler] {
+        completionHandler(data, 0);
         g_cond_signal(&condition);
     }, queue);
     g_cond_wait(&condition, &mutex);
@@ -216,6 +229,7 @@
 struct WriteAsyncData {
     RefPtr<IOChannel> channel;
     GRefPtr<SoupBuffer> buffer;
+    RefPtr<WorkQueue> queue;
     std::function<void (int error)> completionHandler;
 };
 
@@ -224,13 +238,23 @@
     std::unique_ptr<WriteAsyncData> asyncData(static_cast<WriteAsyncData*>(userData));
     gssize bytesWritten = g_output_stream_write_finish(stream, result, nullptr);
     if (bytesWritten == -1) {
-        asyncData->completionHandler(-1);
+        WorkQueue* queue = asyncData->queue.get();
+        auto* asyncDataPtr = asyncData.release();
+        runTaskInQueue([asyncDataPtr] {
+            std::unique_ptr<WriteAsyncData> asyncData(asyncDataPtr);
+            asyncData->completionHandler(-1);
+        }, queue);
         return;
     }
 
     gssize pendingBytesToWrite = asyncData->buffer->length - bytesWritten;
     if (!pendingBytesToWrite) {
-        asyncData->completionHandler(0);
+        WorkQueue* queue = asyncData->queue.get();
+        auto* asyncDataPtr = asyncData.release();
+        runTaskInQueue([asyncDataPtr] {
+            std::unique_ptr<WriteAsyncData> asyncData(asyncDataPtr);
+            asyncData->completionHandler(0);
+        }, queue);
         return;
     }
 
@@ -241,33 +265,30 @@
         reinterpret_cast<GAsyncReadyCallback>(outputStreamWriteReadyCallback), asyncData.release());
 }
 
-void IOChannel::write(size_t offset, const Data& data, std::function<void (int error)> completionHandler)
+void IOChannel::write(size_t offset, const Data& data, WorkQueue* queue, std::function<void (int error)> completionHandler)
 {
+    RefPtr<IOChannel> channel(this);
     if (!m_outputStream && !m_ioStream) {
-        completionHandler(-1);
+        runTaskInQueue([channel, completionHandler] {
+            completionHandler(-1);
+        }, queue);
         return;
     }
 
     GOutputStream* stream = m_outputStream ? m_outputStream.get() : g_io_stream_get_output_stream(G_IO_STREAM(m_ioStream.get()));
     if (!stream) {
-        completionHandler(-1);
+        runTaskInQueue([channel, completionHandler] {
+            completionHandler(-1);
+        }, queue);
         return;
     }
 
-    WriteAsyncData* asyncData = new WriteAsyncData { this, data.soupBuffer(), completionHandler };
+    WriteAsyncData* asyncData = new WriteAsyncData { this, data.soupBuffer(), queue, completionHandler };
     // FIXME: implement offset.
     g_output_stream_write_async(stream, asyncData->buffer->data, data.size(), G_PRIORITY_DEFAULT, nullptr,
         reinterpret_cast<GAsyncReadyCallback>(outputStreamWriteReadyCallback), asyncData);
 }
 
-void IOChannel::write(size_t offset, const Data& data, WorkQueue* queue, std::function<void (int error)> completionHandler)
-{
-    RefPtr<IOChannel> channel(this);
-    runTaskInQueue([channel, offset, data, completionHandler] {
-        channel->write(offset, data, completionHandler);
-    }, queue);
-}
-
 } // namespace NetworkCache
 } // namespace WebKit
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to