Title: [273832] trunk/Source
Revision
273832
Author
commit-qu...@webkit.org
Date
2021-03-03 12:54:26 -0800 (Wed, 03 Mar 2021)

Log Message

Limit HashTable entry size to 500 bytes
https://bugs.webkit.org/show_bug.cgi?id=222658

Patch by Alex Christensen <achristen...@webkit.org> on 2021-03-03
Reviewed by Yusuke Suzuki.

Source/WebCore:

Moving large structures inside HashTables is slower than just moving a pointer.
There is a point at which it becomes more efficient to add a separate allocation
rather than have gigantic HashTables that use all that memory for each empty slot
and use all those read and write operations to move when rehashing.  I'm not sure
exactly where that point is, but I'm pretty sure it's less than 500 bytes.  This
introduces a limit and removes the two largest HashTables in WebKit, replacing their
values with std::unique_ptr to save memory and time.

* Modules/cache/DOMCacheEngine.h:
* platform/network/NetworkLoadInformation.h:

Source/WebKit:

* NetworkProcess/NetworkConnectionToWebProcess.h:
(WebKit::NetworkConnectionToWebProcess::getNetworkLoadInformationResponse):
(WebKit::NetworkConnectionToWebProcess::getNetworkLoadIntermediateInformation):
(WebKit::NetworkConnectionToWebProcess::takeNetworkLoadInformationMetrics):
(WebKit::NetworkConnectionToWebProcess::addNetworkLoadInformation):
(WebKit::NetworkConnectionToWebProcess::addNetworkLoadInformationMetrics):
* NetworkProcess/cache/CacheStorageEngineCaches.cpp:
(WebKit::CacheStorage::Caches::writeRecord):
(WebKit::CacheStorage::Caches::readRecord):
* NetworkProcess/cache/CacheStorageEngineCaches.h:

Source/WTF:

* wtf/HashTable.h:
(WTF::KeyTraits>::inlineLookup):

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (273831 => 273832)


--- trunk/Source/WTF/ChangeLog	2021-03-03 20:19:00 UTC (rev 273831)
+++ trunk/Source/WTF/ChangeLog	2021-03-03 20:54:26 UTC (rev 273832)
@@ -1,3 +1,13 @@
+2021-03-03  Alex Christensen  <achristen...@webkit.org>
+
+        Limit HashTable entry size to 500 bytes
+        https://bugs.webkit.org/show_bug.cgi?id=222658
+
+        Reviewed by Yusuke Suzuki.
+
+        * wtf/HashTable.h:
+        (WTF::KeyTraits>::inlineLookup):
+
 2021-03-03  Youenn Fablet  <you...@apple.com>
 
         WebKitLegacy needs to keep JSDOMWindow even though it is used while its origin is not set

Modified: trunk/Source/WTF/wtf/HashTable.h (273831 => 273832)


--- trunk/Source/WTF/wtf/HashTable.h	2021-03-03 20:19:00 UTC (rev 273831)
+++ trunk/Source/WTF/wtf/HashTable.h	2021-03-03 20:54:26 UTC (rev 273832)
@@ -665,6 +665,8 @@
     template<typename HashTranslator, typename T>
     ALWAYS_INLINE auto HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::inlineLookup(const T& key) -> ValueType*
     {
+        static_assert(sizeof(Key) + sizeof(Value) < 500, "Your HashTable types are too big to efficiently move when rehashing.  Consider using std::unique_ptr instead");
+
         checkKey<HashTranslator>(key);
 
         unsigned k = 0;

Modified: trunk/Source/WebCore/ChangeLog (273831 => 273832)


--- trunk/Source/WebCore/ChangeLog	2021-03-03 20:19:00 UTC (rev 273831)
+++ trunk/Source/WebCore/ChangeLog	2021-03-03 20:54:26 UTC (rev 273832)
@@ -1,3 +1,21 @@
+2021-03-03  Alex Christensen  <achristen...@webkit.org>
+
+        Limit HashTable entry size to 500 bytes
+        https://bugs.webkit.org/show_bug.cgi?id=222658
+
+        Reviewed by Yusuke Suzuki.
+
+        Moving large structures inside HashTables is slower than just moving a pointer.
+        There is a point at which it becomes more efficient to add a separate allocation
+        rather than have gigantic HashTables that use all that memory for each empty slot
+        and use all those read and write operations to move when rehashing.  I'm not sure
+        exactly where that point is, but I'm pretty sure it's less than 500 bytes.  This
+        introduces a limit and removes the two largest HashTables in WebKit, replacing their
+        values with std::unique_ptr to save memory and time.
+
+        * Modules/cache/DOMCacheEngine.h:
+        * platform/network/NetworkLoadInformation.h:
+
 2021-03-03  Megan Gardner  <megan_gard...@apple.com>
 
         Preserve information about the origin of the app highlight request

Modified: trunk/Source/WebCore/Modules/cache/DOMCacheEngine.h (273831 => 273832)


--- trunk/Source/WebCore/Modules/cache/DOMCacheEngine.h	2021-03-03 20:19:00 UTC (rev 273831)
+++ trunk/Source/WebCore/Modules/cache/DOMCacheEngine.h	2021-03-03 20:54:26 UTC (rev 273832)
@@ -61,6 +61,7 @@
 WEBCORE_EXPORT ResponseBody copyResponseBody(const ResponseBody&);
 
 struct Record {
+    WTF_MAKE_STRUCT_FAST_ALLOCATED;
     WEBCORE_EXPORT Record copy() const;
 
     uint64_t identifier;

Modified: trunk/Source/WebCore/platform/network/NetworkLoadInformation.h (273831 => 273832)


--- trunk/Source/WebCore/platform/network/NetworkLoadInformation.h	2021-03-03 20:19:00 UTC (rev 273831)
+++ trunk/Source/WebCore/platform/network/NetworkLoadInformation.h	2021-03-03 20:54:26 UTC (rev 273832)
@@ -33,7 +33,7 @@
 namespace WebCore {
 
 struct NetworkTransactionInformation {
-    enum class Type { Redirection, Preflight };
+    enum class Type : bool { Redirection, Preflight };
     Type type;
     ResourceRequest request;
     ResourceResponse response;
@@ -44,6 +44,7 @@
 };
 
 struct NetworkLoadInformation {
+    WTF_MAKE_STRUCT_FAST_ALLOCATED;
     ResourceRequest request;
     ResourceResponse response;
     NetworkLoadMetrics metrics;
@@ -50,20 +51,6 @@
     Vector<NetworkTransactionInformation> transactions;
 };
 
-}
-
-namespace WTF {
-template<> struct EnumTraits<WebCore::NetworkTransactionInformation::Type> {
-    using values = EnumValues<
-        WebCore::NetworkTransactionInformation::Type,
-        WebCore::NetworkTransactionInformation::Type::Redirection,
-        WebCore::NetworkTransactionInformation::Type::Preflight
-    >;
-};
-}
-
-namespace WebCore {
-
 template<class Encoder> inline void NetworkTransactionInformation::encode(Encoder& encoder) const
 {
     encoder << type;

Modified: trunk/Source/WebKit/ChangeLog (273831 => 273832)


--- trunk/Source/WebKit/ChangeLog	2021-03-03 20:19:00 UTC (rev 273831)
+++ trunk/Source/WebKit/ChangeLog	2021-03-03 20:54:26 UTC (rev 273832)
@@ -1,3 +1,21 @@
+2021-03-03  Alex Christensen  <achristen...@webkit.org>
+
+        Limit HashTable entry size to 500 bytes
+        https://bugs.webkit.org/show_bug.cgi?id=222658
+
+        Reviewed by Yusuke Suzuki.
+
+        * NetworkProcess/NetworkConnectionToWebProcess.h:
+        (WebKit::NetworkConnectionToWebProcess::getNetworkLoadInformationResponse):
+        (WebKit::NetworkConnectionToWebProcess::getNetworkLoadIntermediateInformation):
+        (WebKit::NetworkConnectionToWebProcess::takeNetworkLoadInformationMetrics):
+        (WebKit::NetworkConnectionToWebProcess::addNetworkLoadInformation):
+        (WebKit::NetworkConnectionToWebProcess::addNetworkLoadInformationMetrics):
+        * NetworkProcess/cache/CacheStorageEngineCaches.cpp:
+        (WebKit::CacheStorage::Caches::writeRecord):
+        (WebKit::CacheStorage::Caches::readRecord):
+        * NetworkProcess/cache/CacheStorageEngineCaches.h:
+
 2021-03-03  Megan Gardner  <megan_gard...@apple.com>
 
         Preserve information about the origin of the app highlight request

Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h (273831 => 273832)


--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h	2021-03-03 20:19:00 UTC (rev 273831)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h	2021-03-03 20:54:26 UTC (rev 273832)
@@ -124,23 +124,32 @@
 
     void getNetworkLoadInformationResponse(ResourceLoadIdentifier identifier, CompletionHandler<void(const WebCore::ResourceResponse&)>&& completionHandler)
     {
-        completionHandler(m_networkLoadInformationByID.get(identifier).response);
+        if (auto* info = m_networkLoadInformationByID.get(identifier))
+            return completionHandler(info->response);
+        ASSERT_NOT_REACHED();
+        completionHandler({ });
     }
 
     void getNetworkLoadIntermediateInformation(ResourceLoadIdentifier identifier, CompletionHandler<void(const Vector<WebCore::NetworkTransactionInformation>&)>&& completionHandler)
     {
-        completionHandler(m_networkLoadInformationByID.get(identifier).transactions);
+        if (auto* info = m_networkLoadInformationByID.get(identifier))
+            return completionHandler(info->transactions);
+        ASSERT_NOT_REACHED();
+        completionHandler({ });
     }
 
     void takeNetworkLoadInformationMetrics(ResourceLoadIdentifier identifier, CompletionHandler<void(const WebCore::NetworkLoadMetrics&)>&& completionHandler)
     {
-        completionHandler(m_networkLoadInformationByID.take(identifier).metrics);
+        if (auto info = m_networkLoadInformationByID.take(identifier))
+            return completionHandler(info->metrics);
+        ASSERT_NOT_REACHED();
+        completionHandler({ });
     }
 
     void addNetworkLoadInformation(ResourceLoadIdentifier identifier, WebCore::NetworkLoadInformation&& information)
     {
         ASSERT(!m_networkLoadInformationByID.contains(identifier));
-        m_networkLoadInformationByID.add(identifier, WTFMove(information));
+        m_networkLoadInformationByID.add(identifier, makeUnique<WebCore::NetworkLoadInformation>(WTFMove(information)));
     }
 
     void addNetworkLoadInformationMetrics(ResourceLoadIdentifier identifier, const WebCore::NetworkLoadMetrics& metrics)
@@ -147,8 +156,8 @@
     {
         ASSERT(m_networkLoadInformationByID.contains(identifier));
         m_networkLoadInformationByID.ensure(identifier, [] {
-            return WebCore::NetworkLoadInformation { };
-        }).iterator->value.metrics = metrics;
+            return makeUnique<WebCore::NetworkLoadInformation>();
+        }).iterator->value->metrics = metrics;
     }
 
     void removeNetworkLoadInformation(ResourceLoadIdentifier identifier)
@@ -357,7 +366,7 @@
     HashMap<String, RefPtr<WebCore::BlobDataFileReference>> m_blobDataFileReferences;
     Vector<ResourceNetworkActivityTracker> m_networkActivityTrackers;
 
-    HashMap<ResourceLoadIdentifier, WebCore::NetworkLoadInformation> m_networkLoadInformationByID;
+    HashMap<ResourceLoadIdentifier, std::unique_ptr<WebCore::NetworkLoadInformation>> m_networkLoadInformationByID;
 
 
 #if USE(LIBWEBRTC)

Modified: trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp (273831 => 273832)


--- trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp	2021-03-03 20:19:00 UTC (rev 273831)
+++ trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp	2021-03-03 20:54:26 UTC (rev 273832)
@@ -544,7 +544,7 @@
     m_size -= previousRecordSize;
 
     if (!shouldPersist()) {
-        m_volatileStorage.set(recordInformation.key, WTFMove(record));
+        m_volatileStorage.set(recordInformation.key, makeUnique<Record>(WTFMove(record)));
         callback(WTF::nullopt);
         return;
     }
@@ -566,13 +566,9 @@
     ASSERT(m_isInitialized);
 
     if (!shouldPersist()) {
-        auto iterator = m_volatileStorage.find(key);
-        if (iterator == m_volatileStorage.end()) {
-            callback(makeUnexpected(Error::Internal));
-            return;
-        }
-        callback(iterator->value.copy());
-        return;
+        if (auto* record = m_volatileStorage.get(key))
+            return callback(record->copy());
+        return callback(makeUnexpected(Error::Internal));
     }
 
     m_storage->retrieve(key, 4, [protectedStorage = makeRef(*m_storage), callback = WTFMove(callback)](std::unique_ptr<Storage::Record> storage, const Storage::Timings&) mutable {

Modified: trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h (273831 => 273832)


--- trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h	2021-03-03 20:19:00 UTC (rev 273831)
+++ trunk/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h	2021-03-03 20:54:26 UTC (rev 273832)
@@ -109,7 +109,7 @@
     Vector<Cache> m_caches;
     Vector<Cache> m_removedCaches;
     RefPtr<NetworkCache::Storage> m_storage;
-    HashMap<NetworkCache::Key, WebCore::DOMCacheEngine::Record> m_volatileStorage;
+    HashMap<NetworkCache::Key, std::unique_ptr<WebCore::DOMCacheEngine::Record>> m_volatileStorage;
     mutable Optional<NetworkCache::Salt> m_volatileSalt;
     Vector<WebCore::DOMCacheEngine::CompletionCallback> m_pendingInitializationCallbacks;
     bool m_isWritingCachesToDisk { false };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to