Changeset 179547 in webkit

Timestamp:

Feb 3, 2015, 6:17:12 AM (10 years ago)

Author:

Antti Koivisto

Message:

Memory cache for resources pending disk write
​https://bugs.webkit.org/show_bug.cgi?id=141159

Reviewed by Andreas Kling.

If a resource was requested from the disk cache before it has been written we ended up
loading it again. Add a short lived memory cache to avoid this.

• NetworkProcess/cache/NetworkCache.cpp:

(WebKit::decodeStorageEntry):

• NetworkProcess/cache/NetworkCacheStorage.h:

(WebKit::NetworkCacheStorage::Data::isMap):

Add a way to know whether a Data is backed by map or not. SharedMemory does not work correctly
if it is not a map.

• NetworkProcess/cache/NetworkCacheStorageCocoa.mm:

(WebKit::NetworkCacheStorage::Data::Data):
(WebKit::NetworkCacheStorage::initialize):

Also do some renaming.

(WebKit::decodeEntry):
(WebKit::NetworkCacheStorage::removeEntry):
(WebKit::NetworkCacheStorage::retrieve):
(WebKit::NetworkCacheStorage::store):
(WebKit::NetworkCacheStorage::clear):
(WebKit::NetworkCacheStorage::shrinkIfNeeded):

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/cache/NetworkCache.cpp (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheStorage.h (modified) (5 diffs)
• NetworkProcess/cache/NetworkCacheStorageCocoa.mm (modified) (11 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-02-03  Antti Koivisto  <antti@apple.com>
+
+        Memory cache for resources pending disk write
+        https://bugs.webkit.org/show_bug.cgi?id=141159
+
+        Reviewed by Andreas Kling.
+
+        If a resource was requested from the disk cache before it has been written we ended up
+        loading it again. Add a short lived memory cache to avoid this.
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::decodeStorageEntry):
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+        (WebKit::NetworkCacheStorage::Data::isMap):
+
+            Add a way to know whether a Data is backed by map or not. SharedMemory does not work correctly
+            if it is not a map.
+
+        * NetworkProcess/cache/NetworkCacheStorageCocoa.mm:
+        (WebKit::NetworkCacheStorage::Data::Data):
+        (WebKit::NetworkCacheStorage::initialize):
+
+            Also do some renaming.
+
+        (WebKit::decodeEntry):
+        (WebKit::NetworkCacheStorage::removeEntry):
+        (WebKit::NetworkCacheStorage::retrieve):
+        (WebKit::NetworkCacheStorage::store):
+        (WebKit::NetworkCacheStorage::clear):
+        (WebKit::NetworkCacheStorage::shrinkIfNeeded):
+
 2015-02-02  Antti Koivisto  <antti@apple.com>
 

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

@@ -175 +175 @@
 
 #if ENABLE(SHAREABLE_RESOURCE)
-    RefPtr<SharedMemory> sharedMemory = storageEntry.body.size() ? SharedMemory::createFromVMBuffer(const_cast<uint8_t*>(storageEntry.body.data()), storageEntry.body.size()) : nullptr;
+    RefPtr<SharedMemory> sharedMemory = storageEntry.body.isMap() ? SharedMemory::createFromVMBuffer(const_cast<uint8_t*>(storageEntry.body.data()), storageEntry.body.size()) : nullptr;
     RefPtr<ShareableResource> shareableResource = sharedMemory ? ShareableResource::create(sharedMemory.release(), 0, storageEntry.body.size()) : nullptr;
 

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

@@ -106 +106 @@
     class Data {
     public:
-        Data();
+        Data() { }
         Data(const uint8_t*, size_t);
+
+        enum class Backing { Buffer, Map };
 #if PLATFORM(COCOA)
-        explicit Data(DispatchPtr<dispatch_data_t>);
+        explicit Data(DispatchPtr<dispatch_data_t>, Backing = Backing::Buffer);
 #endif
         bool isNull() const;
@@ -115 +117 @@
         const uint8_t* data() const;
         size_t size() const { return m_size; }
+        bool isMap() const { return m_isMap; }
 
 #if PLATFORM(COCOA)
@@ -123 +126 @@
         mutable DispatchPtr<dispatch_data_t> m_dispatchData;
 #endif
-        mutable const uint8_t* m_data;
-        size_t m_size;
+        mutable const uint8_t* m_data { nullptr };
+        size_t m_size { 0 };
+        bool m_isMap { false };
     };
 
@@ -161 +165 @@
     size_t m_maximumSize { std::numeric_limits<size_t>::max() };
 
-    BloomFilter<20> m_keyFilter;
+    BloomFilter<20> m_contentsFilter;
     std::atomic<size_t> m_approximateEntryCount { 0 };
     std::atomic<bool> m_shrinkInProgress { false };
@@ -167 +171 @@
     Vector<Deque<RetrieveOperation>> m_pendingRetrieveOperationsByPriority;
     unsigned m_activeRetrieveOperationCount { 0 };
+
+    typedef std::pair<NetworkCacheKey, Entry> KeyEntryPair;
+    HashMap<NetworkCacheKey::HashType, std::shared_ptr<KeyEntryPair>, AlreadyHashed> m_writeCache;
 
 #if PLATFORM(COCOA)

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

@@ -74 +74 @@
 }
 
-NetworkCacheStorage::Data::Data()
-    : m_data(nullptr)
-    , m_size(0)
-{
-}
-
 NetworkCacheStorage::Data::Data(const uint8_t* data, size_t size)
     : m_dispatchData(adoptDispatch(dispatch_data_create(data, size, nullptr, DISPATCH_DATA_DESTRUCTOR_DEFAULT)))
-    , m_data(nullptr)
     , m_size(size)
 {
 }
 
-NetworkCacheStorage::Data::Data(DispatchPtr<dispatch_data_t> dispatchData)
+NetworkCacheStorage::Data::Data(DispatchPtr<dispatch_data_t> dispatchData, Backing backing)
 {
     const void* data;
     m_dispatchData = adoptDispatch(dispatch_data_create_map(dispatchData.get(), &data, &m_size));
     m_data = static_cast<const uint8_t*>(data);
+    m_isMap = m_size && backing == Backing::Map;
 }
 
@@ -136 +130 @@
 
     StringCapture cachePathCapture(m_directoryPath);
-    auto& keyFilter = m_keyFilter;
+    auto& diskContentsFilter = m_contentsFilter;
     auto& entryCount = m_approximateEntryCount;
 
-    dispatch_async(m_backgroundIOQueue.get(), [cachePathCapture, &keyFilter, &entryCount] {
+    dispatch_async(m_backgroundIOQueue.get(), [cachePathCapture, &diskContentsFilter, &entryCount] {
         String cachePath = cachePathCapture.string();
-        traverseCacheFiles(cachePath, [&keyFilter, &entryCount](const String& fileName, const String&) {
+        traverseCacheFiles(cachePath, [&diskContentsFilter, &entryCount](const String& fileName, const String&) {
             NetworkCacheKey::HashType hash;
             if (!NetworkCacheKey::stringToHash(fileName, hash))
                 return;
-            keyFilter.add(hash);
+            diskContentsFilter.add(hash);
             ++entryCount;
         });
@@ -294 +288 @@
     return std::make_unique<NetworkCacheStorage::Entry>(NetworkCacheStorage::Entry {
         metaData.timeStamp,
-        NetworkCacheStorage::Data(headerData),
-        NetworkCacheStorage::Data(bodyData)
+        NetworkCacheStorage::Data { headerData },
+        NetworkCacheStorage::Data { bodyData, NetworkCacheStorage::Data::Backing::Map }
     });
 }
@@ -347 +341 @@
     ASSERT(RunLoop::isMain());
 
-    if (m_keyFilter.mayContain(key.hash()))
-        m_keyFilter.remove(key.hash());
+    if (m_contentsFilter.mayContain(key.hash()))
+        m_contentsFilter.remove(key.hash());
 
     StringCapture filePathCapture(filePathForKey(key, m_directoryPath));
@@ -417 +411 @@
     ASSERT(RunLoop::isMain());
 
-    if (!m_keyFilter.mayContain(key.hash())) {
+    if (!m_contentsFilter.mayContain(key.hash())) {
         completionHandler(nullptr);
         return;
+    }
+
+    // Write cache is a temporary memory cache used to respond to requests while a write is pending.
+    if (auto keyEntryPair = m_writeCache.get(key.hash())) {
+        if (keyEntryPair->first == key) {
+            LOG(NetworkCacheStorage, "(NetworkProcess) found from the write cache");
+            dispatch_async(dispatch_get_main_queue(), [keyEntryPair, completionHandler] {
+                completionHandler(std::make_unique<Entry>(keyEntryPair->second));
+            });
+            return;
+        }
     }
 
@@ -432 +437 @@
 {
     ASSERT(RunLoop::isMain());
+
+    m_contentsFilter.add(key.hash());
+    ++m_approximateEntryCount;
+
+    m_writeCache.set(key.hash(), std::make_shared<KeyEntryPair>(key, entry));
 
     StringCapture cachePathCapture(m_directoryPath);
@@ -442 +452 @@
             ASSERT_UNUSED(done, done);
             LOG(NetworkCacheStorage, "(NetworkProcess) write complete error=%d", error);
-            if (!error) {
-                m_keyFilter.add(key.hash());
-                ++m_approximateEntryCount;
-                shrinkIfNeeded();
+            if (error) {
+                if (m_contentsFilter.mayContain(key.hash()))
+                    m_contentsFilter.remove(key.hash());
+                if (m_approximateEntryCount)
+                    --m_approximateEntryCount;
             }
+            m_writeCache.remove(key.hash());
+
             completionHandler(!error);
         });
     });
+
+    shrinkIfNeeded();
 }
 
@@ -456 +471 @@
     ASSERT(RunLoop::isMain());
 
-    if (!m_keyFilter.mayContain(key.hash())) {
+    if (!m_contentsFilter.mayContain(key.hash())) {
         LOG(NetworkCacheStorage, "(NetworkProcess) existing entry not found, storing full entry");
         store(key, updateEntry, completionHandler);
         return;
     }
+
+    m_writeCache.set(key.hash(), std::make_shared<KeyEntryPair>(key, updateEntry));
 
     // Try to update the header of an existing entry.
@@ -485 +502 @@
             if (error)
                 removeEntry(key);
+            m_writeCache.remove(key.hash());
 
             completionHandler(!error);
@@ -504 +522 @@
     LOG(NetworkCacheStorage, "(NetworkProcess) clearing cache");
 
-    m_keyFilter.clear();
+    m_writeCache.clear();
+    m_contentsFilter.clear();
     m_approximateEntryCount = 0;
 
@@ -556 +575 @@
                 return;
             dispatch_async(dispatch_get_main_queue(), [this, hash] {
-                if (m_keyFilter.mayContain(hash))
-                    m_keyFilter.remove(hash);
+                if (m_contentsFilter.mayContain(hash))
+                    m_contentsFilter.remove(hash);
             });
         });