Bug 1286798 - Part 43: Coalesce database operations before they are applied to disk; r=asuth draft
authorJan Varga <jan.varga@gmail.com>
Wed, 24 Oct 2018 06:59:08 +0200
changeset 481721 413cee18d1f90f3d668495e7bae4c36a72540cf3
parent 481720 564e6a8da37f657c000ee3ab771874d1d7766b8e
child 481722 7fe2c3512187460b4deed2618f56553e717bd4bd
push id10
push userbugmail@asutherland.org
push dateSun, 18 Nov 2018 18:57:42 +0000
reviewersasuth
bugs1286798
milestone65.0a1
Bug 1286798 - Part 43: Coalesce database operations before they are applied to disk; r=asuth This avoids persistence to disk in many cases since sites aften do setItem/removeItem for the same key in one JS function.
dom/localstorage/ActorsParent.cpp
--- a/dom/localstorage/ActorsParent.cpp
+++ b/dom/localstorage/ActorsParent.cpp
@@ -1138,26 +1138,28 @@ class Connection final
 {
   friend class ConnectionThread;
 
 public:
   class CachedStatement;
 
 private:
   class WriteInfo;
-  class SetItemInfo;
+  class AddItemInfo;
+  class UpdateItemInfo;
   class RemoveItemInfo;
   class ClearInfo;
   class EndUpdateBatchOp;
   class CloseOp;
 
   RefPtr<ConnectionThread> mConnectionThread;
   nsCOMPtr<mozIStorageConnection> mStorageConnection;
   nsAutoPtr<ArchivedOriginInfo> mArchivedOriginInfo;
-  nsTArray<nsAutoPtr<WriteInfo>> mWriteInfos;
+  nsAutoPtr<WriteInfo> mClearInfo;
+  nsClassHashtable<nsStringHashKey, WriteInfo> mWriteInfos;
   nsInterfaceHashtable<nsCStringHashKey, mozIStorageStatement>
     mCachedStatements;
   const nsCString mOrigin;
   const nsString mFilePath;
 #ifdef DEBUG
   bool mInUpdateBatch;
 #endif
 
@@ -1184,20 +1186,24 @@ public:
   Dispatch(ConnectionDatastoreOperationBase* aOp);
 
   // This method is used to asynchronously close the storage connection on the
   // connection thread.
   void
   Close(nsIRunnable* aCallback);
 
   void
-  SetItem(const nsString& aKey,
+  AddItem(const nsString& aKey,
           const nsString& aValue);
 
   void
+  UpdateItem(const nsString& aKey,
+             const nsString& aValue);
+
+  void
   RemoveItem(const nsString& aKey);
 
   void
   Clear();
 
   void
   BeginUpdateBatch();
 
@@ -1260,76 +1266,127 @@ private:
   // No funny business allowed.
   CachedStatement(const CachedStatement&) = delete;
   CachedStatement& operator=(const CachedStatement&) = delete;
 };
 
 class Connection::WriteInfo
 {
 public:
+  enum Type {
+    AddItem = 0,
+    UpdateItem,
+    RemoveItem,
+    Clear,
+  };
+
+  virtual Type
+  GetType() = 0;
+
   virtual nsresult
   Perform(Connection* aConnection) = 0;
 
   virtual ~WriteInfo() = default;
 };
 
-class Connection::SetItemInfo final
+class Connection::AddItemInfo
   : public WriteInfo
 {
   nsString mKey;
   nsString mValue;
 
 public:
-  SetItemInfo(const nsAString& aKey,
+  AddItemInfo(const nsAString& aKey,
               const nsAString& aValue)
     : mKey(aKey)
     , mValue(aValue)
   { }
 
 private:
+  Type
+  GetType() override
+  {
+    return AddItem;
+  }
+
   nsresult
   Perform(Connection* aConnection) override;
 };
 
+class Connection::UpdateItemInfo final
+  : public AddItemInfo
+{
+  nsString mKey;
+  nsString mValue;
+
+public:
+  UpdateItemInfo(const nsAString& aKey,
+                 const nsAString& aValue)
+    : AddItemInfo(aKey, aValue)
+  { }
+
+private:
+  Type
+  GetType() override
+  {
+    return UpdateItem;
+  }
+};
+
 class Connection::RemoveItemInfo final
   : public WriteInfo
 {
   nsString mKey;
 
 public:
   explicit RemoveItemInfo(const nsAString& aKey)
     : mKey(aKey)
   { }
 
 private:
+Type
+  GetType() override
+  {
+    return RemoveItem;
+  }
+
   nsresult
   Perform(Connection* aConnection) override;
 };
 
 class Connection::ClearInfo final
   : public WriteInfo
 {
 public:
   ClearInfo()
   { }
 
 private:
+  Type
+  GetType() override
+  {
+    return Clear;
+  }
+
   nsresult
   Perform(Connection* aConnection) override;
 };
 
 class Connection::EndUpdateBatchOp final
   : public ConnectionDatastoreOperationBase
 {
-  nsTArray<nsAutoPtr<WriteInfo>> mWriteInfos;
+  nsAutoPtr<WriteInfo> mClearInfo;
+  nsClassHashtable<nsStringHashKey, WriteInfo> mWriteInfos;
 
 public:
-  explicit EndUpdateBatchOp(Connection* aConnection,
-                            nsTArray<nsAutoPtr<WriteInfo>>& aWriteInfos)
+  EndUpdateBatchOp(Connection* aConnection,
+                   nsAutoPtr<WriteInfo>&& aClearInfo,
+                   nsClassHashtable<nsStringHashKey, WriteInfo>& aWriteInfos)
     : ConnectionDatastoreOperationBase(aConnection)
+    , mClearInfo(std::move(aClearInfo))
   {
     mWriteInfos.SwapElements(aWriteInfos);
   }
 
 private:
   nsresult
   DoDatastoreWork() override;
 };
@@ -3311,44 +3368,81 @@ Connection::Close(nsIRunnable* aCallback
   MOZ_ASSERT(aCallback);
 
   RefPtr<CloseOp> op = new CloseOp(this, aCallback);
 
   Dispatch(op);
 }
 
 void
-Connection::SetItem(const nsString& aKey,
+Connection::AddItem(const nsString& aKey,
                     const nsString& aValue)
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mInUpdateBatch);
 
-  nsAutoPtr<WriteInfo> writeInfo(new SetItemInfo(aKey, aValue));
-  mWriteInfos.AppendElement(writeInfo.forget());
+  WriteInfo* existingWriteInfo;
+  nsAutoPtr<WriteInfo> newWriteInfo;
+  if (mWriteInfos.Get(aKey, &existingWriteInfo) &&
+      existingWriteInfo->GetType() == WriteInfo::RemoveItem) {
+    newWriteInfo = new UpdateItemInfo(aKey, aValue);
+  } else {
+    newWriteInfo = new AddItemInfo(aKey, aValue);
+  }
+
+  mWriteInfos.Put(aKey, newWriteInfo.forget());
+}
+
+void
+Connection::UpdateItem(const nsString& aKey,
+                       const nsString& aValue)
+{
+  AssertIsOnOwningThread();
+  MOZ_ASSERT(mInUpdateBatch);
+
+  WriteInfo* existingWriteInfo;
+  nsAutoPtr<WriteInfo> newWriteInfo;
+  if (mWriteInfos.Get(aKey, &existingWriteInfo) &&
+      existingWriteInfo->GetType() == WriteInfo::AddItem) {
+    newWriteInfo = new AddItemInfo(aKey, aValue);
+  } else {
+    newWriteInfo = new UpdateItemInfo(aKey, aValue);
+  }
+
+  mWriteInfos.Put(aKey, newWriteInfo.forget());
 }
 
 void
 Connection::RemoveItem(const nsString& aKey)
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mInUpdateBatch);
 
-  nsAutoPtr<WriteInfo> writeInfo(new RemoveItemInfo(aKey));
-  mWriteInfos.AppendElement(writeInfo.forget());
+  WriteInfo* existingWriteInfo;
+  if (mWriteInfos.Get(aKey, &existingWriteInfo) &&
+      existingWriteInfo->GetType() == WriteInfo::AddItem) {
+    mWriteInfos.Remove(aKey);
+    return;
+  }
+
+  nsAutoPtr<WriteInfo> newWriteInfo(new RemoveItemInfo(aKey));
+  mWriteInfos.Put(aKey, newWriteInfo.forget());
 }
 
 void
 Connection::Clear()
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mInUpdateBatch);
 
-  nsAutoPtr<WriteInfo> writeInfo(new ClearInfo());
-  mWriteInfos.AppendElement(writeInfo.forget());
+  mWriteInfos.Clear();
+
+  if (!mClearInfo) {
+    mClearInfo = new ClearInfo();
+  }
 }
 
 void
 Connection::BeginUpdateBatch()
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(!mInUpdateBatch);
 
@@ -3358,21 +3452,20 @@ Connection::BeginUpdateBatch()
 }
 
 void
 Connection::EndUpdateBatch()
 {
   AssertIsOnOwningThread();
   MOZ_ASSERT(mInUpdateBatch);
 
-  if (!mWriteInfos.IsEmpty()) {
-    RefPtr<EndUpdateBatchOp> op = new EndUpdateBatchOp(this, mWriteInfos);
-
-    Dispatch(op);
-  }
+  RefPtr<EndUpdateBatchOp> op =
+    new EndUpdateBatchOp(this, std::move(mClearInfo), mWriteInfos);
+
+  Dispatch(op);
 
 #ifdef DEBUG
   mInUpdateBatch = false;
 #endif
 }
 
 nsresult
 Connection::EnsureStorageConnection()
@@ -3489,17 +3582,17 @@ CachedStatement::Assign(Connection* aCon
 
   if (mStatement) {
     mScoper.emplace(mStatement);
   }
 }
 
 nsresult
 Connection::
-SetItemInfo::Perform(Connection* aConnection)
+AddItemInfo::Perform(Connection* aConnection)
 {
   AssertIsOnConnectionThread();
   MOZ_ASSERT(aConnection);
 
   Connection::CachedStatement stmt;
   nsresult rv = aConnection->GetCachedStatement(NS_LITERAL_CSTRING(
     "INSERT OR REPLACE INTO data (key, value) "
     "VALUES(:key, :value)"),
@@ -3689,18 +3782,22 @@ EndUpdateBatchOp::DoDatastoreWork()
     return rv;
   }
 
   rv = stmt->Execute();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
-  for (auto writeInfo : mWriteInfos) {
-    writeInfo->Perform(mConnection);
+  if (mClearInfo) {
+    mClearInfo->Perform(mConnection);
+  }
+
+  for (auto iter = mWriteInfos.ConstIter(); !iter.Done(); iter.Next()) {
+    iter.Data()->Perform(mConnection);
   }
 
   rv = mConnection->GetCachedStatement(NS_LITERAL_CSTRING("COMMIT;"), &stmt);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   rv = stmt->Execute();
@@ -4173,17 +4270,21 @@ Datastore::SetItem(Database* aDatabase,
                       static_cast<int64_t>(oldValue.Length());
 
       mUpdateBatchUsage += delta;
 
       mSizeOfItems += delta;
     }
 
     if (IsPersistent()) {
-      mConnection->SetItem(aKey, aValue);
+      if (oldValue.IsVoid()) {
+        mConnection->AddItem(aKey, aValue);
+      } else {
+        mConnection->UpdateItem(aKey, aValue);
+      }
     }
   }
 
   NotifyObservers(aDatabase, aDocumentURI, aKey, aOldValue, aValue);
 }
 
 void
 Datastore::RemoveItem(Database* aDatabase,