Changeset 185452 in webkit

Timestamp:

Jun 11, 2015, 5:05:25 AM (11 years ago)

Author:

Antti Koivisto

Message:

3% cold PLT regression from network cache on iOS
​https://bugs.webkit.org/show_bug.cgi?id=145694
rdar://problem/21158245

Reviewed by Chris Dumez.

Cache does not help in cold page loads but it shouldn't' be hurting either. Write I/O needs to be toned down a bit.

• NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm:

(WebKit::NetworkCache::IOChannel::IOChannel):
(WebKit::NetworkCache::IOChannel::open):

Dispatch channels inherit their I/O priority from the target queue. Use background queue for write I/O.

• NetworkProcess/cache/NetworkCacheStorage.cpp:

(WebKit::NetworkCache::Storage::Storage):
(WebKit::NetworkCache::Storage::dispatchPendingReadOperations):
(WebKit::NetworkCache::Storage::dispatchPendingWriteOperations):

Only write one file at a time instead of maximum of three.

(WebKit::NetworkCache::Storage::retrieve):

For consistency with store prepend new entries here too.

(WebKit::NetworkCache::Storage::store):

Delay start of the first write operation by 1s.
Prepend instead of append to the pending write deque so retrieveFromMemory lookup finds newest entries first in case of duplicates

(WebKit::NetworkCache::Storage::traverse):

• NetworkProcess/cache/NetworkCacheStorage.h:

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm (modified) (3 diffs)
• NetworkProcess/cache/NetworkCacheStorage.cpp (modified) (6 diffs)
• NetworkProcess/cache/NetworkCacheStorage.h (modified) (2 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-06-11  Antti Koivisto  <antti@apple.com>
+
+        3% cold PLT regression from network cache on iOS
+        https://bugs.webkit.org/show_bug.cgi?id=145694
+        rdar://problem/21158245
+
+        Reviewed by Chris Dumez.
+
+        Cache does not help in cold page loads but it shouldn't' be hurting either. Write I/O needs to be toned down a bit.
+
+        * NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm:
+        (WebKit::NetworkCache::IOChannel::IOChannel):
+        (WebKit::NetworkCache::IOChannel::open):
+
+            Dispatch channels inherit their I/O priority from the target queue. Use background queue for write I/O.
+
+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
+        (WebKit::NetworkCache::Storage::Storage):
+        (WebKit::NetworkCache::Storage::dispatchPendingReadOperations):
+        (WebKit::NetworkCache::Storage::dispatchPendingWriteOperations):
+
+            Only write one file at a time instead of maximum of three.
+
+        (WebKit::NetworkCache::Storage::retrieve):
+
+            For consistency with store prepend new entries here too.
+
+        (WebKit::NetworkCache::Storage::store):
+
+            Delay start of the first write operation by 1s.
+            Prepend instead of append to the pending write deque so retrieveFromMemory lookup finds newest entries first in case of duplicates
+
+        (WebKit::NetworkCache::Storage::traverse):
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+
 2015-06-10  Yongjun Zhang  <yongjun_zhang@apple.com>
 

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelCocoa.mm

@@ -47 +47 @@
     int oflag;
     mode_t mode;
+    bool useLowIOPriority = false;
 
     switch (m_type) {
@@ -54 +55 @@
         oflag = O_RDWR | O_CREAT | O_NONBLOCK;
         mode = S_IRUSR | S_IWUSR;
+        useLowIOPriority = true;
         break;
     case Type::Write:
         oflag = O_WRONLY | O_NONBLOCK;
         mode = S_IRUSR | S_IWUSR;
+        useLowIOPriority = true;
         break;
     case Type::Read:
@@ -71 +74 @@
     }));
     ASSERT(m_dispatchIO.get());
+
     // This makes the channel read/write all data before invoking the handlers.
     dispatch_io_set_low_water(m_dispatchIO.get(), std::numeric_limits<size_t>::max());
+
+    if (useLowIOPriority) {
+        // The target queue of a dispatch I/O channel specifies the priority of the global queue where its I/O operations are executed.
+        dispatch_set_target_queue(m_dispatchIO.get(), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_BACKGROUND, 0));
+    }
 }
 

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp

@@ -127 +127 @@
     : m_basePath(baseDirectoryPath)
     , m_recordsPath(makeRecordsDirectoryPath(baseDirectoryPath))
+    , m_writeOperationDispatchTimer(*this, &Storage::dispatchPendingWriteOperations)
     , m_ioQueue(WorkQueue::create("com.apple.WebKit.Cache.Storage", WorkQueue::Type::Concurrent))
     , m_backgroundIOQueue(WorkQueue::create("com.apple.WebKit.Cache.Storage.background", WorkQueue::Type::Concurrent, WorkQueue::QOS::Background))
@@ -544 +545 @@
         if (pendingRetrieveQueue.isEmpty())
             continue;
-        auto readOperation = pendingRetrieveQueue.takeFirst();
+        auto readOperation = pendingRetrieveQueue.takeLast();
         auto& read = *readOperation;
         m_activeReadOperations.add(WTF::move(readOperation));
@@ -570 +571 @@
     ASSERT(RunLoop::isMain());
 
-    const int maximumActiveWriteOperationCount { 3 };
+    const int maximumActiveWriteOperationCount { 1 };
 
     while (!m_pendingWriteOperations.isEmpty()) {
@@ -577 +578 @@
             return;
         }
-        auto writeOperation = m_pendingWriteOperations.takeFirst();
+        auto writeOperation = m_pendingWriteOperations.takeLast();
         auto& write = *writeOperation;
         m_activeWriteOperations.add(WTF::move(writeOperation));
@@ -660 +661 @@
         return;
 
-    m_pendingReadOperationsByPriority[priority].append(new ReadOperation { key, WTF::move(completionHandler) } );
+    m_pendingReadOperationsByPriority[priority].prepend(new ReadOperation { key, WTF::move(completionHandler) } );
     dispatchPendingReadOperations();
 }
@@ -672 +673 @@
         return;
 
-    m_pendingWriteOperations.append(new WriteOperation { record, WTF::move(mappedBodyHandler) });
+    m_pendingWriteOperations.prepend(new WriteOperation { record, WTF::move(mappedBodyHandler) });
 
     // Add key to the filter already here as we do lookups from the pending operations too.
     addToRecordFilter(record.key);
 
-    dispatchPendingWriteOperations();
+    bool isInitialWrite = m_pendingWriteOperations.size() == 1;
+    if (!isInitialWrite)
+        return;
+
+    // Delay the start of writes a bit to avoid affecting early page load.
+    // Completing writes will dispatch more writes without delay.
+    static const auto initialWriteDelay = 1_s;
+    m_writeOperationDispatchTimer.startOneShot(initialWriteDelay);
 }
 

trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h

@@ -32 +32 @@
 #include "NetworkCacheData.h"
 #include "NetworkCacheKey.h"
+#include <WebCore/Timer.h>
 #include <wtf/BloomFilter.h>
 #include <wtf/Deque.h>
@@ -151 +152 @@
     Deque<std::unique_ptr<WriteOperation>> m_pendingWriteOperations;
     HashSet<std::unique_ptr<WriteOperation>> m_activeWriteOperations;
+    WebCore::Timer m_writeOperationDispatchTimer;
 
     Ref<WorkQueue> m_ioQueue;