Bug 1286798 - Part 43: Coalesce database operations before they are applied to disk; r=asuth
authorJan Varga <jan.varga@gmail.com>
Thu, 29 Nov 2018 21:49:34 +0100
changeset 508041 5dbbe4015d16797f33ae37207b526dc81b89d50b
parent 508040 398f80b485a969faead8677260975d1b6e2f4606
child 508042 c29b8b8fbf4d209d5e98cde786a6fe58c2dce5a5
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1286798
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -1157,26 +1157,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
 
@@ -1203,20 +1205,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();
 
@@ -1279,76 +1285,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;
 };
@@ -3326,44 +3383,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);
 
@@ -3373,21 +3467,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()
@@ -3504,17 +3597,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)"),
@@ -3704,18 +3797,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();
@@ -4188,17 +4285,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,