Bug 1168152 P3 Perform incremental vacuum at tail end of Cache db connections. r=ehsan

dom/cache/Connection.cpp

@@ -6,6 +6,9 @@
 
 #include "mozilla/dom/cache/Connection.h"
 
+#include "mozilla/dom/cache/DBSchema.h"
+#include "mozStorageHelper.h"
+
 namespace mozilla {
 namespace dom {
 namespace cache {
@@ -15,12 +18,32 @@ NS_IMPL_ISUPPORTS(cache::Connection, mozIStorageAsyncConnection,
 
 Connection::Connection(mozIStorageConnection* aBase)
   : mBase(aBase)
+  , mClosed(false)
 {
   MOZ_ASSERT(mBase);
 }
 
 Connection::~Connection()
 {
+  NS_ASSERT_OWNINGTHREAD(Connection);
+  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(Close()));
+}
+
+NS_IMETHODIMP
+Connection::Close()
+{
+  NS_ASSERT_OWNINGTHREAD(Connection);
+
+  if (mClosed) {
+    return NS_OK;
+  }
+  mClosed = true;
+
+  // If we are closing here, then Cache must not have a transaction
+  // open anywhere else.  This should be guaranteed to succeed.
+  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(db::IncrementalVacuum(this)));
+
+  return mBase->Close();
 }
 
 // The following methods are all boilerplate that either forward to the
@@ -115,12 +138,6 @@ Connection::RemoveProgressHandler(mozIStorageProgressHandler** aHandlerOut)
 
 // mozIStorageConnection methods
 
-NS_IMETHODIMP
-Connection::Close()
-{
-  return mBase->Close();
-}
-
 NS_IMETHODIMP
 Connection::Clone(bool aReadOnly, mozIStorageConnection** aConnectionOut)
 {

dom/cache/Connection.h

@@ -22,6 +22,7 @@ private:
   ~Connection();
 
   nsCOMPtr<mozIStorageConnection> mBase;
+  bool mClosed;
 
   NS_DECL_ISUPPORTS
   NS_DECL_MOZISTORAGEASYNCCONNECTION

dom/cache/DBSchema.cpp

@@ -44,6 +44,9 @@ const uint32_t kGrowthPages = kGrowthSize / kPageSize;
 static_assert(kGrowthSize % kPageSize == 0,
               "Growth size must be multiple of page size");
 
+// Only release free pages when we have more than this limit
+const int32_t kMaxFreePages = kGrowthPages;
+
 // Limit WAL journal to a reasonable size
 const uint32_t kWalAutoCheckpointSize = 512 * 1024;
 const uint32_t kWalAutoCheckpointPages = kWalAutoCheckpointSize / kPageSize;
@@ -227,28 +230,12 @@ CreateSchema(mozIStorageConnection* aConn)
   MOZ_ASSERT(!NS_IsMainThread());
   MOZ_ASSERT(aConn);
 
-  nsAutoCString pragmas(
-    // Enable auto-vaccum but in incremental mode in order to avoid doing a lot
-    // of work at the end of each transaction.
-    // NOTE: This must be done here instead of InitializeConnection() because it
-    //       only works when the database is empty.
-    "PRAGMA auto_vacuum = INCREMENTAL; "
-  );
-
-  nsresult rv = aConn->ExecuteSimpleSQL(pragmas);
-  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
-
   int32_t schemaVersion;
-  rv = aConn->GetSchemaVersion(&schemaVersion);
+  nsresult rv = aConn->GetSchemaVersion(&schemaVersion);
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
   if (schemaVersion == kLatestSchemaVersion) {
-    // We already have the correct schema, so just do an incremental vaccum and
-    // get started.
-    rv = aConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
-      "PRAGMA incremental_vacuum;"));
-    if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
-
+    // We already have the correct schema, so just get started.
     return rv;
   }
 
@@ -397,16 +384,10 @@ InitializeConnection(mozIStorageConnection* aConn)
   nsPrintfCString pragmas(
     // Use a smaller page size to improve perf/footprint; default is too large
     "PRAGMA page_size = %u; "
-    // WAL journal can grow to given number of *pages*
-    "PRAGMA wal_autocheckpoint = %u; "
-    // Always truncate the journal back to given number of *bytes*
-    "PRAGMA journal_size_limit = %u; "
-    // WAL must be enabled at the end to allow page size to be changed, etc.
-    "PRAGMA journal_mode = WAL; "
+    // Enable auto_vacuum; this must happen after page_size and before WAL
+    "PRAGMA auto_vacuum = INCREMENTAL; "
     "PRAGMA foreign_keys = ON; ",
-    kPageSize,
-    kWalAutoCheckpointPages,
-    kWalAutoCheckpointSize
+    kPageSize
   );
 
   // Note, the default encoding of UTF-8 is preferred.  mozStorage does all
@@ -425,6 +406,44 @@ InitializeConnection(mozIStorageConnection* aConn)
   }
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
+  // Enable WAL journaling.  This must be performed in a separate transaction
+  // after changing the page_size and enabling auto_vacuum.
+  nsPrintfCString wal(
+    // WAL journal can grow to given number of *pages*
+    "PRAGMA wal_autocheckpoint = %u; "
+    // Always truncate the journal back to given number of *bytes*
+    "PRAGMA journal_size_limit = %u; "
+    // WAL must be enabled at the end to allow page size to be changed, etc.
+    "PRAGMA journal_mode = WAL; ",
+    kWalAutoCheckpointPages,
+    kWalAutoCheckpointSize
+  );
+
+  rv = aConn->ExecuteSimpleSQL(wal);
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  // Verify that we successfully set the vacuum mode to incremental.  It
+  // is very easy to put the database in a state where the auto_vacuum
+  // pragma above fails silently.
+#ifdef DEBUG
+  nsCOMPtr<mozIStorageStatement> state;
+  rv = aConn->CreateStatement(NS_LITERAL_CSTRING(
+    "PRAGMA auto_vacuum;"
+  ), getter_AddRefs(state));
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  bool hasMoreData = false;
+  rv = state->ExecuteStep(&hasMoreData);
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  int32_t mode;
+  rv = state->GetInt32(0, &mode);
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  // integer value 2 is incremental mode
+  if (NS_WARN_IF(mode != 2)) { return NS_ERROR_UNEXPECTED; }
+#endif
+
   return NS_OK;
 }
 
@@ -1943,6 +1962,70 @@ CreateAndBindKeyStatement(mozIStorageConnection* aConn,
 
 } // anonymouns namespace
 
+nsresult
+IncrementalVacuum(mozIStorageConnection* aConn)
+{
+  // Determine how much free space is in the database.
+  nsCOMPtr<mozIStorageStatement> state;
+  nsresult rv = aConn->CreateStatement(NS_LITERAL_CSTRING(
+    "PRAGMA freelist_count;"
+  ), getter_AddRefs(state));
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  bool hasMoreData = false;
+  rv = state->ExecuteStep(&hasMoreData);
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  int32_t freePages = 0;
+  rv = state->GetInt32(0, &freePages);
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  // We have a relatively small page size, so we want to be careful to avoid
+  // fragmentation.  We already use a growth incremental which will cause
+  // sqlite to allocate and release multiple pages at the same time.  We can
+  // further reduce fragmentation by making our allocated chunks a bit
+  // "sticky".  This is done by creating some hysteresis where we allocate
+  // pages/chunks as soon as we need them, but we only release pages/chunks
+  // when we have a large amount of free space.  This helps with the case
+  // where a page is adding and remove resources causing it to dip back and
+  // forth across a chunk boundary.
+  //
+  // So only proceed with releasing pages if we have more than our constant
+  // threshold.
+  if (freePages <= kMaxFreePages) {
+    return NS_OK;
+  }
+
+  // Release the excess pages back to the sqlite VFS.  This may also release
+  // chunks of multiple pages back to the OS.
+  int32_t pagesToRelease = freePages - kMaxFreePages;
+
+  rv = aConn->ExecuteSimpleSQL(nsPrintfCString(
+    "PRAGMA incremental_vacuum(%d);", pagesToRelease
+  ));
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  // Verify that our incremental vacuum actually did something
+#ifdef DEBUG
+  rv = aConn->CreateStatement(NS_LITERAL_CSTRING(
+    "PRAGMA freelist_count;"
+  ), getter_AddRefs(state));
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  hasMoreData = false;
+  rv = state->ExecuteStep(&hasMoreData);
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  freePages = 0;
+  rv = state->GetInt32(0, &freePages);
+  if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
+
+  MOZ_ASSERT(freePages <= kMaxFreePages);
+#endif
+
+  return NS_OK;
+}
+
 } // namespace db
 } // namespace cache
 } // namespace dom

dom/cache/DBSchema.h

@@ -32,6 +32,7 @@ namespace db {
 nsresult
 CreateSchema(mozIStorageConnection* aConn);
 
+// Note, this cannot be executed within a transaction.
 nsresult
 InitializeConnection(mozIStorageConnection* aConn);
 
@@ -104,6 +105,10 @@ nsresult
 StorageGetKeys(mozIStorageConnection* aConn, Namespace aNamespace,
                nsTArray<nsString>& aKeysOut);
 
+// Note, this works best when its NOT executed within a transaction.
+nsresult
+IncrementalVacuum(mozIStorageConnection* aConn);
+
 // We will wipe out databases with a schema versions less than this.
 extern const int32_t kMaxWipeSchemaVersion;