Changeset 278786 in webkit

Timestamp:

Jun 11, 2021, 3:56:18 PM (5 years ago)

Author:

Chris Dumez

Message:

[WK2] Batch local storage database writes using transactions
​https://bugs.webkit.org/show_bug.cgi?id=226938

Reviewed by Geoff Garen.

Batch local storage writes using transactions, to improve performance
and reduce disk writes. In this patch, we use a simple time-based
approach where we batch transactions happening in the same 500ms
period.

• NetworkProcess/WebStorage/LocalStorageDatabase.cpp:

(WebKit::LocalStorageDatabase::create):
(WebKit::LocalStorageDatabase::LocalStorageDatabase):
(WebKit::LocalStorageDatabase::startTransactionIfNecessary):
(WebKit::LocalStorageDatabase::removeItem):
(WebKit::LocalStorageDatabase::setItem):
(WebKit::LocalStorageDatabase::clear):
(WebKit::LocalStorageDatabase::close):

• NetworkProcess/WebStorage/LocalStorageDatabase.h:
• NetworkProcess/WebStorage/StorageArea.cpp:

(WebKit::StorageArea::ensureDatabase const):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/WebStorage/LocalStorageDatabase.cpp (modified) (7 diffs)
• NetworkProcess/WebStorage/LocalStorageDatabase.h (modified) (4 diffs)
• NetworkProcess/WebStorage/StorageArea.cpp (modified) (2 diffs)
• NetworkProcess/WebStorage/StorageArea.h (modified) (1 diff)
• NetworkProcess/WebStorage/StorageManagerSet.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-06-11  Chris Dumez  <cdumez@apple.com>
+
+        [WK2] Batch local storage database writes using transactions
+        https://bugs.webkit.org/show_bug.cgi?id=226938
+
+        Reviewed by Geoff Garen.
+
+        Batch local storage writes using transactions, to improve performance
+        and reduce disk writes. In this patch, we use a simple time-based
+        approach where we batch transactions happening in the same 500ms
+        period.
+
+        * NetworkProcess/WebStorage/LocalStorageDatabase.cpp:
+        (WebKit::LocalStorageDatabase::create):
+        (WebKit::LocalStorageDatabase::LocalStorageDatabase):
+        (WebKit::LocalStorageDatabase::startTransactionIfNecessary):
+        (WebKit::LocalStorageDatabase::removeItem):
+        (WebKit::LocalStorageDatabase::setItem):
+        (WebKit::LocalStorageDatabase::clear):
+        (WebKit::LocalStorageDatabase::close):
+        * NetworkProcess/WebStorage/LocalStorageDatabase.h:
+        * NetworkProcess/WebStorage/StorageArea.cpp:
+        (WebKit::StorageArea::ensureDatabase const):
+
 2021-06-11  Alex Christensen  <achristensen@webkit.org>
 

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

@@ -41 +41 @@
 constexpr auto getItemsQueryString { "SELECT key, value FROM ItemTable"_s };
 constexpr unsigned maximumSizeForValuesKeptInMemory { 1024 }; // 1 KB
-
-Ref<LocalStorageDatabase> LocalStorageDatabase::create(String&& databasePath, unsigned quotaInBytes)
-{
-    return adoptRef(*new LocalStorageDatabase(WTFMove(databasePath), quotaInBytes));
-}
-
-LocalStorageDatabase::LocalStorageDatabase(String&& databasePath, unsigned quotaInBytes)
-    : m_databasePath(WTFMove(databasePath))
+constexpr Seconds transactionDuration { 500_ms };
+
+Ref<LocalStorageDatabase> LocalStorageDatabase::create(Ref<WorkQueue>&& workQueue, String&& databasePath, unsigned quotaInBytes)
+{
+    return adoptRef(*new LocalStorageDatabase(WTFMove(workQueue), WTFMove(databasePath), quotaInBytes));
+}
+
+LocalStorageDatabase::LocalStorageDatabase(Ref<WorkQueue>&& workQueue, String&& databasePath, unsigned quotaInBytes)
+    : m_workQueue(WTFMove(workQueue))
+    , m_databasePath(WTFMove(databasePath))
     , m_quotaInBytes(quotaInBytes)
 {
@@ -99 +101 @@
 
     return true;
+}
+
+void LocalStorageDatabase::startTransactionIfNecessary()
+{
+    if (!m_transaction)
+        m_transaction = makeUnique<SQLiteTransaction>(m_database);
+
+    if (m_transaction->inProgress())
+        return;
+
+    m_transaction->begin();
+    m_workQueue->dispatchAfter(transactionDuration, [weakThis = makeWeakPtr(*this)] {
+        if (weakThis)
+            weakThis->m_transaction->commit();
+    });
 }
 
@@ -185 +202 @@
         return;
 
+    startTransactionIfNecessary();
     oldValue = item(key);
     if (oldValue.isNull())
@@ -249 +267 @@
         return;
 
+    startTransactionIfNecessary();
     oldValue = item(key);
 
@@ -281 +300 @@
         return false;
 
+    startTransactionIfNecessary();
     auto clearStatement = scopedStatement(m_clearStatement, "DELETE FROM ItemTable"_s);
     if (!clearStatement) {
@@ -299 +319 @@
 
     return m_database.lastChanges() > 0;
+}
+
+void LocalStorageDatabase::flushToDisk()
+{
+    if (m_transaction)
+        m_transaction->commit();
 }
 
@@ -316 +342 @@
     m_deleteItemStatement = nullptr;
     m_items = std::nullopt;
+    if (auto transaction = std::exchange(m_transaction, nullptr))
+        transaction->commit();
 
     if (m_database.isOpen())

trunk/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabase.h

@@ -28 +28 @@
 #include <WebCore/SQLiteDatabase.h>
 #include <wtf/HashMap.h>
+#include <wtf/WeakPtr.h>
+#include <wtf/WorkQueue.h>
 
 namespace WebCore {
 class SQLiteStatementAutoResetScope;
+class SQLiteTransaction;
 
 struct SecurityOriginData;
@@ -37 +40 @@
 namespace WebKit {
 
-class LocalStorageDatabase : public RefCounted<LocalStorageDatabase> {
+class LocalStorageDatabase : public RefCounted<LocalStorageDatabase>, public CanMakeWeakPtr<LocalStorageDatabase> {
 public:
-    static Ref<LocalStorageDatabase> create(String&& databasePath, unsigned quotaInBytes);
+    static Ref<LocalStorageDatabase> create(Ref<WorkQueue>&&, String&& databasePath, unsigned quotaInBytes);
     ~LocalStorageDatabase();
 
@@ -52 +55 @@
     void close();
 
+    void flushToDisk();
     void handleLowMemoryWarning();
 
 private:
-    LocalStorageDatabase(String&& databasePath, unsigned quotaInBytes);
+    LocalStorageDatabase(Ref<WorkQueue>&&, String&& databasePath, unsigned quotaInBytes);
 
     enum class ShouldCreateDatabase : bool { No, Yes };
     bool openDatabase(ShouldCreateDatabase);
 
+    void startTransactionIfNecessary();
     bool migrateItemTableIfNeeded();
     bool databaseIsEmpty() const;
@@ -67 +72 @@
     WebCore::SQLiteStatementAutoResetScope scopedStatement(std::unique_ptr<WebCore::SQLiteStatement>&, ASCIILiteral query) const;
 
+    Ref<WorkQueue> m_workQueue;
     String m_databasePath;
     mutable WebCore::SQLiteDatabase m_database;
+    std::unique_ptr<WebCore::SQLiteTransaction> m_transaction;
     const unsigned m_quotaInBytes { 0 };
     bool m_isClosed { false };

trunk/Source/WebKit/NetworkProcess/WebStorage/StorageArea.cpp

@@ -176 +176 @@
     if (!m_localStorageDatabase) {
         auto* localStorageDatabaseTracker = m_localStorageNamespace->storageManager()->localStorageDatabaseTracker();
-        m_localStorageDatabase = LocalStorageDatabase::create(localStorageDatabaseTracker->databasePath(m_securityOrigin), m_quotaInBytes);
+        m_localStorageDatabase = LocalStorageDatabase::create(m_queue.copyRef(), localStorageDatabaseTracker->databasePath(m_securityOrigin), m_quotaInBytes);
         m_localStorageDatabase->openIfExisting();
     }
@@ -200 +200 @@
 }
 
+void StorageArea::syncToDatabase()
+{
+    if (m_localStorageDatabase)
+        m_localStorageDatabase->flushToDisk();
+}
+
 void StorageArea::handleLowMemoryWarning()
 {

trunk/Source/WebKit/NetworkProcess/WebStorage/StorageArea.h

@@ -72 +72 @@
     void close();
 
+    void syncToDatabase();
     void handleLowMemoryWarning();
 

trunk/Source/WebKit/NetworkProcess/WebStorage/StorageManagerSet.cpp

@@ -156 +156 @@
     ASSERT(RunLoop::isMain());
 
-    m_queue->dispatchSync([] { });
+    BinarySemaphore semaphore;
+    m_queue->dispatch([this, &semaphore] {
+        for (const auto& storageArea : m_storageAreas.values()) {
+            ASSERT(storageArea);
+            if (storageArea)
+                storageArea->syncToDatabase();
+        }
+        semaphore.signal();
+    });
+    semaphore.wait();
 }