Prewarm local storage in the NetworkProcess to reduce WebContent process hangs
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Jul 2019 02:52:16 +0000 (02:52 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Jul 2019 02:52:16 +0000 (02:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199879
<rdar://problem/53217757>

Reviewed by Ryosuke Niwa.

Source/WebCore:

When JS accesses window.localStorage for the first time, we end up doing a
synchronous IPC to the network process to pull in all items in the local
storage for the origin. If the network process does not have this data in
memory, it has to read it from a database on disk, which may take a significant
amount of time and hang the WebContent process during this time.

To alleviate this problem, this patch introduces prewarming on the local storage
in the network process when loading a given origin in the WebContent process.
This way, in most cases, when the JS accesses window.localStorage for the first
time, the synchronous IPC to the network process returns much faster (measured
50-100ms for a very large database, down from 250-300ms), as it only needs to
IPC the data over, without the need to fetch it from disk.

As a safety net to avoid excessive prewarming, we currently prewarm at most 5
security origins per page load.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::commitData):
* page/DOMWindow.cpp:
(WebCore::DOMWindow::prewarmLocalStorageIfNecessary):
* page/DOMWindow.h:
* page/Frame.cpp:
(WebCore::Frame::didPrewarmLocalStorage):
(WebCore::Frame::mayPrewarmLocalStorage const):
* page/Frame.h:
* storage/Storage.cpp:
(WebCore::Storage::prewarm):
* storage/Storage.h:
* storage/StorageArea.h:
(WebCore::StorageArea::prewarm):

Source/WebKit:

* NetworkProcess/WebStorage/StorageManager.cpp:
(WebKit::StorageManager::prewarm):
(WebKit::StorageManager::getValues):
* NetworkProcess/WebStorage/StorageManager.h:
* NetworkProcess/WebStorage/StorageManager.messages.in:
* WebProcess/WebStorage/StorageAreaImpl.cpp:
(WebKit::StorageAreaImpl::prewarm):
* WebProcess/WebStorage/StorageAreaImpl.h:
* WebProcess/WebStorage/StorageAreaMap.cpp:
(WebKit::StorageAreaMap::loadValuesIfNeeded):
(WebKit::StorageAreaMap::prewarm):
* WebProcess/WebStorage/StorageAreaMap.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247555 268f45cc-cd09-0410-ab3c-d52691b4dbfc

17 files changed:
Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/DOMWindow.h
Source/WebCore/page/Frame.cpp
Source/WebCore/page/Frame.h
Source/WebCore/storage/Storage.cpp
Source/WebCore/storage/Storage.h
Source/WebCore/storage/StorageArea.h
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp
Source/WebKit/NetworkProcess/WebStorage/StorageManager.h
Source/WebKit/NetworkProcess/WebStorage/StorageManager.messages.in
Source/WebKit/WebProcess/WebStorage/StorageAreaImpl.cpp
Source/WebKit/WebProcess/WebStorage/StorageAreaImpl.h
Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp
Source/WebKit/WebProcess/WebStorage/StorageAreaMap.h

index 1895525..c854972 100644 (file)
@@ -1,3 +1,42 @@
+2019-07-17  Chris Dumez  <cdumez@apple.com>
+
+        Prewarm local storage in the NetworkProcess to reduce WebContent process hangs
+        https://bugs.webkit.org/show_bug.cgi?id=199879
+        <rdar://problem/53217757>
+
+        Reviewed by Ryosuke Niwa.
+
+        When JS accesses window.localStorage for the first time, we end up doing a
+        synchronous IPC to the network process to pull in all items in the local
+        storage for the origin. If the network process does not have this data in
+        memory, it has to read it from a database on disk, which may take a significant
+        amount of time and hang the WebContent process during this time.
+
+        To alleviate this problem, this patch introduces prewarming on the local storage
+        in the network process when loading a given origin in the WebContent process.
+        This way, in most cases, when the JS accesses window.localStorage for the first
+        time, the synchronous IPC to the network process returns much faster (measured
+        50-100ms for a very large database, down from 250-300ms), as it only needs to
+        IPC the data over, without the need to fetch it from disk.
+
+        As a safety net to avoid excessive prewarming, we currently prewarm at most 5
+        security origins per page load.
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::commitData):
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::prewarmLocalStorageIfNecessary):
+        * page/DOMWindow.h:
+        * page/Frame.cpp:
+        (WebCore::Frame::didPrewarmLocalStorage):
+        (WebCore::Frame::mayPrewarmLocalStorage const):
+        * page/Frame.h:
+        * storage/Storage.cpp:
+        (WebCore::Storage::prewarm):
+        * storage/Storage.h:
+        * storage/StorageArea.h:
+        (WebCore::StorageArea::prewarm):
+
 2019-07-17  Robin Morisset  <rmorisset@apple.com>
 
         [WHLSL] The lexer should not choke on a single '/'
index 719f1e8..276a306 100644 (file)
@@ -1084,6 +1084,9 @@ void DocumentLoader::commitData(const char* bytes, size_t length)
         if (!isLoading())
             return;
 
+        if (auto* window = m_frame->document()->domWindow())
+            window->prewarmLocalStorageIfNecessary();
+
         bool userChosen;
         String encoding;
         if (overrideEncoding().isNull()) {
index 95b8888..29bbe82 100644 (file)
@@ -423,6 +423,29 @@ void DOMWindow::didSecureTransitionTo(Document& document)
     m_performance = nullptr;
 }
 
+void DOMWindow::prewarmLocalStorageIfNecessary()
+{
+    auto* page = this->page();
+
+    // No need to prewarm for ephemeral sessions since the data is in memory only.
+    if (!page || page->usesEphemeralSession())
+        return;
+
+    if (!page->mainFrame().mayPrewarmLocalStorage())
+        return;
+
+    auto localStorageResult = this->localStorage();
+    if (localStorageResult.hasException())
+        return;
+
+    auto* localStorage = localStorageResult.returnValue();
+    if (!localStorage)
+        return;
+
+    if (localStorage->prewarm())
+        page->mainFrame().didPrewarmLocalStorage();
+}
+
 DOMWindow::~DOMWindow()
 {
     if (m_suspendedForDocumentSuspension)
index 6e562a9..6db321e 100644 (file)
@@ -181,6 +181,8 @@ public:
 
     void showModalDialog(const String& urlString, const String& dialogFeaturesString, DOMWindow& activeWindow, DOMWindow& firstWindow, const WTF::Function<void(DOMWindow&)>& prepareDialogFunction);
 
+    void prewarmLocalStorageIfNecessary();
+
     void alert(const String& message = emptyString());
     bool confirm(const String& message);
     String prompt(const String& message, const String& defaultValue);
index 5165554..6466a60 100644 (file)
@@ -117,6 +117,9 @@ static const Seconds scrollFrequency { 1000_s / 60. };
 
 DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, frameCounter, ("Frame"));
 
+// We prewarm local storage for at most 5 origins in a given page.
+static const unsigned maxlocalStoragePrewarmingCount { 5 };
+
 static inline Frame* parentFromOwnerElement(HTMLFrameOwnerElement* ownerElement)
 {
     if (!ownerElement)
@@ -975,6 +978,19 @@ void Frame::dropChildren()
         tree().removeChild(*child);
 }
 
+void Frame::didPrewarmLocalStorage()
+{
+    ASSERT(isMainFrame());
+    ASSERT(m_localStoragePrewarmingCount < maxlocalStoragePrewarmingCount);
+    ++m_localStoragePrewarmingCount;
+}
+
+bool Frame::mayPrewarmLocalStorage() const
+{
+    ASSERT(isMainFrame());
+    return m_localStoragePrewarmingCount < maxlocalStoragePrewarmingCount;
+}
+
 void Frame::selfOnlyRef()
 {
     ASSERT(isMainFrame());
index 907a019..3e01d3a 100644 (file)
@@ -289,6 +289,9 @@ public:
 
     WEBCORE_EXPORT bool isAlwaysOnLoggingAllowed() const;
 
+    void didPrewarmLocalStorage();
+    bool mayPrewarmLocalStorage() const;
+
 // ========
 
     void selfOnlyRef();
@@ -348,6 +351,7 @@ private:
     unsigned m_navigationDisableCount { 0 };
     unsigned m_selfOnlyRefCount { 0 };
     bool m_hasHadUserInteraction { false };
+    unsigned m_localStoragePrewarmingCount { 0 };
 
 protected:
     UniqueRef<EventHandler> m_eventHandler;
index 47dbb17..9197b60 100644 (file)
@@ -97,6 +97,11 @@ ExceptionOr<void> Storage::removeItem(const String& key)
     return { };
 }
 
+bool Storage::prewarm()
+{
+    return m_storageArea->prewarm();
+}
+
 ExceptionOr<void> Storage::clear()
 {
     auto* frame = this->frame();
index 28aa9ab..5215f47 100644 (file)
@@ -46,6 +46,7 @@ public:
     ExceptionOr<void> removeItem(const String& key);
     ExceptionOr<void> clear();
     bool contains(const String& key) const;
+    bool prewarm();
 
     // Bindings support functions.
     bool isSupportedPropertyName(const String&) const;
index 9732b66..f39ccd7 100644 (file)
@@ -58,6 +58,7 @@ public:
     virtual void incrementAccessCount() { }
     virtual void decrementAccessCount() { }
     virtual void closeDatabaseIfIdle() { }
+    virtual bool prewarm() { return false; }
 
     virtual const SecurityOriginData& securityOrigin() const = 0;
 };
index 5ae7203..a8c9d78 100644 (file)
@@ -1,3 +1,24 @@
+2019-07-17  Chris Dumez  <cdumez@apple.com>
+
+        Prewarm local storage in the NetworkProcess to reduce WebContent process hangs
+        https://bugs.webkit.org/show_bug.cgi?id=199879
+        <rdar://problem/53217757>
+
+        Reviewed by Ryosuke Niwa.
+
+        * NetworkProcess/WebStorage/StorageManager.cpp:
+        (WebKit::StorageManager::prewarm):
+        (WebKit::StorageManager::getValues):
+        * NetworkProcess/WebStorage/StorageManager.h:
+        * NetworkProcess/WebStorage/StorageManager.messages.in:
+        * WebProcess/WebStorage/StorageAreaImpl.cpp:
+        (WebKit::StorageAreaImpl::prewarm):
+        * WebProcess/WebStorage/StorageAreaImpl.h:
+        * WebProcess/WebStorage/StorageAreaMap.cpp:
+        (WebKit::StorageAreaMap::loadValuesIfNeeded):
+        (WebKit::StorageAreaMap::prewarm):
+        * WebProcess/WebStorage/StorageAreaMap.h:
+
 2019-07-17  Megan Gardner  <megan_gardner@apple.com>
 
         Set WordIsNearTap flag, was not being set at all before
index 7e0336f..3b46ed0 100644 (file)
@@ -65,11 +65,11 @@ public:
 
     bool isEphemeral() const { return !m_localStorageNamespace; }
 
+    void openDatabaseAndImportItemsIfNeeded() const;
+
 private:
     explicit StorageArea(LocalStorageNamespace*, const SecurityOriginData&, unsigned quotaInBytes);
 
-    void openDatabaseAndImportItemsIfNeeded() const;
-
     void dispatchEvents(IPC::Connection::UniqueID sourceConnection, uint64_t sourceStorageAreaID, const String& key, const String& oldValue, const String& newValue, const String& urlString) const;
 
     // Will be null if the storage area belongs to a session storage namespace or the storage area is in an ephemeral session.
@@ -845,7 +845,14 @@ void StorageManager::destroyStorageMap(IPC::Connection& connection, uint64_t sto
     m_storageAreasByConnection.remove(connectionAndStorageMapIDPair);
 }
 
-void StorageManager::getValues(IPC::Connection& connection, WebCore::SecurityOriginData&& securityOriginData, uint64_t storageMapID, uint64_t storageMapSeed, GetValuesCallback&& completionHandler)
+void StorageManager::prewarm(IPC::Connection& connection, uint64_t storageMapID)
+{
+    ASSERT(!RunLoop::isMain());
+    if (auto* storageArea = findStorageArea(connection, storageMapID))
+        storageArea->openDatabaseAndImportItemsIfNeeded();
+}
+
+void StorageManager::getValues(IPC::Connection& connection, uint64_t storageMapID, uint64_t storageMapSeed, GetValuesCallback&& completionHandler)
 {
     ASSERT(!RunLoop::isMain());
     auto* storageArea = findStorageArea(connection, storageMapID);
index 4cdf501..e954a2c 100644 (file)
@@ -86,7 +86,8 @@ private:
     void createSessionStorageMap(IPC::Connection&, uint64_t storageMapID, uint64_t storageNamespaceID, WebCore::SecurityOriginData&&);
     void destroyStorageMap(IPC::Connection&, uint64_t storageMapID);
 
-    void getValues(IPC::Connection&, WebCore::SecurityOriginData&&, uint64_t storageMapID, uint64_t storageMapSeed, GetValuesCallback&&);
+    void getValues(IPC::Connection&, uint64_t storageMapID, uint64_t storageMapSeed, GetValuesCallback&&);
+    void prewarm(IPC::Connection&, uint64_t storageMapID);
     void setItem(IPC::Connection&, WebCore::SecurityOriginData&&, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& key, const String& value, const String& urlString);
     void setItems(IPC::Connection&, uint64_t storageMapID, const HashMap<String, String>& items);
     void removeItem(IPC::Connection&, WebCore::SecurityOriginData&&, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& key, const String& urlString);
index ef37b47..78bb6ab 100644 (file)
@@ -26,7 +26,8 @@ messages -> StorageManager {
     CreateSessionStorageMap(uint64_t storageMapID, uint64_t storageNamespaceID, struct WebCore::SecurityOriginData securityOriginData) WantsConnection
     DestroyStorageMap(uint64_t storageMapID) WantsConnection
 
-    GetValues(struct WebCore::SecurityOriginData securityOriginData, uint64_t storageMapID, uint64_t storageMapSeed) -> (HashMap<String, String> values) Synchronous WantsConnection
+    Prewarm(uint64_t storageMapID) WantsConnection
+    GetValues(uint64_t storageMapID, uint64_t storageMapSeed) -> (HashMap<String, String> values) Synchronous WantsConnection
 
     SetItem(struct WebCore::SecurityOriginData securityOriginData, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, String key, String value, String urlString) WantsConnection
     SetItems(uint64_t storageMapID, HashMap<String, String> items) WantsConnection
index e306f73..740c0db 100644 (file)
@@ -73,6 +73,11 @@ String StorageAreaImpl::item(const String& key)
     return m_storageAreaMap->item(key);
 }
 
+bool StorageAreaImpl::prewarm()
+{
+    return m_storageAreaMap->prewarm();
+}
+
 void StorageAreaImpl::setItem(Frame* sourceFrame, const String& key, const String& value, bool& quotaException)
 {
     ASSERT(!value.isNull());
index 165af44..b8f16f2 100644 (file)
@@ -61,6 +61,7 @@ private:
     void decrementAccessCount() override;
     void closeDatabaseIfIdle() override;
     const WebCore::SecurityOriginData& securityOrigin() const override;
+    bool prewarm() final;
 
     uint64_t m_storageAreaID;
     Ref<StorageAreaMap> m_storageAreaMap;
index bf0dc6c..3e050df 100644 (file)
@@ -178,7 +178,7 @@ void StorageAreaMap::loadValuesIfNeeded()
     // FIXME: This should use a special sendSync flag to indicate that we don't want to process incoming messages while waiting for a reply.
     // (This flag does not yet exist). Since loadValuesIfNeeded() ends up being called from within JavaScript code, processing incoming synchronous messages
     // could lead to weird reentrency bugs otherwise.
-    WebProcess::singleton().ensureNetworkProcessConnection().connection().sendSync(Messages::StorageManager::GetValues(m_securityOrigin->data(), m_storageMapID, m_currentSeed), Messages::StorageManager::GetValues::Reply(values), 0);
+    WebProcess::singleton().ensureNetworkProcessConnection().connection().sendSync(Messages::StorageManager::GetValues(m_storageMapID, m_currentSeed), Messages::StorageManager::GetValues::Reply(values), 0);
 
     m_storageMap = StorageMap::create(m_quotaInBytes);
     m_storageMap->importItems(WTFMove(values));
@@ -187,6 +187,17 @@ void StorageAreaMap::loadValuesIfNeeded()
     m_hasPendingGetValues = true;
 }
 
+bool StorageAreaMap::prewarm()
+{
+    if (m_didPrewarm || m_storageMap)
+        return false;
+    m_didPrewarm = true;
+
+    connect();
+    WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::StorageManager::Prewarm(m_storageMapID), 0);
+    return true;
+}
+
 void StorageAreaMap::didGetValues(uint64_t storageMapSeed)
 {
     if (m_currentSeed != storageMapSeed)
index 29ba7d8..fb91bc9 100644 (file)
@@ -57,6 +57,7 @@ public:
     void removeItem(WebCore::Frame* sourceFrame, StorageAreaImpl* sourceArea, const String& key);
     void clear(WebCore::Frame* sourceFrame, StorageAreaImpl* sourceArea);
     bool contains(const String& key);
+    bool prewarm();
 
     // IPC::MessageReceiver
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
@@ -104,6 +105,7 @@ private:
     HashCountedSet<String> m_pendingValueChanges;
 
     bool m_isDisconnected { true };
+    bool m_didPrewarm { false };
 };
 
 } // namespace WebKit