Bug 1346554 - Incremental vacuum favicons.sqlite. r=standard8 draft
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 18 Jan 2018 15:06:39 +0100
changeset 737990 11921dd6178b173deaa28f40b5ceef830ded2ea1
parent 737939 6602576987baec9edbaaad117114ba5227db6261
push id96818
push usermak77@bonardo.net
push dateThu, 25 Jan 2018 15:14:31 +0000
reviewersstandard8
bugs1346554
milestone60.0a1
Bug 1346554 - Incremental vacuum favicons.sqlite. r=standard8 MozReview-Commit-ID: 9bcCgkF5uU1
toolkit/components/places/Database.cpp
toolkit/components/places/Database.h
toolkit/components/places/PlacesDBUtils.jsm
toolkit/components/places/tests/favicons/test_incremental_vacuum.js
toolkit/components/places/tests/favicons/xpcshell.ini
toolkit/components/places/tests/head_common.js
toolkit/components/places/tests/migration/favicons_v41.sqlite
toolkit/components/places/tests/migration/head_migration.js
toolkit/components/places/tests/migration/test_current_from_v38.js
toolkit/components/places/tests/migration/test_current_from_v41.js
toolkit/components/places/tests/migration/xpcshell.ini
toolkit/components/places/tests/unit/test_telemetry.js
toolkit/components/telemetry/Histograms.json
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -370,16 +370,17 @@ NS_IMPL_ISUPPORTS(Database
 Database::Database()
   : mMainThreadStatements(mMainConn)
   , mMainThreadAsyncStatements(mMainConn)
   , mAsyncThreadStatements(mMainConn)
   , mDBPageSize(0)
   , mDatabaseStatus(nsINavHistoryService::DATABASE_STATUS_OK)
   , mClosed(false)
   , mShouldConvertIconPayloads(false)
+  , mShouldVacuumIcons(false)
   , mClientsShutdown(new ClientsShutdownBlocker())
   , mConnectionShutdown(new ConnectionShutdownBlocker(this))
   , mMaxUrlLength(0)
   , mCacheObservers(TOPIC_PLACES_INIT_COMPLETE)
 {
   MOZ_ASSERT(!XRE_IsContentProcess(),
              "Cannot instantiate Places in the content process");
   // Attempting to create two instances of the service?
@@ -697,30 +698,30 @@ Database::EnsureFaviconsDatabaseFile(nsC
     // Ensure we'll close the connection when done.
     auto cleanup = MakeScopeExit([&] () {
       // We cannot use AsyncClose() here, because by the time we try to ATTACH
       // this database, its transaction could be still be running and that would
       // cause the ATTACH query to fail.
       MOZ_ALWAYS_TRUE(NS_SUCCEEDED(conn->Close()));
     });
 
-    int32_t defaultPageSize;
-    rv = conn->GetDefaultPageSize(&defaultPageSize);
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv = SetupDurability(conn, defaultPageSize);
-    NS_ENSURE_SUCCESS(rv, rv);
-
     // Enable incremental vacuum for this database. Since it will contain even
     // large blobs and can be cleared with history, it's worth to have it.
     // Note that it will be necessary to manually use PRAGMA incremental_vacuum.
     rv = conn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
       "PRAGMA auto_vacuum = INCREMENTAL"
     ));
     NS_ENSURE_SUCCESS(rv, rv);
 
+    int32_t defaultPageSize;
+    rv = conn->GetDefaultPageSize(&defaultPageSize);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = SetupDurability(conn, defaultPageSize);
+    NS_ENSURE_SUCCESS(rv, rv);
+
     // We are going to update the database, so everything from now on should be
     // in a transaction for performances.
     mozStorageTransaction transaction(conn, false);
     rv = conn->ExecuteSimpleSQL(CREATE_MOZ_ICONS);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = conn->ExecuteSimpleSQL(CREATE_IDX_MOZ_ICONS_ICONURLHASH);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = conn->ExecuteSimpleSQL(CREATE_MOZ_PAGES_W_ICONS);
@@ -1085,16 +1086,31 @@ Database::InitSchema(bool* aDatabaseMigr
   NS_ENSURE_SUCCESS(rv, rv);
   bool databaseInitialized = currentSchemaVersion > 0;
 
   if (databaseInitialized && currentSchemaVersion == DATABASE_SCHEMA_VERSION) {
     // The database is up to date and ready to go.
     return NS_OK;
   }
 
+  auto guard = MakeScopeExit([&]() {
+    // This runs at the end of the migration, out of the transaction,
+    // regardless of its success.
+    if (mShouldVacuumIcons) {
+      mShouldVacuumIcons = false;
+      MOZ_ALWAYS_SUCCEEDS(mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+        "VACUUM favicons"
+      )));
+    }
+    if (mShouldConvertIconPayloads) {
+      mShouldConvertIconPayloads = false;
+      nsFaviconService::ConvertUnsupportedPayloads(mMainConn);
+    }
+  });
+
   // We are going to update the database, so everything from now on should be in
   // a transaction for performances.
   mozStorageTransaction transaction(mMainConn, false);
 
   if (databaseInitialized) {
     // Migration How-to:
     //
     // 1. increment PLACES_SCHEMA_VERSION.
@@ -1113,24 +1129,16 @@ Database::InitSchema(bool* aDatabaseMigr
         // These are versions older than Firefox 45 that are not supported
         // anymore.  In this case it's safer to just replace the database.
         // Note that Firefox 45 is the ESR release before the latest one (52),
         // and Firefox 48 is a watershed release, so any version older than 48
         // will first have to go through it.
         return NS_ERROR_FILE_CORRUPTED;
       }
 
-      auto guard = MakeScopeExit([&]() {
-        // This runs at the end of the migration, regardless of its success.
-        if (mShouldConvertIconPayloads) {
-          mShouldConvertIconPayloads = false;
-          nsFaviconService::ConvertUnsupportedPayloads(mMainConn);
-        }
-      });
-
       // Firefox 45 ESR uses schema version 30.
 
       if (currentSchemaVersion < 31) {
         rv = MigrateV31Up();
         NS_ENSURE_SUCCESS(rv, rv);
       }
 
       // Firefox 48 uses schema version 31.
@@ -1196,16 +1204,23 @@ Database::InitSchema(bool* aDatabaseMigr
 
       if (currentSchemaVersion < 41) {
         rv = MigrateV41Up();
         NS_ENSURE_SUCCESS(rv, rv);
       }
 
       // Firefox 58 uses schema version 41.
 
+      if (currentSchemaVersion < 42) {
+        rv = MigrateV42Up();
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+
+      // Firefox 60 uses schema version 42.
+
       // Schema Upgrades must add migration code here.
       // >>> IMPORTANT! <<<
       // NEVER MIX UP SYNC AND ASYNC EXECUTION IN MIGRATORS, YOU MAY LOCK THE
       // CONNECTION AND CAUSE FURTHER STEPS TO FAIL.
       // In case, set a bool and do the async work in the ScopeExit guard just
       // before the migration steps.
 
       rv = UpdateBookmarkRootTitles();
@@ -1949,16 +1964,43 @@ Database::MigrateV41Up() {
   NS_ENSURE_SUCCESS(rv, rv);
   rv = mMainConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
     "DROP TABLE IF EXISTS moz_favicons"));
   NS_ENSURE_SUCCESS(rv, rv);
   return NS_OK;
 }
 
 nsresult
+Database::MigrateV42Up() {
+  MOZ_ASSERT(NS_IsMainThread());
+  // 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"));
+    NS_ENSURE_SUCCESS(rv, rv);
+    // For the change to be effective, we must vacuum the database.
+    mShouldVacuumIcons = true;
+  }
+  return NS_OK;
+}
+
+nsresult
 Database::GetItemsWithAnno(const nsACString& aAnnoName, int32_t aItemType,
                            nsTArray<int64_t>& aItemIds)
 {
   nsCOMPtr<mozIStorageStatement> stmt;
   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 "
--- a/toolkit/components/places/Database.h
+++ b/toolkit/components/places/Database.h
@@ -14,17 +14,17 @@
 #include "mozilla/storage/StatementCache.h"
 #include "mozilla/Attributes.h"
 #include "nsIEventTarget.h"
 #include "Shutdown.h"
 #include "nsCategoryCache.h"
 
 // This is the schema version. Update it at any schema change and add a
 // corresponding migrateVxx method below.
-#define DATABASE_SCHEMA_VERSION 41
+#define DATABASE_SCHEMA_VERSION 42
 
 // Fired after Places inited.
 #define TOPIC_PLACES_INIT_COMPLETE "places-init-complete"
 // This topic is received when the profile is about to be lost.  Places does
 // initial shutdown work and notifies TOPIC_PLACES_SHUTDOWN to all listeners.
 // Any shutdown work that requires the Places APIs should happen here.
 #define TOPIC_PROFILE_CHANGE_TEARDOWN "profile-change-teardown"
 // Fired when Places is shutting down.  Any code should stop accessing Places
@@ -297,16 +297,17 @@ protected:
   nsresult MigrateV34Up();
   nsresult MigrateV35Up();
   nsresult MigrateV36Up();
   nsresult MigrateV37Up();
   nsresult MigrateV38Up();
   nsresult MigrateV39Up();
   nsresult MigrateV40Up();
   nsresult MigrateV41Up();
+  nsresult MigrateV42Up();
 
   nsresult UpdateBookmarkRootTitles();
 
   friend class ConnectionShutdownBlocker;
 
   int64_t CreateMobileRoot();
   nsresult GetItemsWithAnno(const nsACString& aAnnoName, int32_t aItemType,
                             nsTArray<int64_t>& aItemIds);
@@ -329,16 +330,19 @@ private:
   mutable StatementCache mAsyncThreadStatements;
 
   int32_t mDBPageSize;
   uint16_t mDatabaseStatus;
   bool mClosed;
   // Used to track whether icon payloads should be converted at the end of
   // schema migration.
   bool mShouldConvertIconPayloads;
+  // Used to track whether the favicons database should be vacuumed at the end
+  // of the schema migration.
+  bool mShouldVacuumIcons;
 
   /**
    * Phases for shutting down the Database.
    * See Shutdown.h for further details about the shutdown procedure.
    */
   already_AddRefed<nsIAsyncShutdownClient> GetProfileChangeTeardownPhase();
   already_AddRefed<nsIAsyncShutdownClient> GetProfileBeforeChangePhase();
 
--- a/toolkit/components/places/PlacesDBUtils.jsm
+++ b/toolkit/components/places/PlacesDBUtils.jsm
@@ -3,19 +3,21 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 const BYTES_PER_MEBIBYTE = 1048576;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://gre/modules/Services.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
-                                  "resource://gre/modules/PlacesUtils.jsm");
+XPCOMUtils.defineLazyModuleGetters(this, {
+  Services: "resource://gre/modules/Services.jsm",
+  OS: "resource://gre/modules/osfile.jsm",
+  PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
+});
 
 this.EXPORTED_SYMBOLS = [ "PlacesDBUtils" ];
 
 this.PlacesDBUtils = {
   _isShuttingDown: false,
   shutdown() {
     PlacesDBUtils._isShuttingDown = true;
   },
@@ -32,17 +34,18 @@ this.PlacesDBUtils = {
    *         - succeeded: boolean
    *         - logs: an array of strings containing the messages logged by the task.
    */
   async maintenanceOnIdle() {
     let tasks = [
       this.checkIntegrity,
       this.invalidateCaches,
       this.checkCoherence,
-      this._refreshUI
+      this._refreshUI,
+      this.incrementalVacuum
     ];
     let telemetryStartTime = Date.now();
     let taskStatusMap = await PlacesDBUtils.runTasks(tasks);
 
     Services.prefs.setIntPref("places.database.lastMaintenance",
                                parseInt(Date.now() / 1000));
     Services.telemetry.getHistogramById("PLACES_IDLE_MAINTENANCE_TIME_MS")
                       .add(Date.now() - telemetryStartTime);
@@ -108,17 +111,17 @@ this.PlacesDBUtils = {
       await db.execute("PRAGMA integrity_check", null, (r, cancel) => {
         row = r;
         cancel();
       });
       return row.getResultByIndex(0) === "ok";
     }
     try {
       // Run a integrity check, but stop at the first error.
-      await PlacesUtils.withConnectionWrapper("PlacesDBUtils: check the integrity", async (db) => {
+      await PlacesUtils.withConnectionWrapper("PlacesDBUtils: check the integrity", async db => {
         let isOk = await integrity(db);
         if (isOk) {
           logs.push("The database is sane");
         } else {
           // We stopped due to an integrity corruption, try to fix if possible.
           logs.push("The database is corrupt");
           // Try to reindex, this often fixes simple indices corruption.
           await db.execute("REINDEX");
@@ -139,38 +142,33 @@ this.PlacesDBUtils = {
         // There was some other error, so throw.
         PlacesDBUtils.clearPendingTasks();
         throw new Error("Unable to check database integrity");
       }
     }
     return logs;
   },
 
-  async invalidateCaches() {
+  invalidateCaches() {
     let logs = [];
-    try {
-      await PlacesUtils.withConnectionWrapper(
-        "PlacesDBUtils: invalidate caches",
-        async (db) => {
-          let idsWithInvalidGuidsRows = await db.execute(`
-            SELECT id FROM moz_bookmarks
-            WHERE guid IS NULL OR
-                  NOT IS_VALID_GUID(guid)`);
-          for (let row of idsWithInvalidGuidsRows) {
-            let id = row.getResultByName("id");
-            PlacesUtils.invalidateCachedGuidFor(id);
-          }
-        }
-      );
+    return PlacesUtils.withConnectionWrapper("PlacesDBUtils: invalidate caches", async db => {
+      let idsWithInvalidGuidsRows = await db.execute(`
+        SELECT id FROM moz_bookmarks
+        WHERE guid IS NULL OR
+              NOT IS_VALID_GUID(guid)`);
+      for (let row of idsWithInvalidGuidsRows) {
+        let id = row.getResultByName("id");
+        PlacesUtils.invalidateCachedGuidFor(id);
+      }
       logs.push("The caches have been invalidated");
-    } catch (ex) {
+      return logs;
+    }).catch(ex => {
       PlacesDBUtils.clearPendingTasks();
       throw new Error("Unable to invalidate caches");
-    }
-    return logs;
+    });
   },
 
   /**
    * Checks data coherence and tries to fix most common errors.
    *
    * @return {Promise} resolves when coherence is checked.
    * @resolves to an array of logs for this task.
    * @rejects if database is not coherent.
@@ -197,16 +195,43 @@ this.PlacesDBUtils = {
       logs.push("The database is coherent");
     } else {
       PlacesDBUtils.clearPendingTasks();
       throw new Error("Unable to complete the coherence check");
     }
     return logs;
   },
 
+  /**
+   * Runs incremental vacuum on databases supporting it.
+   *
+   * @return {Promise} resolves when done.
+   * @resolves to an array of logs for this task.
+   * @rejects if we were unable to vacuum.
+   */
+  async incrementalVacuum() {
+    let logs = [];
+    return PlacesUtils.withConnectionWrapper("PlacesDBUtils: incrementalVacuum",
+      async db => {
+        let count = (await db.execute("PRAGMA favicons.freelist_count"))[0].getResultByIndex(0);
+        if (count < 10) {
+          logs.push(`The favicons database has only ${count} free pages, not vacuuming.`);
+        } else {
+          logs.push(`The favicons database has ${count} free pages, vacuuming.`);
+          await db.execute("PRAGMA favicons.incremental_vacuum");
+          count = (await db.execute("PRAGMA favicons.freelist_count"))[0].getResultByIndex(0);
+          logs.push(`The database has been vacuumed and has now ${count} free pages.`);
+        }
+        return logs;
+      }).catch(ex => {
+        PlacesDBUtils.clearPendingTasks();
+        throw new Error("Unable to incrementally vacuum the favicons database " + ex);
+      });
+  },
+
   async _getCoherenceStatements() {
     let updateRootTitleSql = `UPDATE moz_bookmarks SET title = :title
                               WHERE id = :root_id AND title <> :title`;
     let cleanupStatements = [
       // MOZ_ANNO_ATTRIBUTES
       // A.1 remove obsolete annotations from moz_annos.
       // The 'weave0' idiom exploits character ordering (0 follows /) to
       // efficiently select all annos with a 'weave/' prefix.
@@ -789,35 +814,29 @@ this.PlacesDBUtils = {
    * allow us to maintain a simple, consistent API for the tasks within this object.
    *
    * @return {Promise} resolves when database is vacuumed.
    * @resolves to an array of logs for this task.
    * @rejects if we are unable to vacuum database.
    */
   async vacuum() {
     let logs = [];
-    let DBFile = Services.dirsvc.get("ProfD", Ci.nsIFile);
-    DBFile.append("places.sqlite");
-    logs.push("Initial database size is " +
-                parseInt(DBFile.fileSize / 1024) + " KiB");
-    return PlacesUtils.withConnectionWrapper(
-      "PlacesDBUtils: vacuum",
-      async (db) => {
-        await db.execute("VACUUM");
-      }).then(() => {
-        logs.push("The database has been vacuumed");
-        let vacuumedDBFile = Services.dirsvc.get("ProfD", Ci.nsIFile);
-        vacuumedDBFile.append("places.sqlite");
-        logs.push("Final database size is " +
-                   parseInt(vacuumedDBFile.fileSize / 1024) + " KiB");
-        return logs;
-      }).catch(() => {
-        PlacesDBUtils.clearPendingTasks();
-        throw new Error("Unable to vacuum database");
-      });
+    let placesDbPath = OS.Path.join(OS.Constants.Path.profileDir, "places.sqlite");
+    let info = await OS.File.stat(placesDbPath);
+    logs.push(`Initial database size is ${parseInt(info.size / 1024)}KiB`);
+    return PlacesUtils.withConnectionWrapper("PlacesDBUtils: vacuum", async db => {
+      await db.execute("VACUUM");
+      logs.push("The database has been vacuumed");
+      info = await OS.File.stat(placesDbPath);
+      logs.push(`Final database size is ${parseInt(info.size / 1024)}KiB`);
+      return logs;
+    }).catch(() => {
+      PlacesDBUtils.clearPendingTasks();
+      throw new Error("Unable to vacuum database");
+    });
   },
 
   /**
    * Forces a full expiration on the database.
    *
    * Note: although this function isn't actually async, we keep it async to
    * allow us to maintain a simple, consistent API for the tasks within this object.
    *
@@ -848,38 +867,39 @@ this.PlacesDBUtils = {
    * Collects statistical data on the database.
    *
    * @return {Promise} resolves when statistics are collected.
    * @resolves to an array of logs for this task.
    * @rejects if we are unable to collect stats for some reason.
    */
   async stats() {
     let logs = [];
-    let DBFile = Services.dirsvc.get("ProfD", Ci.nsIFile);
-    DBFile.append("places.sqlite");
-    logs.push("Database size is " + parseInt(DBFile.fileSize / 1024) + " KiB");
+    let placesDbPath = OS.Path.join(OS.Constants.Path.profileDir, "places.sqlite");
+    let info = await OS.File.stat(placesDbPath);
+    logs.push(`Places.sqlite size is ${parseInt(info.size / 1024)}KiB`);
+    let faviconsDbPath = OS.Path.join(OS.Constants.Path.profileDir, "favicons.sqlite");
+    info = await OS.File.stat(faviconsDbPath);
+    logs.push(`Favicons.sqlite size is ${parseInt(info.size / 1024)}KiB`);
 
     // Execute each step async.
     let pragmas = [ "user_version",
                     "page_size",
                     "cache_size",
                     "journal_mode",
                     "synchronous"
                   ].map(p => `pragma_${p}`);
     let pragmaQuery = `SELECT * FROM ${ pragmas.join(", ") }`;
-    await PlacesUtils.withConnectionWrapper(
-      "PlacesDBUtils: pragma for stats",
-      async (db) => {
-        let row = (await db.execute(pragmaQuery))[0];
-        for (let i = 0; i != pragmas.length; i++) {
-          logs.push(`${ pragmas[i] } is ${ row.getResultByIndex(i) }`);
-        }
-      }).catch(() => {
-        logs.push("Could not set pragma for stat collection");
-      });
+    await PlacesUtils.withConnectionWrapper("PlacesDBUtils: pragma for stats", async db => {
+      let row = (await db.execute(pragmaQuery))[0];
+      for (let i = 0; i != pragmas.length; i++) {
+        logs.push(`${ pragmas[i] } is ${ row.getResultByIndex(i) }`);
+      }
+    }).catch(() => {
+      logs.push("Could not set pragma for stat collection");
+    });
 
     // Get maximum number of unique URIs.
     try {
       let limitURIs = Services.prefs.getIntPref(
         "places.history.expiration.transient_current_max_pages");
       logs.push("History can store a maximum of " + limitURIs + " unique pages");
     } catch (ex) {}
 
@@ -945,55 +965,75 @@ this.PlacesDBUtils = {
     let probes = [
       { histogram: "PLACES_PAGES_COUNT",
         query:     "SELECT count(*) FROM moz_places" },
 
       { histogram: "PLACES_BOOKMARKS_COUNT",
         query:     `SELECT count(*) FROM moz_bookmarks b
                     JOIN moz_bookmarks t ON t.id = b.parent
                     AND t.parent <> :tags_folder
-                    WHERE b.type = :type_bookmark` },
+                    WHERE b.type = :type_bookmark`,
+        params: {
+          tags_folder: PlacesUtils.tagsFolderId,
+          type_bookmark: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+        }
+      },
 
       { histogram: "PLACES_TAGS_COUNT",
         query:     `SELECT count(*) FROM moz_bookmarks
-                    WHERE parent = :tags_folder` },
+                    WHERE parent = :tags_folder`,
+        params: {
+          tags_folder: PlacesUtils.tagsFolderId,
+        }
+      },
 
       { histogram: "PLACES_KEYWORDS_COUNT",
         query:     "SELECT count(*) FROM moz_keywords" },
 
       { histogram: "PLACES_SORTED_BOOKMARKS_PERC",
         query:     `SELECT IFNULL(ROUND((
                       SELECT count(*) FROM moz_bookmarks b
                       JOIN moz_bookmarks t ON t.id = b.parent
                       AND t.parent <> :tags_folder AND t.parent > :places_root
                       WHERE b.type  = :type_bookmark
                       ) * 100 / (
                       SELECT count(*) FROM moz_bookmarks b
                       JOIN moz_bookmarks t ON t.id = b.parent
                       AND t.parent <> :tags_folder
                       WHERE b.type = :type_bookmark
-                    )), 0)` },
+                    )), 0)`,
+        params: {
+          places_root: PlacesUtils.placesRootId,
+          tags_folder: PlacesUtils.tagsFolderId,
+          type_bookmark: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+        }
+      },
 
       { histogram: "PLACES_TAGGED_BOOKMARKS_PERC",
         query:     `SELECT IFNULL(ROUND((
                       SELECT count(*) FROM moz_bookmarks b
                       JOIN moz_bookmarks t ON t.id = b.parent
                       AND t.parent = :tags_folder
                       ) * 100 / (
                       SELECT count(*) FROM moz_bookmarks b
                       JOIN moz_bookmarks t ON t.id = b.parent
                       AND t.parent <> :tags_folder
                       WHERE b.type = :type_bookmark
-                    )), 0)` },
+                    )), 0)`,
+        params: {
+          tags_folder: PlacesUtils.tagsFolderId,
+          type_bookmark: PlacesUtils.bookmarks.TYPE_BOOKMARK,
+        }
+      },
 
       { histogram: "PLACES_DATABASE_FILESIZE_MB",
-        callback() {
-          let DBFile = Services.dirsvc.get("ProfD", Ci.nsIFile);
-          DBFile.append("places.sqlite");
-          return parseInt(DBFile.fileSize / BYTES_PER_MEBIBYTE);
+        async callback() {
+          let placesDbPath = OS.Path.join(OS.Constants.Path.profileDir, "places.sqlite");
+          let info = await OS.File.stat(placesDbPath);
+          return parseInt(info.size / BYTES_PER_MEBIBYTE);
         }
       },
 
       { histogram: "PLACES_DATABASE_PAGESIZE_B",
         query:     "PRAGMA page_size /* PlacesDBUtils.jsm PAGESIZE_B */" },
 
       { histogram: "PLACES_DATABASE_SIZE_PER_PAGE_B",
         query:     "PRAGMA page_count",
@@ -1001,16 +1041,24 @@ this.PlacesDBUtils = {
           // Note that the database file size would not be meaningful for this
           // calculation, because the file grows in fixed-size chunks.
           let dbPageSize = probeValues.PLACES_DATABASE_PAGESIZE_B;
           let placesPageCount = probeValues.PLACES_PAGES_COUNT;
           return Math.round((dbPageSize * aDbPageCount) / placesPageCount);
         }
       },
 
+      { histogram: "PLACES_DATABASE_FAVICONS_FILESIZE_MB",
+        async callback() {
+          let faviconsDbPath = OS.Path.join(OS.Constants.Path.profileDir, "favicons.sqlite");
+          let info = await OS.File.stat(faviconsDbPath);
+          return parseInt(info.size / BYTES_PER_MEBIBYTE);
+        }
+      },
+
       { histogram: "PLACES_ANNOS_BOOKMARKS_COUNT",
         query:     "SELECT count(*) FROM moz_items_annos" },
 
       { histogram: "PLACES_ANNOS_PAGES_COUNT",
         query:     "SELECT count(*) FROM moz_annos" },
 
       { histogram: "PLACES_MAINTENANCE_DAYSFROMLAST",
         callback() {
@@ -1020,53 +1068,29 @@ this.PlacesDBUtils = {
             return parseInt((nowSeconds - lastMaintenance) / 86400);
           } catch (ex) {
             return 60;
           }
         }
       },
     ];
 
-    let params = {
-      tags_folder: PlacesUtils.tagsFolderId,
-      type_folder: PlacesUtils.bookmarks.TYPE_FOLDER,
-      type_bookmark: PlacesUtils.bookmarks.TYPE_BOOKMARK,
-      places_root: PlacesUtils.placesRootId
-    };
-
-    for (let i = 0; i < probes.length; i++) {
-      let probe = probes[i];
-
-      let promiseDone = new Promise((resolve, reject) => {
-        if (!("query" in probe)) {
-          resolve([probe]);
-          return;
-        }
-
-        let filteredParams = {};
-        for (let p in params) {
-          if (probe.query.includes(`:${p}`)) {
-            filteredParams[p] = params[p];
-          }
-        }
-        PlacesUtils.promiseDBConnection()
-          .then(db => db.execute(probe.query, filteredParams))
-          .then(rows => resolve([probe, rows[0].getResultByIndex(0)]))
-          .catch(ex => reject(new Error("Unable to get telemetry from database.")));
-      });
+    for (let probe of probes) {
+      let val;
+      if (("query" in probe)) {
+        let db = await PlacesUtils.promiseDBConnection();
+        val = (await db.execute(probe.query, probe.params || {}))[0].getResultByIndex(0);
+      }
       // Report the result of the probe through Telemetry.
       // The resulting promise cannot reject.
-      promiseDone.then(([aProbe, aValue]) => {
-        let value = aValue;
-        if ("callback" in aProbe) {
-          value = aProbe.callback(value);
-        }
-        probeValues[aProbe.histogram] = value;
-        Services.telemetry.getHistogramById(aProbe.histogram).add(value);
-      }).catch(Cu.reportError);
+      if ("callback" in probe) {
+        val = await probe.callback(val);
+      }
+      probeValues[probe.histogram] = val;
+      Services.telemetry.getHistogramById(probe.histogram).add(val);
     }
   },
 
   /**
    * Runs a list of tasks, returning a Map when done.
    *
    * @param tasks
    *        Array of tasks to be executed, in form of pointers to methods in
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/favicons/test_incremental_vacuum.js
@@ -0,0 +1,38 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Tests incremental vacuum of the favicons database.
+
+Cu.import("resource://gre/modules/PlacesDBUtils.jsm");
+
+add_task(async function() {
+  let icon = {
+    file: do_get_file("noise.png"),
+    mimetype: "image/png"
+  };
+
+  let url = "http://foo.bar/";
+  await PlacesTestUtils.addVisits(url);
+  for (let i = 0; i < 10; ++i) {
+    let iconUri = NetUtil.newURI("http://mozilla.org/" + i);
+    let data = readFileData(icon.file);
+    PlacesUtils.favicons.replaceFaviconData(iconUri, data, data.length,
+                                            icon.mimetype);
+    await setFaviconForPage(url, iconUri);
+  }
+
+  let promise = TestUtils.topicObserved("places-favicons-expired");
+  PlacesUtils.favicons.expireAllFavicons();
+  await promise;
+
+  let db = await PlacesUtils.promiseDBConnection();
+  let state = (await db.execute("PRAGMA favicons.auto_vacuum"))[0].getResultByIndex(0);
+  Assert.equal(state, 2, "auto_vacuum should be incremental");
+  let count = (await db.execute("PRAGMA favicons.freelist_count"))[0].getResultByIndex(0);
+  info(`Found ${count} freelist pages`);
+  let log = await PlacesDBUtils.incrementalVacuum();
+  info(log);
+  let newCount = (await db.execute("PRAGMA favicons.freelist_count"))[0].getResultByIndex(0);
+  info(`Found ${newCount} freelist pages`);
+  Assert.ok(newCount < count, "The number of freelist pages should have reduced");
+});
--- a/toolkit/components/places/tests/favicons/xpcshell.ini
+++ b/toolkit/components/places/tests/favicons/xpcshell.ini
@@ -28,16 +28,17 @@ support-files =
 [test_expireAllFavicons.js]
 [test_expire_migrated_icons.js]
 [test_expire_on_new_icons.js]
 [test_favicons_conversions.js]
 [test_favicons_protocols_ref.js]
 [test_getFaviconDataForPage.js]
 [test_getFaviconURLForPage.js]
 [test_heavy_favicon.js]
+[test_incremental_vacuum.js]
 [test_moz-anno_favicon_mime_type.js]
 [test_multiple_frames.js]
 [test_page-icon_protocol.js]
 [test_query_result_favicon_changed_on_child.js]
 [test_replaceFaviconData.js]
 [test_replaceFaviconDataFromDataURL.js]
 [test_root_icons.js]
 [test_svg_favicon.js]
--- a/toolkit/components/places/tests/head_common.js
+++ b/toolkit/components/places/tests/head_common.js
@@ -1,21 +1,20 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // It is expected that the test files importing this file define Cu etc.
 /* global Cu, Ci, Cc, Cr */
 
-const CURRENT_SCHEMA_VERSION = 41;
+const CURRENT_SCHEMA_VERSION = 42;
 const FIRST_UPGRADABLE_SCHEMA_VERSION = 30;
 
 const NS_APP_USER_PROFILE_50_DIR = "ProfD";
-const NS_APP_PROFILE_DIR_STARTUP = "ProfDS";
 
 // Shortcuts to transitions type.
 const TRANSITION_LINK = Ci.nsINavHistoryService.TRANSITION_LINK;
 const TRANSITION_TYPED = Ci.nsINavHistoryService.TRANSITION_TYPED;
 const TRANSITION_BOOKMARK = Ci.nsINavHistoryService.TRANSITION_BOOKMARK;
 const TRANSITION_EMBED = Ci.nsINavHistoryService.TRANSITION_EMBED;
 const TRANSITION_FRAMED_LINK = Ci.nsINavHistoryService.TRANSITION_FRAMED_LINK;
 const TRANSITION_REDIRECT_PERMANENT = Ci.nsINavHistoryService.TRANSITION_REDIRECT_PERMANENT;
@@ -38,19 +37,18 @@ XPCOMUtils.defineLazyModuleGetters(this,
   PlacesBackups: "resource://gre/modules/PlacesBackups.jsm",
   PlacesSyncUtils: "resource://gre/modules/PlacesSyncUtils.jsm",
   PlacesTestUtils: "resource://testing-common/PlacesTestUtils.jsm",
   PlacesTransactions: "resource://gre/modules/PlacesTransactions.jsm",
   OS: "resource://gre/modules/osfile.jsm",
   Sqlite: "resource://gre/modules/Sqlite.jsm",
   TestUtils: "resource://testing-common/TestUtils.jsm",
   AppConstants: "resource://gre/modules/AppConstants.jsm",
+  PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
 });
-// This imports various other objects in addition to PlacesUtils.
-Cu.import("resource://gre/modules/PlacesUtils.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "SMALLPNG_DATA_URI", function() {
   return NetUtil.newURI(
          "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAA" +
          "AAAA6fptVAAAACklEQVQI12NgAAAAAgAB4iG8MwAAAABJRU5ErkJggg==");
 });
 XPCOMUtils.defineLazyGetter(this, "SMALLSVG_DATA_URI", function() {
   return NetUtil.newURI(
new file mode 100644
index 0000000000000000000000000000000000000000..a59d9d286f32eee5600d781f74bd74d39f04e73f
GIT binary patch
literal 229376
zc%1FjT~C`;902gAz^EbP!mGx3!SP}VO*&b}Hs`J^@GzTI&{DEZFPcKT(F7=KyFhS`
zGW`O67ZWf21b!1gge|XyR=hE3Vt&7*C+GeA`#&eo+INL|yB1zGnmd(tn2$Qq*jV&c
z2vL;yJ&K}abm^n+Z=$cAi2hY~kIxg|Pj_pRKmHcI|LcAJr`zxTc(>SX0000000000
z0000000000000000000000000006*$<oNbvGL@f89N(?)R%-`4jl=R@<#nx9-Y?fT
z8@sLY+vfIGrL{GDG`U!ebEP=sSC--z;pW9*b!BiynC&%t^UA%6>BC#eRBk45{Mp5M
zryKRk>yey{Ob$dae8KtYXQ#fZAJnQRjmxv2O-!cl+)4b<u54`AT5q=N?OM6juC%lL
z^;>7>m2wM(IP{Oqwl}sj_3A;U*GhkQGM-Fjvx#43PPV%Ytlc=xz+lhBm8}QI!)!Q7
zR=wK&x25=LTnsC#rLeMIC}hIPUl}^`Z885%uJ|%Mi(j5)yI#GRpU#Bk)nc4~x^iVa
z9g6XCT#Q#1<Fz4|*3IQoT!_1|F6P!2b4&5HUCuYZzRN$X4(aebU;27=y%dV8&+|*+
zIE*DzbKOVTxJcfW00uh72U2(Km4&|4^d%${&Yxc>#V<;IS-bKq2eKN{A3jbbQ!_J(
zpWj@)-;2uF@Y}p0l=IhkJ(MADb;vdE*Q@OzfiA_%x%EOR+{=V!qtU)`!e(Q;(LA4#
z3AKa0db3vEY2CP}T4`6pLSc0wq^Elyl+K7hXaE2J0000000000000000000000000
z0000000000Fk;iaqBG(k000000000000000000000000000000000000001B#HM;h
zXT(7O00000000000000000000000000000000000006*<P4|jU6dnC}^y|^ah=TzD
z000000000000000000000000000000000000N{V@?)bz^?vtIyVY$B9*lk@@PRh5<
z?X5~{tG-*U9X$N<@x5^W$=JkP{?p#9y~^uatGs`?>`F(uf9c}~+3bACW)l<H?9Al}
z?Z)Yv!JbDC9(^$%?uYDDFT~D>Qv?7200000000000000000000000000000000000
u03$ZlD>@?%0ssI200000000000000000000000000000000000M(l5G^R{mQ
--- a/toolkit/components/places/tests/migration/head_migration.js
+++ b/toolkit/components/places/tests/migration/head_migration.js
@@ -23,22 +23,22 @@ const DB_FILENAME = "places.sqlite";
  * Sets the database to use for the given test.  This should be the very first
  * thing in the test, otherwise this database will not be used!
  *
  * @param aFileName
  *        The filename of the database to use.  This database must exist in
  *        toolkit/components/places/tests/migration!
  * @return {Promise}
  */
-var setupPlacesDatabase = async function(aFileName) {
+var setupPlacesDatabase = async function(aFileName, aDestFileName = DB_FILENAME) {
   let currentDir = await OS.File.getCurrentDirectory();
 
   let src = OS.Path.join(currentDir, aFileName);
   Assert.ok((await OS.File.exists(src)), "Database file found");
 
   // Ensure that our database doesn't already exist.
-  let dest = OS.Path.join(OS.Constants.Path.profileDir, DB_FILENAME);
+  let dest = OS.Path.join(OS.Constants.Path.profileDir, aDestFileName);
   Assert.ok(!(await OS.File.exists(dest)), "Database file should not exist yet");
 
   await OS.File.copy(src, dest);
 };
 
 // This works provided all tests in this folder use add_task.
--- a/toolkit/components/places/tests/migration/test_current_from_v38.js
+++ b/toolkit/components/places/tests/migration/test_current_from_v38.js
@@ -1,18 +1,18 @@
-add_task(function* setup() {
-  yield setupPlacesDatabase("places_v38.sqlite");
+add_task(async function setup() {
+  await setupPlacesDatabase("places_v38.sqlite");
 });
 
-add_task(function* database_is_valid() {
+add_task(async function database_is_valid() {
   // Accessing the database for the first time triggers migration.
   Assert.equal(PlacesUtils.history.databaseStatus,
                PlacesUtils.history.DATABASE_STATUS_UPGRADED);
 
-  let db = yield PlacesUtils.promiseDBConnection();
-  Assert.equal((yield db.getSchemaVersion()), CURRENT_SCHEMA_VERSION);
+  let db = await PlacesUtils.promiseDBConnection();
+  Assert.equal((await db.getSchemaVersion()), CURRENT_SCHEMA_VERSION);
 });
 
-add_task(function* test_select_new_fields() {
-  let db = yield PlacesUtils.promiseDBConnection();
-  yield db.execute(`SELECT description, preview_image_url FROM moz_places`);
+add_task(async function test_select_new_fields() {
+  let db = await PlacesUtils.promiseDBConnection();
+  await db.execute(`SELECT description, preview_image_url FROM moz_places`);
   Assert.ok(true, "should be able to select description and preview_image_url");
 });
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/migration/test_current_from_v41.js
@@ -0,0 +1,17 @@
+add_task(async function setup() {
+  // Since this migration doesn't affect places.sqlite, we can reuse v38.
+  await setupPlacesDatabase("places_v38.sqlite");
+  await setupPlacesDatabase("favicons_v41.sqlite", "favicons.sqlite");
+});
+
+add_task(async function database_is_valid() {
+  // Accessing the database for the first time triggers migration.
+  Assert.equal(PlacesUtils.history.databaseStatus,
+               PlacesUtils.history.DATABASE_STATUS_UPGRADED);
+
+  let db = await PlacesUtils.promiseDBConnection();
+  Assert.equal((await db.getSchemaVersion()), CURRENT_SCHEMA_VERSION);
+
+  let vacuum = (await db.execute(`PRAGMA favicons.auto_vacuum`))[0].getResultByIndex(0);
+  Assert.equal(vacuum, 2, "Incremental vacuum is enabled");
+});
--- a/toolkit/components/places/tests/migration/xpcshell.ini
+++ b/toolkit/components/places/tests/migration/xpcshell.ini
@@ -1,19 +1,21 @@
 [DEFAULT]
 head = head_migration.js
 
 support-files =
+  favicons_v41.sqlite
   places_outdated.sqlite
   places_v31.sqlite
   places_v34.sqlite
   places_v35.sqlite
   places_v36.sqlite
   places_v38.sqlite
 
 [test_current_from_downgraded.js]
 [test_current_from_outdated.js]
 [test_current_from_v31.js]
 [test_current_from_v34.js]
 [test_current_from_v34_no_roots.js]
 [test_current_from_v35.js]
 [test_current_from_v36.js]
 [test_current_from_v38.js]
+[test_current_from_v41.js]
--- a/toolkit/components/places/tests/unit/test_telemetry.js
+++ b/toolkit/components/places/tests/unit/test_telemetry.js
@@ -8,16 +8,17 @@ Components.utils.import("resource://gre/
 var histograms = {
   PLACES_PAGES_COUNT: val => Assert.equal(val, 1),
   PLACES_BOOKMARKS_COUNT: val => Assert.equal(val, 1),
   PLACES_TAGS_COUNT: val => Assert.equal(val, 1),
   PLACES_KEYWORDS_COUNT: val => Assert.equal(val, 1),
   PLACES_SORTED_BOOKMARKS_PERC: val => Assert.equal(val, 100),
   PLACES_TAGGED_BOOKMARKS_PERC: val => Assert.equal(val, 100),
   PLACES_DATABASE_FILESIZE_MB: val => Assert.ok(val > 0),
+  PLACES_DATABASE_FAVICONS_FILESIZE_MB: val => Assert.ok(val > 0),
   PLACES_DATABASE_PAGESIZE_B: val => Assert.equal(val, 32768),
   PLACES_DATABASE_SIZE_PER_PAGE_B: val => Assert.ok(val > 0),
   PLACES_EXPIRATION_STEPS_TO_CLEAN2: val => Assert.ok(val > 1),
   // PLACES_AUTOCOMPLETE_1ST_RESULT_TIME_MS:  val => do_check_true(val > 1),
   PLACES_IDLE_FRECENCY_DECAY_TIME_MS: val => Assert.ok(val >= 0),
   PLACES_IDLE_MAINTENANCE_TIME_MS: val => Assert.ok(val > 0),
   // One from the `setItemAnnotation` call; the other from the mobile root.
   // This can be removed along with the anno in bug 1306445.
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -5388,16 +5388,27 @@
     "record_in_processes": ["main", "content"],
     "expires_in_version": "never",
     "kind": "exponential",
     "low": 500,
     "high": 10240,
     "n_buckets": 20,
     "description": "PLACES: Average size of a place in the database (bytes)"
   },
+  "PLACES_DATABASE_FAVICONS_FILESIZE_MB": {
+    "record_in_processes": ["main"],
+    "alert_emails": ["mak@mozilla.com", "fx-search@mozilla.com"],
+    "bug_numbers": [1346554],
+    "expires_in_version": "never",
+    "kind": "exponential",
+    "low": 5,
+    "high": 100,
+    "n_buckets": 10,
+    "description": "PLACES: Favicons database filesize (MB)"
+  },
   "PLACES_EXPIRATION_STEPS_TO_CLEAN2": {
     "record_in_processes": ["main", "content"],
     "expires_in_version": "never",
     "kind": "enumerated",
     "n_values": 10,
     "description": "PLACES: Expiration steps to cleanup the database"
   },
   "PLACES_AUTOCOMPLETE_1ST_RESULT_TIME_MS": {