Bug 1951283 - Speedup id lookups by using a hash set in BodyDeleteOrphanedFiles. r=dom-storage-reviewers,asuth

The basic functionality remains the same, including the use of
BodyTraverseFile with all its cleanups.

Differential Revision: https://phabricator.services.mozilla.com/D240734

dom/cache/DBSchema.cpp

@@ -727,19 +727,36 @@ Result<int64_t, nsresult> GetTotalDiskUsage(mozIStorageConnection& aConn) {
   QM_TRY_RETURN(MOZ_TO_RESULT_INVOKE_MEMBER(*state, GetInt64, 0));
 }
 
-Result<nsTArray<nsID>, nsresult> GetKnownBodyIds(mozIStorageConnection& aConn) {
+Result<nsTHashSet<nsID>, nsresult> GetKnownBodyIds(
+    mozIStorageConnection& aConn) {
   MOZ_ASSERT(!NS_IsMainThread());
 
+  int32_t numEntries = 0;
+  {
+    QM_TRY_INSPECT(const auto& cnt,
+                   MOZ_TO_RESULT_INVOKE_MEMBER_TYPED(
+                       nsCOMPtr<mozIStorageStatement>, aConn, CreateStatement,
+                       "SELECT COUNT(*) FROM entries;"_ns));
+
+    QM_TRY(quota::CollectWhileHasResult(
+        *cnt, [&numEntries](auto& stmt) -> Result<Ok, nsresult> {
+          QM_TRY(MOZ_TO_RESULT(stmt.GetInt32(0, &numEntries)));
+
+          return Ok{};
+        }));
+  }
+
+  // Each row can have 0 to 2 nsID values, prepare for the maximum.
+  nsTHashSet<nsID> idSet(numEntries * 2);
+
   QM_TRY_INSPECT(
       const auto& state,
       MOZ_TO_RESULT_INVOKE_MEMBER_TYPED(
           nsCOMPtr<mozIStorageStatement>, aConn, CreateStatement,
           "SELECT request_body_id, response_body_id FROM entries;"_ns));
 
-  AutoTArray<nsID, 64> idList;
-
   QM_TRY(quota::CollectWhileHasResult(
-      *state, [&idList](auto& stmt) -> Result<Ok, nsresult> {
+      *state, [&idSet](auto& stmt) -> Result<Ok, nsresult> {
         // extract 0 to 2 nsID structs per row
         for (uint32_t i = 0; i < 2; ++i) {
           QM_TRY_INSPECT(const bool& isNull,
@@ -748,14 +765,14 @@ Result<nsTArray<nsID>, nsresult> GetKnownBodyIds(mozIStorageConnection& aConn) {
           if (!isNull) {
             QM_TRY_INSPECT(const auto& id, ExtractId(stmt, i));
 
-            idList.AppendElement(id);
+            idSet.Insert(id);
           }
         }
 
         return Ok{};
       }));
 
-  return std::move(idList);
+  return std::move(idSet);
 }
 
 Result<Maybe<SavedResponse>, nsresult> CacheMatch(

dom/cache/DBSchema.h

@@ -44,7 +44,8 @@ Result<int64_t, nsresult> FindOverallPaddingSize(mozIStorageConnection& aConn);
 
 Result<int64_t, nsresult> GetTotalDiskUsage(mozIStorageConnection& aConn);
 
-Result<nsTArray<nsID>, nsresult> GetKnownBodyIds(mozIStorageConnection& aConn);
+Result<nsTHashSet<nsID>, nsresult> GetKnownBodyIds(
+    mozIStorageConnection& aConn);
 
 Result<Maybe<SavedResponse>, nsresult> CacheMatch(
     mozIStorageConnection& aConn, CacheId aCacheId,

dom/cache/FileUtils.cpp

@@ -376,7 +376,7 @@ nsresult DirectoryPaddingWrite(nsIFile& aBaseDir,
 
 nsresult BodyDeleteOrphanedFiles(
     const CacheDirectoryMetadata& aDirectoryMetadata, nsIFile& aBaseDir,
-    const nsTArray<nsID>& aKnownBodyIdList) {
+    nsTHashSet<nsID>& aKnownBodyIds) {
   // body files are stored in a directory structure like:
   //
   //  /morgue/01/{01fdddb2-884d-4c3d-95ba-0c8062f6c325}.final
@@ -388,14 +388,14 @@ nsresult BodyDeleteOrphanedFiles(
   // Iterate over all the intermediate morgue subdirs
   QM_TRY(quota::CollectEachFile(
       *dir,
-      [&aDirectoryMetadata, &aKnownBodyIdList](
+      [&aDirectoryMetadata, &aKnownBodyIds](
           const nsCOMPtr<nsIFile>& subdir) -> Result<Ok, nsresult> {
         QM_TRY_INSPECT(const auto& dirEntryKind, GetDirEntryKind(*subdir));
 
         switch (dirEntryKind) {
           case nsIFileKind::ExistsAsDirectory: {
             const auto removeOrphanedFiles =
-                [&aDirectoryMetadata, &aKnownBodyIdList](
+                [&aDirectoryMetadata, &aKnownBodyIds](
                     nsIFile& bodyFile,
                     const nsACString& leafName) -> Result<bool, nsresult> {
               // Finally, parse the uuid out of the name.  If it fails to parse,
@@ -409,7 +409,7 @@ nsresult BodyDeleteOrphanedFiles(
               nsID id;
               QM_TRY(OkIf(id.Parse(leafName.BeginReading())), true);
 
-              if (!aKnownBodyIdList.Contains(id)) {
+              if (!aKnownBodyIds.Contains(id)) {
                 return true;
               }
 

dom/cache/FileUtils.h

@@ -57,9 +57,11 @@ nsresult BodyMaybeUpdatePaddingSize(
 nsresult BodyDeleteFiles(const CacheDirectoryMetadata& aDirectoryMetadata,
                          nsIFile& aBaseDir, const nsTArray<nsID>& aIdList);
 
+// Traverse all cache directorys and do a cleanup, leaving only files that
+// belong to known body ids behind.
 nsresult BodyDeleteOrphanedFiles(
     const CacheDirectoryMetadata& aDirectoryMetadata, nsIFile& aBaseDir,
-    const nsTArray<nsID>& aKnownBodyIdList);
+    nsTHashSet<nsID>& aKnownBodyIds);
 
 // If aCanRemoveFiles is true, that means we are safe to touch the files which
 // can be accessed in other threads.

dom/cache/Manager.cpp

@@ -149,11 +149,13 @@ class SetupAction final : public SyncDBAction {
                 return oldValue + deletionInfo.mDeletedPaddingSize;
               }));
 
-      // Clean up orphaned body objects
+      // Clean up orphaned body objects.
-      QM_TRY_INSPECT(const auto& knownBodyIdList, db::GetKnownBodyIds(*aConn));
+      QM_TRY_UNWRAP(auto knownBodyIds, db::GetKnownBodyIds(*aConn));
 
-      QM_TRY(MOZ_TO_RESULT(BodyDeleteOrphanedFiles(aDirectoryMetadata, *aDBDir,
+      // Note that this causes a scan of all cached files. See bug 1952550 that
-                                                   knownBodyIdList)));
+      // wants to reduce the probability to find the marker file above.
+      QM_TRY(MOZ_TO_RESULT(
+          BodyDeleteOrphanedFiles(aDirectoryMetadata, *aDBDir, knownBodyIds)));
 
       // Commit() explicitly here, because we want to ensure the padding file
       // has the correct content.