Changeset 247486 in webkit


Ignore:
Timestamp:
Jul 16, 2019 11:25:35 AM (5 years ago)
Author:
Chris Dumez
Message:

Speed up StorageManager::getValues()
​https://bugs.webkit.org/show_bug.cgi?id=199812

Reviewed by Alex Christensen.

Source/WebCore:

  • storage/StorageMap.cpp:

(WebCore::StorageMap::importItems):

  • storage/StorageMap.h:

Source/WebKit:

Made the following performance improvements:

  • Made StorageManager a WorkQueueMessageReceiver again (like it was before it got moved from the UIProcess to the Network process). This avoids a lot of thread hopping (IPC thread -> Main thread -> StorageManagerThread -> Main Thread) and a lot of isolatedCopying of the strings.
  • Move values around when possible to avoid copying.
  • Add fast path to StorageMap::importItems() for when the StorageMap is empty when importing (15ms -> 2.5ms).
  • NetworkProcess/NetworkConnectionToWebProcess.cpp:

(WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
(WebKit::NetworkConnectionToWebProcess::didReceiveSyncMessage):

  • NetworkProcess/WebStorage/LocalStorageDatabase.cpp:

(WebKit::LocalStorageDatabase::importItems):

  • NetworkProcess/WebStorage/StorageManager.cpp:

(WebKit::StorageManager::addAllowedSessionStorageNamespaceConnection):
(WebKit::StorageManager::removeAllowedSessionStorageNamespaceConnection):
(WebKit::StorageManager::processDidCloseConnection):
(WebKit::StorageManager::createLocalStorageMap):
(WebKit::StorageManager::createTransientLocalStorageMap):
(WebKit::StorageManager::createSessionStorageMap):
(WebKit::StorageManager::destroyStorageMap):
(WebKit::StorageManager::getValues):
(WebKit::StorageManager::setItem):
(WebKit::StorageManager::setItems):
(WebKit::StorageManager::removeItem):
(WebKit::StorageManager::clear):

  • NetworkProcess/WebStorage/StorageManager.h:
  • Platform/IPC/Connection.cpp:

(IPC::Connection::addWorkQueueMessageReceiver):
(IPC::Connection::removeWorkQueueMessageReceiver):
(IPC::Connection::processIncomingMessage):
(IPC::Connection::dispatchMessage):
(IPC::Connection::dispatchMessageToWorkQueueReceiver):

  • Platform/IPC/Connection.h:
  • WebProcess/WebStorage/StorageAreaMap.cpp:

(WebKit::StorageAreaMap::loadValuesIfNeeded):
Messages to WorkQueueMessageReceivers are normally dispatched from the IPC WorkQueue. However, there is a race if
a client (here StorageManager) adds itself as a WorkQueueMessageReceiver as a result of receiving an IPC message
on the main thread (here NetworkConnectionToWebProcess::WebPageWasAdded).
The message might have already been dispatched from the IPC WorkQueue to the main thread by the time the
client registers itself as a WorkQueueMessageReceiver. To address this, we check again for messages receivers
once the message arrives on the main thread.

Source/WebKitLegacy:

  • Storage/StorageAreaImpl.cpp:

(WebKit::StorageAreaImpl::importItems):

  • Storage/StorageAreaImpl.h:
  • Storage/StorageAreaSync.cpp:

(WebKit::StorageAreaSync::performImport):

Location:
trunk/Source
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r247484 r247486  
     12019-07-16  Chris Dumez  <cdumez@apple.com>
     2
     3        Speed up StorageManager::getValues()
     4        https://bugs.webkit.org/show_bug.cgi?id=199812
     5
     6        Reviewed by Alex Christensen.
     7
     8        * storage/StorageMap.cpp:
     9        (WebCore::StorageMap::importItems):
     10        * storage/StorageMap.h:
     11
    1122019-07-16  Wenson Hsieh  <wenson_hsieh@apple.com>
    213

  • trunk/Source/WebCore/storage/StorageMap.cpp

    r208845 r247486  
    183183}
    184184
    185 void StorageMap::importItems(const HashMap<String, String>& items)
    186 {
     185void StorageMap::importItems(HashMap<String, String>&& items)
     186{
     187    if (m_map.isEmpty()) {
     188        // Fast path.
     189        m_map = WTFMove(items);
     190        for (auto& pair : m_map) {
     191            ASSERT(m_currentLength + pair.key.length() + pair.value.length() >= m_currentLength);
     192            m_currentLength += (pair.key.length() + pair.value.length());
     193        }
     194        return;
     195    }
     196
    187197    for (auto& item : items) {
    188         const String& key = item.key;
    189         const String& value = item.value;
    190 
    191         HashMap<String, String>::AddResult result = m_map.add(key, value);
     198        auto& key = item.key;
     199        auto& value = item.value;
     200
     201        ASSERT(m_currentLength + key.length() + value.length() >= m_currentLength);
     202        m_currentLength += (key.length() + value.length());
     203       
     204        auto result = m_map.add(WTFMove(key), WTFMove(value));
    192205        ASSERT_UNUSED(result, result.isNewEntry); // True if the key didn't exist previously.
    193 
    194         ASSERT(m_currentLength + key.length() >= m_currentLength);
    195         m_currentLength += key.length();
    196         ASSERT(m_currentLength + value.length() >= m_currentLength);
    197         m_currentLength += value.length();
    198     }
    199 }
    200 
    201 }
     206    }
     207}
     208
     209}

  • trunk/Source/WebCore/storage/StorageMap.h

    r215315 r247486  
    4747    WEBCORE_EXPORT bool contains(const String& key) const;
    4848
    49     WEBCORE_EXPORT void importItems(const HashMap<String, String>&);
     49    WEBCORE_EXPORT void importItems(HashMap<String, String>&&);
    5050    const HashMap<String, String>& items() const { return m_map; }
    5151

  • trunk/Source/WebKit/ChangeLog

    r247483 r247486  
     12019-07-16  Chris Dumez  <cdumez@apple.com>
     2
     3        Speed up StorageManager::getValues()
     4        https://bugs.webkit.org/show_bug.cgi?id=199812
     5
     6        Reviewed by Alex Christensen.
     7
     8        Made the following performance improvements:
     9        - Made StorageManager a WorkQueueMessageReceiver again (like it was before it
     10          got moved from the UIProcess to the Network process). This avoids a lot of
     11          thread hopping (IPC thread -> Main thread -> StorageManagerThread -> Main Thread)
     12          and a lot of isolatedCopying of the strings.
     13        - Move values around when possible to avoid copying.
     14        - Add fast path to StorageMap::importItems() for when the StorageMap is
     15          empty when importing (15ms -> 2.5ms).
     16
     17        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
     18        (WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
     19        (WebKit::NetworkConnectionToWebProcess::didReceiveSyncMessage):
     20        * NetworkProcess/WebStorage/LocalStorageDatabase.cpp:
     21        (WebKit::LocalStorageDatabase::importItems):
     22        * NetworkProcess/WebStorage/StorageManager.cpp:
     23        (WebKit::StorageManager::addAllowedSessionStorageNamespaceConnection):
     24        (WebKit::StorageManager::removeAllowedSessionStorageNamespaceConnection):
     25        (WebKit::StorageManager::processDidCloseConnection):
     26        (WebKit::StorageManager::createLocalStorageMap):
     27        (WebKit::StorageManager::createTransientLocalStorageMap):
     28        (WebKit::StorageManager::createSessionStorageMap):
     29        (WebKit::StorageManager::destroyStorageMap):
     30        (WebKit::StorageManager::getValues):
     31        (WebKit::StorageManager::setItem):
     32        (WebKit::StorageManager::setItems):
     33        (WebKit::StorageManager::removeItem):
     34        (WebKit::StorageManager::clear):
     35        * NetworkProcess/WebStorage/StorageManager.h:
     36
     37        * Platform/IPC/Connection.cpp:
     38        (IPC::Connection::addWorkQueueMessageReceiver):
     39        (IPC::Connection::removeWorkQueueMessageReceiver):
     40        (IPC::Connection::processIncomingMessage):
     41        (IPC::Connection::dispatchMessage):
     42        (IPC::Connection::dispatchMessageToWorkQueueReceiver):
     43        * Platform/IPC/Connection.h:
     44        * WebProcess/WebStorage/StorageAreaMap.cpp:
     45        (WebKit::StorageAreaMap::loadValuesIfNeeded):
     46        Messages to WorkQueueMessageReceivers are normally dispatched from the IPC WorkQueue. However, there is a race if
     47        a client (here StorageManager) adds itself as a WorkQueueMessageReceiver as a result of receiving an IPC message
     48        on the main thread (here NetworkConnectionToWebProcess::WebPageWasAdded).
     49        The message might have already been dispatched from the IPC WorkQueue to the main thread by the time the
     50        client registers itself as a WorkQueueMessageReceiver. To address this, we check again for messages receivers
     51        once the message arrives on the main thread.
     52
    1532019-07-16  Zalan Bujtas  <zalan@apple.com>
    254

  • trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp

    r247072 r247486  
    5050#include "ServiceWorkerFetchTaskMessages.h"
    5151#include "StorageManager.h"
    52 #include "StorageManagerMessages.h"
    5352#include "WebCoreArgumentCoders.h"
    5453#include "WebErrors.h"
    … …  
    233232#endif
    234233
    235     if (decoder.messageReceiverName() == Messages::StorageManager::messageReceiverName()) {
    236         if (auto* session = m_networkProcess->networkSessionByConnection(connection)) {
    237             session->storageManager().didReceiveMessage(connection, decoder);
    238             return;
    239         }
    240     }
    241 
    242234    ASSERT_NOT_REACHED();
    243235}
    … …  
    278270        return paymentCoordinator().didReceiveSyncMessage(connection, decoder, reply);
    279271#endif
    280 
    281     if (decoder.messageReceiverName() == Messages::StorageManager::messageReceiverName()) {
    282         if (auto* session = m_networkProcess->networkSessionByConnection(connection)) {
    283             session->storageManager().didReceiveSyncMessage(connection, decoder, reply);
    284             return;
    285         }
    286     }
    287272
    288273    ASSERT_NOT_REACHED();

  • trunk/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabase.cpp

    r245540 r247486  
    172172        return;
    173173
    174     SQLiteStatement query(m_database, "SELECT key, value FROM ItemTable");
     174    SQLiteStatement query(m_database, "SELECT key, value FROM ItemTable"_str);
    175175    if (query.prepare() != SQLITE_OK) {
    176176        LOG_ERROR("Unable to select items from ItemTable for local storage");
    … …  
    185185        String value = query.getColumnBlobAsString(1);
    186186        if (!key.isNull() && !value.isNull())
    187             items.set(key, value);
     187            items.add(WTFMove(key), WTFMove(value));
    188188        result = query.step();
    189189    }
    … …  
    194194    }
    195195
    196     storageMap.importItems(items);
     196    storageMap.importItems(WTFMove(items));
    197197}
    198198

  • trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp

    r247394 r247486  
    524524{
    525525    auto allowedConnectionID = allowedConnection.uniqueID();
     526    auto addResult = m_connections.add(allowedConnectionID);
     527    if (addResult.isNewEntry)
     528        allowedConnection.addWorkQueueMessageReceiver(Messages::StorageManager::messageReceiverName(), m_queue.get(), this);
     529
    526530    m_queue->dispatch([this, protectedThis = makeRef(*this), allowedConnectionID, storageNamespaceID]() mutable {
    527531        ASSERT(m_sessionStorageNamespaces.contains(storageNamespaceID));
    … …  
    534538{
    535539    auto allowedConnectionID = allowedConnection.uniqueID();
     540    if (m_connections.remove(allowedConnectionID))
     541        allowedConnection.removeWorkQueueMessageReceiver(Messages::StorageManager::messageReceiverName());
     542
    536543    m_queue->dispatch([this, protectedThis = makeRef(*this), allowedConnectionID, storageNamespaceID]() mutable {
    537544        ASSERT(m_sessionStorageNamespaces.contains(storageNamespaceID));
    … …  
    568575void StorageManager::processDidCloseConnection(IPC::Connection& connection)
    569576{
     577    if (m_connections.removeAll(connection.uniqueID()))
     578        connection.removeWorkQueueMessageReceiver(Messages::StorageManager::messageReceiverName());
     579
    570580    m_queue->dispatch([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID()]() mutable {
    571581        Vector<std::pair<IPC::Connection::UniqueID, uint64_t>> connectionAndStorageMapIDPairsToRemove;
    … …  
    730740void StorageManager::createLocalStorageMap(IPC::Connection& connection, uint64_t storageMapID, uint64_t storageNamespaceID, SecurityOriginData&& securityOriginData)
    731741{
    732     m_queue->dispatch([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID(), storageMapID, storageNamespaceID, securityOriginData = securityOriginData.isolatedCopy()]() mutable {
    733         std::pair<IPC::Connection::UniqueID, uint64_t> connectionAndStorageMapIDPair(connectionID, storageMapID);
    734 
    735         ASSERT((HashMap<std::pair<IPC::Connection::UniqueID, uint64_t>, RefPtr<StorageArea>>::isValidKey(connectionAndStorageMapIDPair)));
    736 
    737         auto result = m_storageAreasByConnection.add(connectionAndStorageMapIDPair, nullptr);
    738         ASSERT(result.isNewEntry);
    739         ASSERT((HashMap<uint64_t, RefPtr<LocalStorageNamespace>>::isValidKey(storageNamespaceID)));
    740 
    741         LocalStorageNamespace* localStorageNamespace = getOrCreateLocalStorageNamespace(storageNamespaceID);
    742         ASSERT(localStorageNamespace);
    743 
    744         auto storageArea = localStorageNamespace->getOrCreateStorageArea(WTFMove(securityOriginData), m_localStorageDatabaseTracker ? StorageManager::LocalStorageNamespace::IsEphemeral::No : StorageManager::LocalStorageNamespace::IsEphemeral::Yes);
    745         storageArea->addListener(connectionID, storageMapID);
    746 
    747         result.iterator->value = WTFMove(storageArea);
    748     });
     742    ASSERT(!RunLoop::isMain());
     743    auto connectionID = connection.uniqueID();
     744    std::pair<IPC::Connection::UniqueID, uint64_t> connectionAndStorageMapIDPair(connectionID, storageMapID);
     745
     746    ASSERT((HashMap<std::pair<IPC::Connection::UniqueID, uint64_t>, RefPtr<StorageArea>>::isValidKey(connectionAndStorageMapIDPair)));
     747
     748    auto result = m_storageAreasByConnection.add(connectionAndStorageMapIDPair, nullptr);
     749    ASSERT(result.isNewEntry);
     750    ASSERT((HashMap<uint64_t, RefPtr<LocalStorageNamespace>>::isValidKey(storageNamespaceID)));
     751
     752    LocalStorageNamespace* localStorageNamespace = getOrCreateLocalStorageNamespace(storageNamespaceID);
     753    ASSERT(localStorageNamespace);
     754
     755    auto storageArea = localStorageNamespace->getOrCreateStorageArea(WTFMove(securityOriginData), m_localStorageDatabaseTracker ? StorageManager::LocalStorageNamespace::IsEphemeral::No : StorageManager::LocalStorageNamespace::IsEphemeral::Yes);
     756    storageArea->addListener(connectionID, storageMapID);
     757
     758    result.iterator->value = WTFMove(storageArea);
    749759}
    750760
    751761void StorageManager::createTransientLocalStorageMap(IPC::Connection& connection, uint64_t storageMapID, uint64_t storageNamespaceID, SecurityOriginData&& topLevelOriginData, SecurityOriginData&& origin)
    752762{
    753     m_queue->dispatch([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID(), storageMapID, storageNamespaceID, topLevelOriginData = topLevelOriginData.isolatedCopy(), origin = origin.isolatedCopy()]() mutable {
    754         ASSERT(m_storageAreasByConnection.isValidKey({ connectionID, storageMapID }));
    755 
    756         // See if we already have session storage for this connection/origin combo.
    757         // If so, update the map with the new ID, otherwise keep on trucking.
    758         for (auto it = m_storageAreasByConnection.begin(), end = m_storageAreasByConnection.end(); it != end; ++it) {
    759             if (it->key.first != connectionID)
    760                 continue;
    761             Ref<StorageArea> area = *it->value;
    762             if (!area->isEphemeral())
    763                 continue;
    764             if (!origin.securityOrigin()->isSameSchemeHostPort(area->securityOrigin().securityOrigin().get()))
    765                 continue;
    766             area->addListener(connectionID, storageMapID);
    767             // If the storageMapID used as key in m_storageAreasByConnection is no longer one of the StorageArea's listeners, then this means
    768             // that destroyStorageMap() was already called for that storageMapID but it decided not to remove it from m_storageAreasByConnection
    769             // so that we could reuse it later on for the same connection/origin combo. In this case, it is safe to remove the previous
    770             // storageMapID from m_storageAreasByConnection.
    771             if (!area->hasListener(connectionID, it->key.second))
    772                 m_storageAreasByConnection.remove(it);
    773             m_storageAreasByConnection.add({ connectionID, storageMapID }, WTFMove(area));
    774             return;
    775         }
    776 
    777         auto& slot = m_storageAreasByConnection.add({ connectionID, storageMapID }, nullptr).iterator->value;
    778         ASSERT(!slot);
    779 
    780         auto* transientLocalStorageNamespace = getOrCreateTransientLocalStorageNamespace(storageNamespaceID, WTFMove(topLevelOriginData));
    781 
    782         auto storageArea = transientLocalStorageNamespace->getOrCreateStorageArea(WTFMove(origin));
    783         storageArea->addListener(connectionID, storageMapID);
    784 
    785         slot = WTFMove(storageArea);
    786     });
     763    ASSERT(!RunLoop::isMain());
     764    auto connectionID = connection.uniqueID();
     765
     766    ASSERT(m_storageAreasByConnection.isValidKey({ connectionID, storageMapID }));
     767
     768    // See if we already have session storage for this connection/origin combo.
     769    // If so, update the map with the new ID, otherwise keep on trucking.
     770    for (auto it = m_storageAreasByConnection.begin(), end = m_storageAreasByConnection.end(); it != end; ++it) {
     771        if (it->key.first != connectionID)
     772            continue;
     773        Ref<StorageArea> area = *it->value;
     774        if (!area->isEphemeral())
     775            continue;
     776        if (!origin.securityOrigin()->isSameSchemeHostPort(area->securityOrigin().securityOrigin().get()))
     777            continue;
     778        area->addListener(connectionID, storageMapID);
     779        // If the storageMapID used as key in m_storageAreasByConnection is no longer one of the StorageArea's listeners, then this means
     780        // that destroyStorageMap() was already called for that storageMapID but it decided not to remove it from m_storageAreasByConnection
     781        // so that we could reuse it later on for the same connection/origin combo. In this case, it is safe to remove the previous
     782        // storageMapID from m_storageAreasByConnection.
     783        if (!area->hasListener(connectionID, it->key.second))
     784            m_storageAreasByConnection.remove(it);
     785        m_storageAreasByConnection.add({ connectionID, storageMapID }, WTFMove(area));
     786        return;
     787    }
     788
     789    auto& slot = m_storageAreasByConnection.add({ connectionID, storageMapID }, nullptr).iterator->value;
     790    ASSERT(!slot);
     791
     792    auto* transientLocalStorageNamespace = getOrCreateTransientLocalStorageNamespace(storageNamespaceID, WTFMove(topLevelOriginData));
     793
     794    auto storageArea = transientLocalStorageNamespace->getOrCreateStorageArea(WTFMove(origin));
     795    storageArea->addListener(connectionID, storageMapID);
     796
     797    slot = WTFMove(storageArea);
    787798}
    788799
    789800void StorageManager::createSessionStorageMap(IPC::Connection& connection, uint64_t storageMapID, uint64_t storageNamespaceID, SecurityOriginData&& securityOriginData)
    790801{
    791     m_queue->dispatch([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID(), storageMapID, storageNamespaceID, securityOriginData = securityOriginData.isolatedCopy()]() mutable {
    792         ASSERT(m_sessionStorageNamespaces.isValidKey(storageNamespaceID));
    793 
    794         SessionStorageNamespace* sessionStorageNamespace = m_sessionStorageNamespaces.get(storageNamespaceID);
    795         if (!sessionStorageNamespace) {
    796             // We're getting an incoming message from the web process that's for session storage for a web page
    797             // that has already been closed, just ignore it.
    798             return;
    799         }
    800 
    801         ASSERT(m_storageAreasByConnection.isValidKey({ connectionID, storageMapID }));
    802 
    803         auto& slot = m_storageAreasByConnection.add({ connectionID, storageMapID }, nullptr).iterator->value;
    804         ASSERT(!slot);
    805         ASSERT(sessionStorageNamespace->allowedConnections().contains(connectionID));
    806 
    807         auto storageArea = sessionStorageNamespace->getOrCreateStorageArea(WTFMove(securityOriginData));
    808         storageArea->addListener(connectionID, storageMapID);
    809 
    810         slot = WTFMove(storageArea);
    811     });
     802    ASSERT(!RunLoop::isMain());
     803    auto connectionID = connection.uniqueID();
     804    ASSERT(m_sessionStorageNamespaces.isValidKey(storageNamespaceID));
     805
     806    SessionStorageNamespace* sessionStorageNamespace = m_sessionStorageNamespaces.get(storageNamespaceID);
     807    if (!sessionStorageNamespace) {
     808        // We're getting an incoming message from the web process that's for session storage for a web page
     809        // that has already been closed, just ignore it.
     810        return;
     811    }
     812
     813    ASSERT(m_storageAreasByConnection.isValidKey({ connectionID, storageMapID }));
     814
     815    auto& slot = m_storageAreasByConnection.add({ connectionID, storageMapID }, nullptr).iterator->value;
     816    ASSERT(!slot);
     817    ASSERT(sessionStorageNamespace->allowedConnections().contains(connectionID));
     818
     819    auto storageArea = sessionStorageNamespace->getOrCreateStorageArea(WTFMove(securityOriginData));
     820    storageArea->addListener(connectionID, storageMapID);
     821
     822    slot = WTFMove(storageArea);
    812823}
    813824
    814825void StorageManager::destroyStorageMap(IPC::Connection& connection, uint64_t storageMapID)
    815826{
    816     m_queue->dispatch([this, protectedThis = makeRef(*this), connectionID = connection.uniqueID(), storageMapID]() mutable {
    817         std::pair<IPC::Connection::UniqueID, uint64_t> connectionAndStorageMapIDPair(connectionID, storageMapID);
    818         ASSERT(m_storageAreasByConnection.isValidKey(connectionAndStorageMapIDPair));
    819 
    820         auto it = m_storageAreasByConnection.find(connectionAndStorageMapIDPair);
    821         if (it == m_storageAreasByConnection.end()) {
    822             // The connection has been removed because the last page was closed.
    823             return;
    824         }
    825 
    826         it->value->removeListener(connectionID, storageMapID);
    827 
    828         // Don't remove session storage maps. The web process may reconnect and expect the data to still be around.
    829         if (it->value->isEphemeral())
    830             return;
    831 
    832         m_storageAreasByConnection.remove(connectionAndStorageMapIDPair);
    833     });
    834 }
    835 
    836 static void didGetValues(IPC::Connection& connection, uint64_t storageMapID, const HashMap<String, String>& items, GetValuesCallback&& completionHandler)
    837 {
    838     RunLoop::main().dispatch([items = crossThreadCopy(items), completionHandler = WTFMove(completionHandler)]() mutable {
    839         completionHandler(items);
    840     });
     827    ASSERT(!RunLoop::isMain());
     828    auto connectionID = connection.uniqueID();
     829
     830    std::pair<IPC::Connection::UniqueID, uint64_t> connectionAndStorageMapIDPair(connectionID, storageMapID);
     831    ASSERT(m_storageAreasByConnection.isValidKey(connectionAndStorageMapIDPair));
     832
     833    auto it = m_storageAreasByConnection.find(connectionAndStorageMapIDPair);
     834    if (it == m_storageAreasByConnection.end()) {
     835        // The connection has been removed because the last page was closed.
     836        return;
     837    }
     838
     839    it->value->removeListener(connectionID, storageMapID);
     840
     841    // Don't remove session storage maps. The web process may reconnect and expect the data to still be around.
     842    if (it->value->isEphemeral())
     843        return;
     844
     845    m_storageAreasByConnection.remove(connectionAndStorageMapIDPair);
    841846}
    842847
    843848void StorageManager::getValues(IPC::Connection& connection, WebCore::SecurityOriginData&& securityOriginData, uint64_t storageMapID, uint64_t storageMapSeed, GetValuesCallback&& completionHandler)
    844849{
    845     m_queue->dispatch([this, protectedThis = makeRef(*this), connection = makeRef(connection), securityOriginData = securityOriginData.isolatedCopy(), storageMapID, storageMapSeed, completionHandler = WTFMove(completionHandler)]() mutable {
    846         auto* storageArea = findStorageArea(connection.get(), storageMapID);
    847 
    848         // This is a session storage area for a page that has already been closed. Ignore it.
    849         if (!storageArea)
    850             return didGetValues(connection.get(), storageMapID, { }, WTFMove(completionHandler));
    851 
    852         didGetValues(connection.get(), storageMapID, storageArea->items(), WTFMove(completionHandler));
    853         connection->send(Messages::StorageAreaMap::DidGetValues(storageMapSeed), storageMapID);
    854     });
     850    ASSERT(!RunLoop::isMain());
     851    auto* storageArea = findStorageArea(connection, storageMapID);
     852
     853    // This is a session storage area for a page that has already been closed. Ignore it.
     854    if (!storageArea)
     855        return completionHandler({ });
     856
     857    completionHandler(storageArea->items());
     858    connection.send(Messages::StorageAreaMap::DidGetValues(storageMapSeed), storageMapID);
    855859}
    856860
    857861void StorageManager::setItem(IPC::Connection& connection, WebCore::SecurityOriginData&& securityOriginData, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& key, const String& value, const String& urlString)
    858862{
    859     m_queue->dispatch([this, protectedThis = makeRef(*this), connection = makeRef(connection), securityOriginData = securityOriginData.isolatedCopy(), storageMapID, sourceStorageAreaID, storageMapSeed, key = key.isolatedCopy(), value = value.isolatedCopy(), urlString = urlString.isolatedCopy()]() mutable {
    860         auto* storageArea = findStorageArea(connection.get(), storageMapID);
    861 
    862         // This is a session storage area for a page that has already been closed. Ignore it.
    863         if (!storageArea)
    864             return;
    865 
    866         bool quotaError;
    867         storageArea->setItem(connection->uniqueID(), sourceStorageAreaID, key, value, urlString, quotaError);
    868         connection->send(Messages::StorageAreaMap::DidSetItem(storageMapSeed, key, quotaError), storageMapID);
    869     });
     863    ASSERT(!RunLoop::isMain());
     864    auto* storageArea = findStorageArea(connection, storageMapID);
     865
     866    // This is a session storage area for a page that has already been closed. Ignore it.
     867    if (!storageArea)
     868        return;
     869
     870    bool quotaError;
     871    storageArea->setItem(connection.uniqueID(), sourceStorageAreaID, key, value, urlString, quotaError);
     872    connection.send(Messages::StorageAreaMap::DidSetItem(storageMapSeed, key, quotaError), storageMapID);
    870873}
    871874
    872875void StorageManager::setItems(IPC::Connection& connection, uint64_t storageMapID, const HashMap<String, String>& items)
    873876{
    874     m_queue->dispatch([this, protectedThis = makeRef(*this), connection = makeRef(connection), storageMapID, items = crossThreadCopy(items)]() mutable {
    875         if (auto* storageArea = findStorageArea(connection.get(), storageMapID))
    876             storageArea->setItems(items);
    877     });
     877    ASSERT(!RunLoop::isMain());
     878    if (auto* storageArea = findStorageArea(connection, storageMapID))
     879        storageArea->setItems(items);
    878880}
    879881
    880882void StorageManager::removeItem(IPC::Connection& connection, WebCore::SecurityOriginData&& securityOriginData, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& key, const String& urlString)
    881883{
    882     m_queue->dispatch([this, protectedThis = makeRef(*this), connection = makeRef(connection), securityOriginData = securityOriginData.isolatedCopy(), storageMapID, sourceStorageAreaID, storageMapSeed, key = key.isolatedCopy(), urlString = urlString.isolatedCopy()]() mutable {
    883         auto* storageArea = findStorageArea(connection.get(), storageMapID);
    884 
    885         // This is a session storage area for a page that has already been closed. Ignore it.
    886         if (!storageArea)
    887             return;
    888 
    889         storageArea->removeItem(connection->uniqueID(), sourceStorageAreaID, key, urlString);
    890         connection->send(Messages::StorageAreaMap::DidRemoveItem(storageMapSeed, key), storageMapID);
    891     });
     884    ASSERT(!RunLoop::isMain());
     885    auto* storageArea = findStorageArea(connection, storageMapID);
     886
     887    // This is a session storage area for a page that has already been closed. Ignore it.
     888    if (!storageArea)
     889        return;
     890
     891    storageArea->removeItem(connection.uniqueID(), sourceStorageAreaID, key, urlString);
     892    connection.send(Messages::StorageAreaMap::DidRemoveItem(storageMapSeed, key), storageMapID);
    892893}
    893894
    894895void StorageManager::clear(IPC::Connection& connection, WebCore::SecurityOriginData&& securityOriginData, uint64_t storageMapID, uint64_t sourceStorageAreaID, uint64_t storageMapSeed, const String& urlString)
    895896{
    896     m_queue->dispatch([this, protectedThis = makeRef(*this), connection = makeRef(connection), securityOriginData = securityOriginData.isolatedCopy(), storageMapID, sourceStorageAreaID, storageMapSeed, urlString = urlString.isolatedCopy()]() mutable {
    897         auto* storageArea = findStorageArea(connection.get(), storageMapID);
    898 
    899         // This is a session storage area for a page that has already been closed. Ignore it.
    900         if (!storageArea)
    901             return;
    902 
    903         storageArea->clear(connection->uniqueID(), sourceStorageAreaID, urlString);
    904         connection->send(Messages::StorageAreaMap::DidClear(storageMapSeed), storageMapID);
    905     });
     897    ASSERT(!RunLoop::isMain());
     898    auto* storageArea = findStorageArea(connection, storageMapID);
     899
     900    // This is a session storage area for a page that has already been closed. Ignore it.
     901    if (!storageArea)
     902        return;
     903
     904    storageArea->clear(connection.uniqueID(), sourceStorageAreaID, urlString);
     905    connection.send(Messages::StorageAreaMap::DidClear(storageMapSeed), storageMapID);
    906906}
    907907

  • trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManager.h

    r247370 r247486  
    3232#include <wtf/Forward.h>
    3333#include <wtf/Function.h>
     34#include <wtf/HashCountedSet.h>
    3435#include <wtf/HashSet.h>
    3536#include <wtf/text/StringHash.h>
    … …  
    4647using GetValuesCallback = CompletionHandler<void(const HashMap<String, String>&)>;
    4748
    48 class StorageManager : public ThreadSafeRefCounted<StorageManager, WTF::DestructionThread::MainRunLoop> {
     49class StorageManager : public IPC::Connection::WorkQueueMessageReceiver {
    4950public:
    5051    static Ref<StorageManager> create(String&& localStorageDirectory);
    … …  
    112113
    113114    HashMap<std::pair<IPC::Connection::UniqueID, uint64_t>, RefPtr<StorageArea>> m_storageAreasByConnection;
     115    HashCountedSet<IPC::Connection::UniqueID> m_connections;
    114116
    115117    enum class State {

  • trunk/Source/WebKit/Platform/IPC/Connection.cpp

    r247392 r247486  
    308308    ASSERT(RunLoop::isMain());
    309309
    310     m_connectionQueue->dispatch([protectedThis = makeRef(*this), messageReceiverName = WTFMove(messageReceiverName), workQueue = &workQueue, workQueueMessageReceiver]() mutable {
    311         ASSERT(!protectedThis->m_workQueueMessageReceivers.contains(messageReceiverName));
    312 
    313         protectedThis->m_workQueueMessageReceivers.add(messageReceiverName, std::make_pair(workQueue, workQueueMessageReceiver));
    314     });
     310    std::lock_guard<Lock> lock(m_workQueueMessageReceiversMutex);
     311    ASSERT(!m_workQueueMessageReceivers.contains(messageReceiverName));
     312
     313    m_workQueueMessageReceivers.add(messageReceiverName, std::make_pair(&workQueue, workQueueMessageReceiver));
    315314}
    316315
    … …  
    319318    ASSERT(RunLoop::isMain());
    320319
    321     m_connectionQueue->dispatch([protectedThis = makeRef(*this), messageReceiverName = WTFMove(messageReceiverName)]() mutable {
    322         ASSERT(protectedThis->m_workQueueMessageReceivers.contains(messageReceiverName));
    323         protectedThis->m_workQueueMessageReceivers.remove(messageReceiverName);
    324     });
     320    std::lock_guard<Lock> lock(m_workQueueMessageReceiversMutex);
     321    ASSERT(m_workQueueMessageReceivers.contains(messageReceiverName));
     322    m_workQueueMessageReceivers.remove(messageReceiverName);
    325323}
    326324
    … …  
    694692    }
    695693
    696     if (!m_workQueueMessageReceivers.isValidKey(message->messageReceiverName())) {
     694    if (!WorkQueueMessageReceiverMap::isValidKey(message->messageReceiverName())) {
    697695        RefPtr<Connection> protectedThis(this);
    698696        StringReference messageReceiverNameReference = message->messageReceiverName();
    … …  
    707705    }
    708706
    709     auto it = m_workQueueMessageReceivers.find(message->messageReceiverName());
    710     if (it != m_workQueueMessageReceivers.end()) {
    711         it->value.first->dispatch([protectedThis = makeRef(*this), workQueueMessageReceiver = it->value.second, decoder = WTFMove(message)]() mutable {
    712             protectedThis->dispatchWorkQueueMessageReceiverMessage(*workQueueMessageReceiver, *decoder);
    713         });
    714         return;
    715     }
     707    if (dispatchMessageToWorkQueueReceiver(message))
     708        return;
    716709
    717710#if HAVE(QOS_CLASSES)
    … …  
    988981        return;
    989982    }
     983
    990984    m_client.didReceiveMessage(*this, decoder);
    991985}
    992986
     987bool Connection::dispatchMessageToWorkQueueReceiver(std::unique_ptr<Decoder>& message)
     988{
     989    std::lock_guard<Lock> lock(m_workQueueMessageReceiversMutex);
     990    auto it = m_workQueueMessageReceivers.find(message->messageReceiverName());
     991    if (it != m_workQueueMessageReceivers.end()) {
     992        it->value.first->dispatch([protectedThis = makeRef(*this), workQueueMessageReceiver = it->value.second, decoder = WTFMove(message)]() mutable {
     993            protectedThis->dispatchWorkQueueMessageReceiverMessage(*workQueueMessageReceiver, *decoder);
     994        });
     995        return true;
     996    }
     997    return false;
     998}
     999
    9931000void Connection::dispatchMessage(std::unique_ptr<Decoder> message)
    9941001{
     1002    ASSERT(RunLoop::isMain());
    9951003    if (!isValid())
     1004        return;
     1005
     1006    // Messages to WorkQueueMessageReceivers are normally dispatched from the IPC WorkQueue. However, there is a race if
     1007    // a client adds itself as a WorkQueueMessageReceiver as a result of receiving an IPC message on the main thread.
     1008    // The message might have already been dispatched from the IPC WorkQueue to the main thread by the time the
     1009    // client registers itself as a WorkQueueMessageReceiver. To address this, we check again for messages receivers
     1010    // once the message arrives on the main thread.
     1011    if (dispatchMessageToWorkQueueReceiver(message))
    9961012        return;
    9971013

  • trunk/Source/WebKit/Platform/IPC/Connection.h

    r247322 r247486  
    263263    std::unique_ptr<Decoder> waitForSyncReply(uint64_t syncRequestID, Seconds timeout, OptionSet<SendSyncOption>);
    264264
     265    bool dispatchMessageToWorkQueueReceiver(std::unique_ptr<Decoder>&);
     266
    265267    // Called on the connection work queue.
    266268    void processIncomingMessage(std::unique_ptr<Decoder>);
    … …  
    326328    Ref<WorkQueue> m_connectionQueue;
    327329
    328     HashMap<StringReference, std::pair<RefPtr<WorkQueue>, RefPtr<WorkQueueMessageReceiver>>> m_workQueueMessageReceivers;
     330    Lock m_workQueueMessageReceiversMutex;
     331    using WorkQueueMessageReceiverMap = HashMap<StringReference, std::pair<RefPtr<WorkQueue>, RefPtr<WorkQueueMessageReceiver>>>;
     332    WorkQueueMessageReceiverMap m_workQueueMessageReceivers;
    329333
    330334    unsigned m_inSendSyncCount;

  • trunk/Source/WebKit/WebProcess/WebStorage/StorageAreaMap.cpp

    r246079 r247486  
    182182
    183183    m_storageMap = StorageMap::create(m_quotaInBytes);
    184     m_storageMap->importItems(values);
     184    m_storageMap->importItems(WTFMove(values));
    185185
    186186    // We want to ignore all changes until we get the DidGetValues message.

  • trunk/Source/WebKitLegacy/ChangeLog

    r247401 r247486  
     12019-07-16  Chris Dumez  <cdumez@apple.com>
     2
     3        Speed up StorageManager::getValues()
     4        https://bugs.webkit.org/show_bug.cgi?id=199812
     5
     6        Reviewed by Alex Christensen.
     7
     8        * Storage/StorageAreaImpl.cpp:
     9        (WebKit::StorageAreaImpl::importItems):
     10        * Storage/StorageAreaImpl.h:
     11        * Storage/StorageAreaSync.cpp:
     12        (WebKit::StorageAreaSync::performImport):
     13
    1142019-07-12  Alex Christensen  <achristensen@webkit.org>
    215

  • trunk/Source/WebKitLegacy/Storage/StorageAreaImpl.cpp

    r220071 r247486  
    195195}
    196196
    197 void StorageAreaImpl::importItems(const HashMap<String, String>& items)
    198 {
    199     ASSERT(!m_isShutdown);
    200 
    201     m_storageMap->importItems(items);
     197void StorageAreaImpl::importItems(HashMap<String, String>&& items)
     198{
     199    ASSERT(!m_isShutdown);
     200
     201    m_storageMap->importItems(WTFMove(items));
    202202}
    203203

  • trunk/Source/WebKitLegacy/Storage/StorageAreaImpl.h

    r229979 r247486  
    6868
    6969    // Only called from a background thread.
    70     void importItems(const HashMap<String, String>& items);
     70    void importItems(HashMap<String, String>&& items);
    7171
    7272    // Used to clear a StorageArea and close db before backing db file is deleted.

  • trunk/Source/WebKitLegacy/Storage/StorageAreaSync.cpp

    r240437 r247486  
    348348    }
    349349
    350     m_storageArea->importItems(itemMap);
     350    m_storageArea->importItems(WTFMove(itemMap));
    351351
    352352    markImported();
Note: See TracChangeset for help on using the changeset viewer.