Bug 1168606 - Extract common function for discarding cached responses. r=ttung,asuth
authorSimon Giesecke <sgiesecke@mozilla.com>
Tue, 05 Nov 2019 16:46:10 +0000
changeset 500661 c6d3254dcd8e3fc253b9620fb9417e6745a61f57
parent 500660 9eeb9fd126bc068a35cba011ff06e25f1908ab01
child 500662 61c29e7a541969295ba51ef4de6f5771a04cb987
push id114166
push userapavel@mozilla.com
push dateThu, 07 Nov 2019 10:04:01 +0000
treeherdermozilla-inbound@d271c572a9bc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttung, asuth
bugs1168606
milestone72.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 1168606 - Extract common function for discarding cached responses. r=ttung,asuth Depends on D45308 Differential Revision: https://phabricator.services.mozilla.com/D45349
dom/indexedDB/ActorsChild.cpp
dom/indexedDB/ActorsChild.h
--- a/dom/indexedDB/ActorsChild.cpp
+++ b/dom/indexedDB/ActorsChild.cpp
@@ -3330,85 +3330,81 @@ void BackgroundCursorChild::SendContinue
 
   switch (params.type()) {
     case CursorRequestParams::TContinueParams: {
       const auto& key = params.get_ContinueParams().key();
       if (key.IsUnset()) {
         break;
       }
 
-      // Invalidate cache entries.
-      size_t discardedCount = 0;
-      while (!mCachedResponses.empty()) {
-        // This duplicates the logic from the parent. We could avoid this
-        // duplication if we invalidated the cached records always for any
-        // continue-with-key operation, but would lose the benefits of
-        // preloading then.
-        const auto& cachedSortKey =
-            mCursor->IsLocaleAware() ? mCachedResponses.front().mLocaleAwareKey
-                                     : mCachedResponses.front().mKey;
-        const auto& keyOperator = GetKeyOperator(mDirection);
-        if ((cachedSortKey.*keyOperator)(key)) {
-          IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST(
-              "PRELOAD: Continue to key %s, keeping cached key %s/%s and "
-              "further",
-              "Continue/keep", mTransaction->LoggingSerialNumber(),
-              mRequest->LoggingSerialNumber(), key.GetBuffer().get(),
-              mCachedResponses.front().mKey.GetBuffer().get(),
-              mCachedResponses.front().mObjectStoreKey.GetBuffer().get());
-          break;
-        }
-        IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST(
-            "PRELOAD: Continue to key %s, discarding cached key %s/%s",
-            "Continue/discard", mTransaction->LoggingSerialNumber(),
-            mRequest->LoggingSerialNumber(), key.GetBuffer().get(),
-            mCachedResponses.front().mKey.GetBuffer().get(),
-            mCachedResponses.front().mObjectStoreKey.GetBuffer().get());
-        mCachedResponses.pop_front();
-        ++discardedCount;
-      }
-      IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST(
-          "PRELOAD: Discarded %zu cached responses before requested "
-          "continue key, %zu remaining",
-          "Discarding", mTransaction->LoggingSerialNumber(),
-          mRequest->LoggingSerialNumber(), discardedCount,
-          mCachedResponses.size());
+      // Discard cache entries before the target key.
+      DiscardCachedResponses(
+          [&key, isLocaleAware = mCursor->IsLocaleAware(),
+           keyOperator = GetKeyOperator(mDirection),
+           transactionSerialNumber = mTransaction->LoggingSerialNumber(),
+           requestSerialNumber = mRequest->LoggingSerialNumber()](
+              const auto& currentCachedResponse) {
+            // This duplicates the logic from the parent. We could avoid this
+            // duplication if we invalidated the cached records always for any
+            // continue-with-key operation, but would lose the benefits of
+            // preloading then.
+            const auto& cachedSortKey =
+                isLocaleAware ? currentCachedResponse.mLocaleAwareKey
+                              : currentCachedResponse.mKey;
+            const bool discard = !(cachedSortKey.*keyOperator)(key);
+            if (discard) {
+              IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST(
+                  "PRELOAD: Continue to key %s, discarding cached key %s/%s",
+                  "Continue, discarding", transactionSerialNumber,
+                  requestSerialNumber, key.GetBuffer().get(),
+                  cachedSortKey.GetBuffer().get(),
+                  currentCachedResponse.mObjectStoreKey.GetBuffer().get());
+            } else {
+              IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST(
+                  "PRELOAD: Continue to key %s, keeping cached key %s/%s and "
+                  "further",
+                  "Continue, keeping", transactionSerialNumber,
+                  requestSerialNumber, key.GetBuffer().get(),
+                  cachedSortKey.GetBuffer().get(),
+                  currentCachedResponse.mObjectStoreKey.GetBuffer().get());
+            }
+
+            return discard;
+          });
+
       break;
     }
 
     case CursorRequestParams::TContinuePrimaryKeyParams:
       // TODO: Implement preloading for this case
       InvalidateCachedResponses();
       break;
 
     case CursorRequestParams::TAdvanceParams: {
       uint32_t& advanceCount = params.get_AdvanceParams().count();
       IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST(
           "PRELOAD: Advancing %" PRIu32 " records", "Advancing",
           mTransaction->LoggingSerialNumber(), mRequest->LoggingSerialNumber(),
           advanceCount);
 
-      // Invalidate cache entries.
-      size_t discardedCount = 0;
-      while (!mCachedResponses.empty() && advanceCount > 1) {
-        --advanceCount;
-
-        // TODO: We only need to update currentKey on the last entry, the others
-        // are overwritten in the next iteration anyway.
-        currentKey = mCachedResponses.front().mKey;
-        currentObjectStoreKey = mCachedResponses.front().mObjectStoreKey;
-        mCachedResponses.pop_front();
-        ++discardedCount;
-      }
-      IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST(
-          "PRELOAD: Discarded %zu cached responses that are advanced over, "
-          "%zu remaining",
-          "Discarded", mTransaction->LoggingSerialNumber(),
-          mRequest->LoggingSerialNumber(), discardedCount,
-          mCachedResponses.size());
+      // Discard cache entries.
+      DiscardCachedResponses(
+          [&advanceCount, &currentKey,
+           &currentObjectStoreKey](const auto& currentCachedResponse) {
+            const bool res = advanceCount > 1;
+            if (res) {
+              --advanceCount;
+
+              // TODO: We only need to update currentKey on the last entry, the
+              // others are overwritten in the next iteration anyway.
+              currentKey = currentCachedResponse.mKey;
+              currentObjectStoreKey = currentCachedResponse.mObjectStoreKey;
+            }
+            return res;
+          });
       break;
     }
 
     default:
       MOZ_CRASH("Should never get here!");
   }
 
   if (!mCachedResponses.empty()) {
@@ -3513,16 +3509,31 @@ void BackgroundCursorChild::InvalidateCa
   IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST(
       "PRELOAD: Invalidating all %zu cached responses", "Invalidating",
       mTransaction->LoggingSerialNumber(), mRequest->LoggingSerialNumber(),
       mCachedResponses.size());
 
   mCachedResponses.clear();
 }
 
+template <typename Condition>
+void BackgroundCursorChild::DiscardCachedResponses(
+    const Condition& aConditionFunc) {
+  size_t discardedCount = 0;
+  while (!mCachedResponses.empty() &&
+         aConditionFunc(mCachedResponses.front())) {
+    mCachedResponses.pop_front();
+    ++discardedCount;
+  }
+  IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST(
+      "PRELOAD: Discarded %zu cached responses, %zu remaining", "Discarded",
+      mTransaction->LoggingSerialNumber(), mRequest->LoggingSerialNumber(),
+      discardedCount, mCachedResponses.size());
+}
+
 void BackgroundCursorChild::HandleResponse(nsresult aResponse) {
   AssertIsOnOwningThread();
   MOZ_ASSERT(NS_FAILED(aResponse));
   MOZ_ASSERT(NS_ERROR_GET_MODULE(aResponse) == NS_ERROR_MODULE_DOM_INDEXEDDB);
   MOZ_ASSERT(mRequest);
   MOZ_ASSERT(mTransaction);
   MOZ_ASSERT(!mStrongRequest);
   MOZ_ASSERT(!mStrongCursor);
--- a/dom/indexedDB/ActorsChild.h
+++ b/dom/indexedDB/ActorsChild.h
@@ -677,16 +677,19 @@ class BackgroundCursorChild final : publ
   void SendContinueInternal(const CursorRequestParams& aParams,
                             const Key& aCurrentKey,
                             const Key& aCurrentObjectStoreKey);
 
   void SendDeleteMeInternal();
 
   void InvalidateCachedResponses();
 
+  template <typename Condition>
+  void DiscardCachedResponses(const Condition& aConditionFunc);
+
   IDBRequest* GetRequest() const {
     AssertIsOnOwningThread();
 
     return mRequest;
   }
 
   IDBObjectStore* GetObjectStore() const {
     AssertIsOnOwningThread();