Bug 1546723 - Part 4: Use a write optimizer in LSSnapshot; r=asuth

This patch adds a write optimizer to LSSnapshot. The optimizer is only used when there are no observers for other content processes. Differential Revision: https://phabricator.services.mozilla.com/D31199
2019-05-15 06:11:11 +02:00 · 2019-05-15 06:11:11 +02:00 · b5acd8a9aa
--- a/dom/localstorage/ActorsParent.cpp
+++ b/dom/localstorage/ActorsParent.cpp
@ -1768,9 +1768,13 @@ class Datastore final

  int64_t RequestUpdateUsage(int64_t aRequestedSize, int64_t aMinSize);

-  void NotifyObservers(Database* aDatabase, const nsString& aDocumentURI,
-                       const nsString& aKey, const LSValue& aOldValue,
-                       const LSValue& aNewValue);
+  bool HasOtherProcessObservers(Database* aDatabase);
+
+  void NotifyOtherProcessObservers(Database* aDatabase,
+                                   const nsString& aDocumentURI,
+                                   const nsString& aKey,
+                                   const LSValue& aOldValue,
+                                   const LSValue& aNewValue);

  NS_INLINE_DECL_REFCOUNTING(Datastore)

@ -2124,6 +2128,9 @@ class Snapshot final : public PBackgroundLSSnapshotParent {

  mozilla::ipc::IPCResult RecvDeleteMe() override;

+  mozilla::ipc::IPCResult RecvCheckpoint(
+      nsTArray<LSWriteInfo>&& aWriteInfos) override;
+
  mozilla::ipc::IPCResult RecvCheckpointAndNotify(
      nsTArray<LSWriteAndNotifyInfo>&& aWriteAndNotifyInfos) override;

@ -5325,10 +5332,37 @@ int64_t Datastore::RequestUpdateUsage(int64_t aRequestedSize,
  return 0;
 }

-void Datastore::NotifyObservers(Database* aDatabase,
-                                const nsString& aDocumentURI,
-                                const nsString& aKey, const LSValue& aOldValue,
-                                const LSValue& aNewValue) {
+bool Datastore::HasOtherProcessObservers(Database* aDatabase) {
+  AssertIsOnBackgroundThread();
+  MOZ_ASSERT(aDatabase);
+
+  if (!gObservers) {
+    return false;
+  }
+
+  nsTArray<Observer*>* array;
+  if (!gObservers->Get(mOrigin, &array)) {
+    return false;
+  }
+
+  MOZ_ASSERT(array);
+
+  PBackgroundParent* databaseBackgroundActor = aDatabase->Manager();
+
+  for (Observer* observer : *array) {
+    if (observer->Manager() != databaseBackgroundActor) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+void Datastore::NotifyOtherProcessObservers(Database* aDatabase,
+                                            const nsString& aDocumentURI,
+                                            const nsString& aKey,
+                                            const LSValue& aOldValue,
+                                            const LSValue& aNewValue) {
  AssertIsOnBackgroundThread();
  MOZ_ASSERT(aDatabase);

@ -5693,6 +5727,8 @@ mozilla::ipc::IPCResult Database::RecvPBackgroundLSSnapshotConstructor(
    peakUsage += size;
  }

+  bool hasOtherProcessObservers = mDatastore->HasOtherProcessObservers(this);
+
  snapshot->Init(loadedItems, unknownItems, nextLoadIndex, totalLength,
                 initialUsage, peakUsage, loadState);

@ -5704,6 +5740,7 @@ mozilla::ipc::IPCResult Database::RecvPBackgroundLSSnapshotConstructor(
  aInitInfo->initialUsage() = initialUsage;
  aInitInfo->peakUsage() = peakUsage;
  aInitInfo->loadState() = loadState;
+  aInitInfo->hasOtherProcessObservers() = hasOtherProcessObservers;

  return IPC_OK();
 }
@ -5813,6 +5850,55 @@ mozilla::ipc::IPCResult Snapshot::RecvDeleteMe() {
  return IPC_OK();
 }

+mozilla::ipc::IPCResult Snapshot::RecvCheckpoint(
+    nsTArray<LSWriteInfo>&& aWriteInfos) {
+  AssertIsOnBackgroundThread();
+  MOZ_ASSERT(mUsage >= 0);
+  MOZ_DIAGNOSTIC_ASSERT(mPeakUsage >= mUsage);
+
+  if (NS_WARN_IF(aWriteInfos.IsEmpty())) {
+    ASSERT_UNLESS_FUZZING();
+    return IPC_FAIL_NO_REASON(this);
+  }
+
+  mDatastore->BeginUpdateBatch(mUsage);
+
+  for (uint32_t index = 0; index < aWriteInfos.Length(); index++) {
+    const LSWriteInfo& writeInfo = aWriteInfos[index];
+
+    switch (writeInfo.type()) {
+      case LSWriteInfo::TLSSetItemInfo: {
+        const LSSetItemInfo& info = writeInfo.get_LSSetItemInfo();
+
+        mDatastore->SetItem(mDatabase, info.key(), info.value());
+
+        break;
+      }
+
+      case LSWriteInfo::TLSRemoveItemInfo: {
+        const LSRemoveItemInfo& info = writeInfo.get_LSRemoveItemInfo();
+
+        mDatastore->RemoveItem(mDatabase, info.key());
+
+        break;
+      }
+
+      case LSWriteInfo::TLSClearInfo: {
+        mDatastore->Clear(mDatabase);
+
+        break;
+      }
+
+      default:
+        MOZ_CRASH("Should never get here!");
+    }
+  }
+
+  mUsage = mDatastore->EndUpdateBatch(-1);
+
+  return IPC_OK();
+}
+
 mozilla::ipc::IPCResult Snapshot::RecvCheckpointAndNotify(
    nsTArray<LSWriteAndNotifyInfo>&& aWriteAndNotifyInfos) {
  AssertIsOnBackgroundThread();
@ -5837,8 +5923,8 @@ mozilla::ipc::IPCResult Snapshot::RecvCheckpointAndNotify(

        mDatastore->SetItem(mDatabase, info.key(), info.value());

-        mDatastore->NotifyObservers(mDatabase, mDocumentURI, info.key(),
-                                    info.oldValue(), info.value());
+        mDatastore->NotifyOtherProcessObservers(
+            mDatabase, mDocumentURI, info.key(), info.oldValue(), info.value());

        break;
      }
@ -5849,8 +5935,9 @@ mozilla::ipc::IPCResult Snapshot::RecvCheckpointAndNotify(

        mDatastore->RemoveItem(mDatabase, info.key());

-        mDatastore->NotifyObservers(mDatabase, mDocumentURI, info.key(),
-                                    info.oldValue(), VoidLSValue());
+        mDatastore->NotifyOtherProcessObservers(mDatabase, mDocumentURI,
+                                                info.key(), info.oldValue(),
+                                                VoidLSValue());

        break;
      }
@ -5858,8 +5945,9 @@ mozilla::ipc::IPCResult Snapshot::RecvCheckpointAndNotify(
      case LSWriteAndNotifyInfo::TLSClearInfo: {
        mDatastore->Clear(mDatabase);

-        mDatastore->NotifyObservers(mDatabase, mDocumentURI, VoidString(),
-                                    VoidLSValue(), VoidLSValue());
+        mDatastore->NotifyOtherProcessObservers(mDatabase, mDocumentURI,
+                                                VoidString(), VoidLSValue(),
+                                                VoidLSValue());

        break;
      }
--- a/dom/localstorage/LSSnapshot.cpp
+++ b/dom/localstorage/LSSnapshot.cpp
@ -17,6 +17,61 @@ const uint32_t kSnapshotTimeoutMs = 20000;

 }  // namespace

+/**
+ * Coalescing manipulation queue used by `LSSnapshot`.  Used by `LSSnapshot` to
+ * buffer and coalesce manipulations before they are sent to the parent process,
+ * when a Snapshot Checkpoints. (This can only be done when there are no
+ * observers for other content processes.)
+ */
+class SnapshotWriteOptimizer final
+    : public LSWriteOptimizer<nsAString, nsString> {
+ public:
+  void Enumerate(nsTArray<LSWriteInfo>& aWriteInfos);
+};
+
+void SnapshotWriteOptimizer::Enumerate(nsTArray<LSWriteInfo>& aWriteInfos) {
+  AssertIsOnOwningThread();
+
+  if (mTruncateInfo) {
+    LSClearInfo clearInfo;
+
+    aWriteInfos.AppendElement(std::move(clearInfo));
+  }
+
+  for (auto iter = mWriteInfos.ConstIter(); !iter.Done(); iter.Next()) {
+    WriteInfo* writeInfo = iter.Data();
+
+    switch (writeInfo->GetType()) {
+      case WriteInfo::InsertItem:
+      case WriteInfo::UpdateItem: {
+        auto insertItemInfo = static_cast<InsertItemInfo*>(writeInfo);
+
+        LSSetItemInfo setItemInfo;
+        setItemInfo.key() = insertItemInfo->GetKey();
+        setItemInfo.value() = LSValue(insertItemInfo->GetValue());
+
+        aWriteInfos.AppendElement(std::move(setItemInfo));
+
+        break;
+      }
+
+      case WriteInfo::DeleteItem: {
+        auto deleteItemInfo = static_cast<DeleteItemInfo*>(writeInfo);
+
+        LSRemoveItemInfo removeItemInfo;
+        removeItemInfo.key() = deleteItemInfo->GetKey();
+
+        aWriteInfos.AppendElement(std::move(removeItemInfo));
+
+        break;
+      }
+
+      default:
+        MOZ_CRASH("Bad type!");
+    }
+  }
+}
+
 LSSnapshot::LSSnapshot(LSDatabase* aDatabase)
    : mDatabase(aDatabase),
      mActor(nullptr),
@ -25,6 +80,7 @@ LSSnapshot::LSSnapshot(LSDatabase* aDatabase)
      mExactUsage(0),
      mPeakUsage(0),
      mLoadState(LoadState::Initial),
+      mHasOtherProcessObservers(false),
      mExplicit(false),
      mHasPendingStableStateCallback(false),
      mHasPendingTimerCallback(false),
@ -102,12 +158,20 @@ nsresult LSSnapshot::Init(const nsAString& aKey,

  mLoadState = aInitInfo.loadState();

+  mHasOtherProcessObservers = aInitInfo.hasOtherProcessObservers();
+
  mExplicit = aExplicit;

 #ifdef DEBUG
  mInitialized = true;
 #endif

+  if (mHasOtherProcessObservers) {
+    mWriteAndNotifyInfos = new nsTArray<LSWriteAndNotifyInfo>();
+  } else {
+    mWriteOptimizer = new SnapshotWriteOptimizer();
+  }
+
  if (!mExplicit) {
    mTimer = NS_NewTimer();
    MOZ_ASSERT(mTimer);
@ -241,12 +305,24 @@ nsresult LSSnapshot::SetItem(const nsAString& aKey, const nsAString& aValue,
      mLength++;
    }

-    LSSetItemAndNotifyInfo setItemAndNotifyInfo;
-    setItemAndNotifyInfo.key() = aKey;
-    setItemAndNotifyInfo.oldValue() = LSValue(oldValue);
-    setItemAndNotifyInfo.value() = LSValue(aValue);
+    if (mHasOtherProcessObservers) {
+      MOZ_ASSERT(mWriteAndNotifyInfos);

-    mWriteAndNotifyInfos.AppendElement(std::move(setItemAndNotifyInfo));
+      LSSetItemAndNotifyInfo setItemAndNotifyInfo;
+      setItemAndNotifyInfo.key() = aKey;
+      setItemAndNotifyInfo.oldValue() = LSValue(oldValue);
+      setItemAndNotifyInfo.value() = LSValue(aValue);
+
+      mWriteAndNotifyInfos->AppendElement(std::move(setItemAndNotifyInfo));
+    } else {
+      MOZ_ASSERT(mWriteOptimizer);
+
+      if (oldValue.IsVoid()) {
+        mWriteOptimizer->InsertItem(aKey, aValue);
+      } else {
+        mWriteOptimizer->UpdateItem(aKey, aValue);
+      }
+    }
  }

  aNotifyInfo.changed() = changed;
@ -287,11 +363,19 @@ nsresult LSSnapshot::RemoveItem(const nsAString& aKey,
      mLength--;
    }

-    LSRemoveItemAndNotifyInfo removeItemAndNotifyInfo;
-    removeItemAndNotifyInfo.key() = aKey;
-    removeItemAndNotifyInfo.oldValue() = LSValue(oldValue);
+    if (mHasOtherProcessObservers) {
+      MOZ_ASSERT(mWriteAndNotifyInfos);

-    mWriteAndNotifyInfos.AppendElement(std::move(removeItemAndNotifyInfo));
+      LSRemoveItemAndNotifyInfo removeItemAndNotifyInfo;
+      removeItemAndNotifyInfo.key() = aKey;
+      removeItemAndNotifyInfo.oldValue() = LSValue(oldValue);
+
+      mWriteAndNotifyInfos->AppendElement(std::move(removeItemAndNotifyInfo));
+    } else {
+      MOZ_ASSERT(mWriteOptimizer);
+
+      mWriteOptimizer->DeleteItem(aKey);
+    }
  }

  aNotifyInfo.changed() = changed;
@ -334,9 +418,17 @@ nsresult LSSnapshot::Clear(LSNotifyInfo& aNotifyInfo) {

    mValues.Clear();

-    LSClearInfo clearInfo;
+    if (mHasOtherProcessObservers) {
+      MOZ_ASSERT(mWriteAndNotifyInfos);

-    mWriteAndNotifyInfos.AppendElement(std::move(clearInfo));
+      LSClearInfo clearInfo;
+
+      mWriteAndNotifyInfos->AppendElement(std::move(clearInfo));
+    } else {
+      MOZ_ASSERT(mWriteOptimizer);
+
+      mWriteOptimizer->Truncate();
+    }
  }

  aNotifyInfo.changed() = changed;
@ -593,28 +685,66 @@ nsresult LSSnapshot::EnsureAllKeys() {
    newValues.Put(key, VoidString());
  }

-  for (uint32_t index = 0; index < mWriteAndNotifyInfos.Length(); index++) {
-    const LSWriteAndNotifyInfo& writeAndNotifyInfo =
-        mWriteAndNotifyInfos[index];
+  if (mHasOtherProcessObservers) {
+    MOZ_ASSERT(mWriteAndNotifyInfos);

-    switch (writeAndNotifyInfo.type()) {
-      case LSWriteAndNotifyInfo::TLSSetItemAndNotifyInfo: {
-        newValues.Put(writeAndNotifyInfo.get_LSSetItemAndNotifyInfo().key(),
-                      VoidString());
-        break;
-      }
-      case LSWriteAndNotifyInfo::TLSRemoveItemAndNotifyInfo: {
-        newValues.Remove(
-            writeAndNotifyInfo.get_LSRemoveItemAndNotifyInfo().key());
-        break;
-      }
-      case LSWriteAndNotifyInfo::TLSClearInfo: {
-        newValues.Clear();
-        break;
-      }
+    if (!mWriteAndNotifyInfos->IsEmpty()) {
+      for (uint32_t index = 0; index < mWriteAndNotifyInfos->Length();
+           index++) {
+        const LSWriteAndNotifyInfo& writeAndNotifyInfo =
+            mWriteAndNotifyInfos->ElementAt(index);

-      default:
-        MOZ_CRASH("Should never get here!");
+        switch (writeAndNotifyInfo.type()) {
+          case LSWriteAndNotifyInfo::TLSSetItemAndNotifyInfo: {
+            newValues.Put(writeAndNotifyInfo.get_LSSetItemAndNotifyInfo().key(),
+                          VoidString());
+            break;
+          }
+          case LSWriteAndNotifyInfo::TLSRemoveItemAndNotifyInfo: {
+            newValues.Remove(
+                writeAndNotifyInfo.get_LSRemoveItemAndNotifyInfo().key());
+            break;
+          }
+          case LSWriteAndNotifyInfo::TLSClearInfo: {
+            newValues.Clear();
+            break;
+          }
+
+          default:
+            MOZ_CRASH("Should never get here!");
+        }
+      }
+    }
+  } else {
+    MOZ_ASSERT(mWriteOptimizer);
+
+    if (mWriteOptimizer->HasWrites()) {
+      nsTArray<LSWriteInfo> writeInfos;
+      mWriteOptimizer->Enumerate(writeInfos);
+
+      MOZ_ASSERT(!writeInfos.IsEmpty());
+
+      for (uint32_t index = 0; index < writeInfos.Length(); index++) {
+        const LSWriteInfo& writeInfo = writeInfos[index];
+
+        switch (writeInfo.type()) {
+          case LSWriteInfo::TLSSetItemInfo: {
+            newValues.Put(writeInfo.get_LSSetItemInfo().key(), VoidString());
+            break;
+          }
+          case LSWriteInfo::TLSRemoveItemInfo: {
+            newValues.Remove(writeInfo.get_LSRemoveItemInfo().key());
+            break;
+          }
+          case LSWriteInfo::TLSClearInfo: {
+            newValues.Clear();
+            break;
+          }
+
+          default:
+            MOZ_CRASH("Should never get here!");
+        }
+      }
    }
  }

@ -682,10 +812,27 @@ nsresult LSSnapshot::Checkpoint() {
  MOZ_ASSERT(mInitialized);
  MOZ_ASSERT(!mSentFinish);

-  if (!mWriteAndNotifyInfos.IsEmpty()) {
-    MOZ_ALWAYS_TRUE(mActor->SendCheckpointAndNotify(mWriteAndNotifyInfos));
+  if (mHasOtherProcessObservers) {
+    MOZ_ASSERT(mWriteAndNotifyInfos);

-    mWriteAndNotifyInfos.Clear();
+    if (!mWriteAndNotifyInfos->IsEmpty()) {
+      MOZ_ALWAYS_TRUE(mActor->SendCheckpointAndNotify(*mWriteAndNotifyInfos));
+
+      mWriteAndNotifyInfos->Clear();
+    }
+  } else {
+    MOZ_ASSERT(mWriteOptimizer);
+
+    if (mWriteOptimizer->HasWrites()) {
+      nsTArray<LSWriteInfo> writeInfos;
+      mWriteOptimizer->Enumerate(writeInfos);
+
+      MOZ_ASSERT(!writeInfos.IsEmpty());
+
+      MOZ_ALWAYS_TRUE(mActor->SendCheckpoint(writeInfos));
+
+      mWriteOptimizer->Reset();
+    }
  }

  return NS_OK;
--- a/dom/localstorage/LSSnapshot.h
+++ b/dom/localstorage/LSSnapshot.h
@ -17,6 +17,7 @@ class LSNotifyInfo;
 class LSSnapshotChild;
 class LSSnapshotInitInfo;
 class LSWriteAndNotifyInfo;
+class SnapshotWriteOptimizer;

 class LSSnapshot final : public nsIRunnable {
 public:
@ -79,7 +80,8 @@ class LSSnapshot final : public nsIRunnable {
  nsTHashtable<nsStringHashKey> mLoadedItems;
  nsTHashtable<nsStringHashKey> mUnknownItems;
  nsDataHashtable<nsStringHashKey, nsString> mValues;
-  nsTArray<LSWriteAndNotifyInfo> mWriteAndNotifyInfos;
+  nsAutoPtr<SnapshotWriteOptimizer> mWriteOptimizer;
+  nsAutoPtr<nsTArray<LSWriteAndNotifyInfo>> mWriteAndNotifyInfos;

  uint32_t mInitLength;
  uint32_t mLength;
@ -88,6 +90,7 @@ class LSSnapshot final : public nsIRunnable {

  LoadState mLoadState;

+  bool mHasOtherProcessObservers;
  bool mExplicit;
  bool mHasPendingStableStateCallback;
  bool mHasPendingTimerCallback;
--- a/dom/localstorage/PBackgroundLSDatabase.ipdl
+++ b/dom/localstorage/PBackgroundLSDatabase.ipdl
@ -59,6 +59,11 @@ struct LSSnapshotInitInfo
  int64_t peakUsage;
  // See `LSSnapshot::LoadState` in `LSSnapshot.h`
  LoadState loadState;
+  /**
+   * Boolean indicating whether there where cross-process observers registered
+   * for this origin at the time the snapshot was created.
+   */
+  bool hasOtherProcessObservers;
 };

 /**
--- a/dom/localstorage/PBackgroundLSSnapshot.ipdl
+++ b/dom/localstorage/PBackgroundLSSnapshot.ipdl
@ -15,10 +15,31 @@ using mozilla::dom::LSValue
 namespace mozilla {
 namespace dom {

+struct LSSetItemInfo
+{
+  nsString key;
+  LSValue value;
+};
+
+struct LSRemoveItemInfo
+{
+  nsString key;
+};
+
 struct LSClearInfo
 {
 };

+/**
+ * Union of LocalStorage mutation types.
+ */
+union LSWriteInfo
+{
+  LSSetItemInfo;
+  LSRemoveItemInfo;
+  LSClearInfo;
+};
+
 struct LSSetItemAndNotifyInfo
 {
  nsString key;
@ -49,6 +70,8 @@ sync protocol PBackgroundLSSnapshot
 parent:
  async DeleteMe();

+  async Checkpoint(LSWriteInfo[] writeInfos);
+
  async CheckpointAndNotify(LSWriteAndNotifyInfo[] writeAndNotifyInfos);

  async Finish();
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@ -1292,7 +1292,7 @@ pref("dom.storage.default_quota",      5120);
 pref("dom.storage.shadow_writes", true);
 pref("dom.storage.snapshot_prefill", 16384);
 pref("dom.storage.snapshot_gradual_prefill", 4096);
-pref("dom.storage.snapshot_reusing", true);
+pref("dom.storage.snapshot_reusing", false);
 pref("dom.storage.testing", false);
 pref("dom.storage.client_validation", true);