Bug 1463017 - Migrate origins frecencies more efficiently. r=adw draft
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 28 May 2018 15:22:11 +0200
changeset 800584 ed9132563b06dac471ccb0b28554fe30a3d75560
parent 800491 a466172aed4bc2afc21169b749b8068a4b98c93f
child 800831 931f101148023eaa148b5b1dbcec64b3d3f2af3e
push id111411
push usermak77@bonardo.net
push dateMon, 28 May 2018 15:20:49 +0000
reviewersadw
bugs1463017
milestone62.0a1
Bug 1463017 - Migrate origins frecencies more efficiently. r=adw Execute a single write transaction per chunk, rather than a larger read/write transaction. Also removes useless statement scopers when we create a single-use statement. MozReview-Commit-ID: 3G37d55Z1dV
toolkit/components/places/Database.cpp
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -1526,18 +1526,16 @@ Database::EnsureBookmarkRoots(const int3
     int64_t mobileRootId = CreateMobileRoot();
     if (mobileRootId <= 0) return NS_ERROR_FAILURE;
     {
       nsCOMPtr<mozIStorageStatement> mobileRootSyncStatusStmt;
       rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
         "UPDATE moz_bookmarks SET syncStatus = :sync_status WHERE id = :id"
       ), getter_AddRefs(mobileRootSyncStatusStmt));
       if (NS_FAILED(rv)) return rv;
-      mozStorageStatementScoper mobileRootSyncStatusScoper(
-        mobileRootSyncStatusStmt);
 
       rv = mobileRootSyncStatusStmt->BindInt32ByName(
         NS_LITERAL_CSTRING("sync_status"),
         nsINavBookmarksService::SYNC_STATUS_NEW
       );
       if (NS_FAILED(rv)) return rv;
       rv = mobileRootSyncStatusStmt->BindInt64ByName(NS_LITERAL_CSTRING("id"),
                                                      mobileRootId);
@@ -1668,30 +1666,28 @@ Database::MigrateV32Up() {
   {
     nsCOMPtr<mozIStorageStatement> stmt;
     rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
       "INSERT OR IGNORE INTO moz_migrate_v32_temp (host) "
         "SELECT fixup_url(get_unreversed_host(rev_host)) "
         "FROM moz_places WHERE LENGTH(url) > :maxlen AND foreign_count = 0"
     ), getter_AddRefs(stmt));
     NS_ENSURE_SUCCESS(rv, rv);
-    mozStorageStatementScoper scoper(stmt);
     rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("maxlen"), MaxUrlLength());
     NS_ENSURE_SUCCESS(rv, rv);
     rv = stmt->Execute();
     NS_ENSURE_SUCCESS(rv, rv);
   }
   // Now remove the pages with a long url.
   {
     nsCOMPtr<mozIStorageStatement> stmt;
     rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
       "DELETE FROM moz_places WHERE LENGTH(url) > :maxlen AND foreign_count = 0"
     ), getter_AddRefs(stmt));
     NS_ENSURE_SUCCESS(rv, rv);
-    mozStorageStatementScoper scoper(stmt);
     rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("maxlen"), MaxUrlLength());
     NS_ENSURE_SUCCESS(rv, rv);
     rv = stmt->Execute();
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   // Expire orphan visits and update moz_hosts.
   // These may be a bit more expensive and are not critical for the DB
@@ -1819,17 +1815,16 @@ Database::MigrateV35Up() {
     // Either the schema is broken or there isn't any root. The latter can
     // happen if a consumer, for example Thunderbird, never used bookmarks.
     // If there are no roots, this migration should not run.
     nsCOMPtr<mozIStorageStatement> checkRootsStmt;
     nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
       "SELECT id FROM moz_bookmarks WHERE parent = 0"
     ), getter_AddRefs(checkRootsStmt));
     NS_ENSURE_SUCCESS(rv, rv);
-    mozStorageStatementScoper scoper(checkRootsStmt);
     bool hasResult = false;
     rv = checkRootsStmt->ExecuteStep(&hasResult);
     if (NS_SUCCEEDED(rv) && !hasResult) {
       return NS_OK;
     }
     return NS_ERROR_FAILURE;
   }
 
@@ -1856,17 +1851,16 @@ Database::MigrateV35Up() {
       "UPDATE moz_bookmarks "
       "SET parent = :root_id, "
           "position = position + IFNULL("
             "(SELECT MAX(position) + 1 FROM moz_bookmarks "
              "WHERE parent = :root_id), 0)"
       "WHERE parent = :folder_id"
     ), getter_AddRefs(moveStmt));
     if (NS_FAILED(rv)) return rv;
-    mozStorageStatementScoper moveScoper(moveStmt);
 
     rv = moveStmt->BindInt64ByName(NS_LITERAL_CSTRING("root_id"),
                                    mobileRootId);
     if (NS_FAILED(rv)) return rv;
     rv = moveStmt->BindInt64ByName(NS_LITERAL_CSTRING("folder_id"),
                                    folderIds[i]);
     if (NS_FAILED(rv)) return rv;
 
@@ -2071,17 +2065,16 @@ Database::MigrateV42Up() {
   // auto_vacuum of the favicons database was broken, we may have to set it again.
   int32_t vacuum = 0;
   {
     nsCOMPtr<mozIStorageStatement> stmt;
     nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
       "PRAGMA favicons.auto_vacuum"
     ), getter_AddRefs(stmt));
     NS_ENSURE_SUCCESS(rv, rv);
-    mozStorageStatementScoper scoper(stmt);
     bool hasResult = false;
     if (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
       vacuum = stmt->AsInt32(0);
     }
   }
   if (vacuum != 2) {
     nsresult rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
       "PRAGMA favicons.auto_vacuum = INCREMENTAL"));
@@ -2337,40 +2330,40 @@ Database::MigrateV48Up() {
   nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
     "SELECT * FROM moz_origins; "
   ), getter_AddRefs(stmt));
   if (NS_FAILED(rv)) {
     rv = mMainConn->ExecuteSimpleSQL(CREATE_MOZ_ORIGINS);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
-    "INSERT OR IGNORE INTO moz_origins (prefix, host, frecency) " \
-    "SELECT get_prefix(url), get_host_and_port(url), -1 " \
+    "INSERT OR IGNORE INTO moz_origins (prefix, host, frecency) "
+    "SELECT get_prefix(url), get_host_and_port(url), -1 "
     "FROM moz_places; "
   ));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Add and populate moz_places.origin_id.
   rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
     "SELECT origin_id FROM moz_places; "
   ), getter_AddRefs(stmt));
   if (NS_FAILED(rv)) {
     rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
-      "ALTER TABLE moz_places " \
+      "ALTER TABLE moz_places "
       "ADD COLUMN origin_id INTEGER REFERENCES moz_origins(id); "
     ));
     NS_ENSURE_SUCCESS(rv, rv);
   }
   rv = mMainConn->ExecuteSimpleSQL(CREATE_IDX_MOZ_PLACES_ORIGIN_ID);
   NS_ENSURE_SUCCESS(rv, rv);
   rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
-    "UPDATE moz_places " \
+    "UPDATE moz_places "
     "SET origin_id = ( "
-      "SELECT id FROM moz_origins " \
-      "WHERE prefix = get_prefix(url) AND host = get_host_and_port(url) " \
+      "SELECT id FROM moz_origins "
+      "WHERE prefix = get_prefix(url) AND host = get_host_and_port(url) "
     "); "
   ));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Setting this pref will cause InitSchema to begin async migration of
   // frecencies to moz_origins.  The reason we don't defer the other steps
   // above, like we do this one here, is because we want to make sure that the
   // main data in moz_origins, prefix and host, are coherent in relation to
@@ -2409,51 +2402,43 @@ NS_IMETHODIMP
 MigrateV48FrecenciesRunnable::Run()
 {
   if (NS_IsMainThread()) {
     // Migration done.  Clear the pref.
     Unused << Preferences::ClearUser(PREF_MIGRATE_V48_FRECENCIES);
     return NS_OK;
   }
 
+  // We do the work in chunks, or the wal journal may grow too much.
+  nsCOMPtr<mozIStorageStatement> updateStmt;
+  nsresult rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "UPDATE moz_origins "
+    "SET frecency = ( "
+      "SELECT MAX(frecency) "
+      "FROM moz_places "
+      "WHERE moz_places.origin_id = moz_origins.id "
+    ") "
+    "WHERE rowid IN ( "
+      "SELECT rowid "
+      "FROM moz_origins "
+      "WHERE frecency = -1 "
+      "LIMIT 400 "
+    ") "
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   nsCOMPtr<mozIStorageStatement> selectStmt;
-  nsresult rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-    "SELECT id FROM moz_origins " \
-    "WHERE frecency = -1 " \
-    "ORDER BY id ASC " \
-    "LIMIT 200; "
+  rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
+    "SELECT id FROM moz_origins WHERE frecency = -1 "
   ), getter_AddRefs(selectStmt));
   NS_ENSURE_SUCCESS(rv, rv);
-
-  nsCOMPtr<mozIStorageStatement> updateStmt;
-  rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-    "UPDATE moz_origins " \
-    "SET frecency = ( " \
-      "SELECT MAX(frecency) " \
-      "FROM moz_places " \
-      "WHERE moz_places.origin_id = moz_origins.id " \
-    ") " \
-    "WHERE id = :id; "
-  ), getter_AddRefs(updateStmt));
+  bool hasResult = false;
+  rv = selectStmt->ExecuteStep(&hasResult);
   NS_ENSURE_SUCCESS(rv, rv);
-
-  mozStorageStatementScoper updateScoper(updateStmt);
-
-  // We should do the work in chunks, or the wal journal may grow too much.
-  bool hasResult;
-  uint8_t count = 0;
-  for (; NS_SUCCEEDED(selectStmt->ExecuteStep(&hasResult)) && hasResult; ++count) {
-    int64_t id = selectStmt->AsInt64(0);
-    rv = updateStmt->BindInt64ByName(NS_LITERAL_CSTRING("id"), id);
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv = updateStmt->Execute();
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
-  if (count == 200) {
+  if (hasResult) {
     // There are more results to handle. Re-dispatch to the same thread for the
     // next chunk.
     return NS_DispatchToCurrentThread(this);
   }
 
   // Re-dispatch to the main-thread to flip the migration pref.
   return NS_DispatchToMainThread(this);
 }
@@ -2660,17 +2645,16 @@ Database::GetItemsWithAnno(const nsACStr
   nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
     "SELECT b.id FROM moz_items_annos a "
     "JOIN moz_anno_attributes n ON n.id = a.anno_attribute_id "
     "JOIN moz_bookmarks b ON b.id = a.item_id "
     "WHERE n.name = :anno_name AND "
           "b.type = :item_type"
   ), getter_AddRefs(stmt));
   if (NS_FAILED(rv)) return rv;
-  mozStorageStatementScoper scoper(stmt);
 
   rv = stmt->BindUTF8StringByName(NS_LITERAL_CSTRING("anno_name"), aAnnoName);
   if (NS_FAILED(rv)) return rv;
   rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("item_type"), aItemType);
   if (NS_FAILED(rv)) return rv;
 
   bool hasMore = false;
   while (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
@@ -2687,32 +2671,30 @@ nsresult
 Database::DeleteBookmarkItem(int32_t aItemId)
 {
   // Delete the old bookmark.
   nsCOMPtr<mozIStorageStatement> deleteStmt;
   nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
     "DELETE FROM moz_bookmarks WHERE id = :item_id"
   ), getter_AddRefs(deleteStmt));
   if (NS_FAILED(rv)) return rv;
-  mozStorageStatementScoper deleteScoper(deleteStmt);
 
   rv = deleteStmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"),
                                    aItemId);
   if (NS_FAILED(rv)) return rv;
 
   rv = deleteStmt->Execute();
   if (NS_FAILED(rv)) return rv;
 
   // Clean up orphan annotations.
   nsCOMPtr<mozIStorageStatement> removeAnnosStmt;
   rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
     "DELETE FROM moz_items_annos WHERE item_id = :item_id"
   ), getter_AddRefs(removeAnnosStmt));
   if (NS_FAILED(rv)) return rv;
-  mozStorageStatementScoper removeAnnosScoper(removeAnnosStmt);
 
   rv = removeAnnosStmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"),
                                         aItemId);
   if (NS_FAILED(rv)) return rv;
 
   rv = removeAnnosStmt->Execute();
   if (NS_FAILED(rv)) return rv;
 
@@ -2730,17 +2712,16 @@ Database::CreateMobileRoot()
   nsresult rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
     "INSERT OR IGNORE INTO moz_bookmarks "
       "(type, title, dateAdded, lastModified, guid, position, parent) "
     "SELECT :item_type, :item_title, :timestamp, :timestamp, :guid, "
       "IFNULL((SELECT MAX(position) + 1 FROM moz_bookmarks p WHERE p.parent = b.id), 0), b.id "
     "FROM moz_bookmarks b WHERE b.parent = 0"
   ), getter_AddRefs(createStmt));
   if (NS_FAILED(rv)) return -1;
-  mozStorageStatementScoper createScoper(createStmt);
 
   rv = createStmt->BindInt32ByName(NS_LITERAL_CSTRING("item_type"),
                                    nsINavBookmarksService::TYPE_FOLDER);
   if (NS_FAILED(rv)) return -1;
   rv = createStmt->BindUTF8StringByName(NS_LITERAL_CSTRING("item_title"),
                                         NS_LITERAL_CSTRING(MOBILE_ROOT_TITLE));
   if (NS_FAILED(rv)) return -1;
   rv = createStmt->BindInt64ByName(NS_LITERAL_CSTRING("timestamp"),
@@ -2755,17 +2736,16 @@ Database::CreateMobileRoot()
 
   // Find the mobile root ID. We can't use the last inserted ID because the
   // root might already exist, and we ignore on conflict.
   nsCOMPtr<mozIStorageStatement> findIdStmt;
   rv = mMainConn->CreateStatement(NS_LITERAL_CSTRING(
     "SELECT id FROM moz_bookmarks WHERE guid = :guid"
   ), getter_AddRefs(findIdStmt));
   if (NS_FAILED(rv)) return -1;
-  mozStorageStatementScoper findIdScoper(findIdStmt);
 
   rv = findIdStmt->BindUTF8StringByName(NS_LITERAL_CSTRING("guid"),
                                         NS_LITERAL_CSTRING(MOBILE_ROOT_GUID));
   if (NS_FAILED(rv)) return -1;
 
   bool hasResult = false;
   rv = findIdStmt->ExecuteStep(&hasResult);
   if (NS_FAILED(rv) || !hasResult) return -1;