Bug 1463017 - Migrate origins frecencies more efficiently. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 28 May 2018 15:22:11 +0200
changeset 420280 eaaf06745eeeabb57932b594777c63d26859ef90
parent 420279 5f5b118631b0a235a75c90c16b75db31d0de0cf0
child 420281 6541e8a5c886eda075983f8800008ab52ab6fd08
push id64590
push usermak77@bonardo.net
push dateTue, 29 May 2018 20:52:00 +0000
treeherderautoland@eaaf06745eee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1463017
milestone62.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 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;