Bug 1168152 P3 Perform incremental vacuum at tail end of Cache db connections. r=ehsan
authorBen Kelly <ben@wanderview.com>
Thu, 28 May 2015 07:46:47 -0700
changeset 246161 30d26d24914b94ed034906abeae6724b4618b64d
parent 246160 f048ced0745e769a80c8fcc122df6ff6c927f7f6
child 246162 20458c713464eb483e4a1333b8e8e04ba9de538a
push id28823
push userryanvm@gmail.com
push dateFri, 29 May 2015 13:33:16 +0000
treeherdermozilla-central@9738f055d98c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1168152
milestone41.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 1168152 P3 Perform incremental vacuum at tail end of Cache db connections. r=ehsan
dom/cache/Connection.cpp
dom/cache/Connection.h
dom/cache/DBSchema.cpp
dom/cache/DBSchema.h
--- a/dom/cache/Connection.cpp
+++ b/dom/cache/Connection.cpp
@@ -1,31 +1,54 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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/. */
 
 #include "mozilla/dom/cache/Connection.h"
 
+#include "mozilla/dom/cache/DBSchema.h"
+#include "mozStorageHelper.h"
+
 namespace mozilla {
 namespace dom {
 namespace cache {
 
 NS_IMPL_ISUPPORTS(cache::Connection, mozIStorageAsyncConnection,
                                      mozIStorageConnection);
 
 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
 // base connection or block the method.  All the async execution methods
 // are blocked because Cache does not use them and they would require more
 // work to wrap properly.
 
 // mozIStorageAsyncConnection methods
@@ -111,22 +134,16 @@ NS_IMETHODIMP
 Connection::RemoveProgressHandler(mozIStorageProgressHandler** aHandlerOut)
 {
   return mBase->RemoveProgressHandler(aHandlerOut);
 }
 
 // mozIStorageConnection methods
 
 NS_IMETHODIMP
-Connection::Close()
-{
-  return mBase->Close();
-}
-
-NS_IMETHODIMP
 Connection::Clone(bool aReadOnly, mozIStorageConnection** aConnectionOut)
 {
   nsCOMPtr<mozIStorageConnection> conn;
   nsresult rv = mBase->Clone(aReadOnly, getter_AddRefs(conn));
   if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
   nsCOMPtr<mozIStorageConnection> wrapped = new Connection(conn);
   wrapped.forget(aConnectionOut);
--- a/dom/cache/Connection.h
+++ b/dom/cache/Connection.h
@@ -17,16 +17,17 @@ class Connection final : public mozIStor
 {
 public:
   explicit Connection(mozIStorageConnection* aBase);
 
 private:
   ~Connection();
 
   nsCOMPtr<mozIStorageConnection> mBase;
+  bool mClosed;
 
   NS_DECL_ISUPPORTS
   NS_DECL_MOZISTORAGEASYNCCONNECTION
   NS_DECL_MOZISTORAGECONNECTION
 };
 
 } // namespace cache
 } // namespace dom
--- a/dom/cache/DBSchema.cpp
+++ b/dom/cache/DBSchema.cpp
@@ -39,16 +39,19 @@ const int32_t kMaxEntriesPerStatement = 
 const uint32_t kPageSize = 4 * 1024;
 
 // Grow the database in chunks to reduce fragmentation
 const uint32_t kGrowthSize = 64 * 1024;
 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;
 static_assert(kWalAutoCheckpointSize % kPageSize == 0,
               "WAL checkpoint size must be multiple of page size");
 
 } // anonymous namespace
 
@@ -222,38 +225,22 @@ static nsresult CreateAndBindKeyStatemen
 } // anonymous namespace
 
 nsresult
 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;
   }
 
   if (!schemaVersion) {
     // The caches table is the single source of truth about what Cache
     // objects exist for the origin.  The contents of the Cache are stored
     // in the entries table that references back to caches.
     //
@@ -392,26 +379,20 @@ InitializeConnection(mozIStorageConnecti
   MOZ_ASSERT(aConn);
 
   // This function needs to perform per-connection initialization tasks that
   // need to happen regardless of the schema.
 
   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
   // the work necessary to convert UTF-16 nsString values for us.  We don't
   // need ordering and the binary equality operations are correct.  So, do
   // NOT set PRAGMA encoding to UTF-16.
 
   nsresult rv = aConn->ExecuteSimpleSQL(pragmas);
@@ -420,16 +401,54 @@ InitializeConnection(mozIStorageConnecti
   // Limit fragmentation by growing the database by many pages at once.
   rv = aConn->SetGrowthIncrement(kGrowthSize, EmptyCString());
   if (rv == NS_ERROR_FILE_TOO_BIG) {
     NS_WARNING("Not enough disk space to set sqlite growth increment.");
     rv = NS_OK;
   }
   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;
 }
 
 nsresult
 CreateCacheId(mozIStorageConnection* aConn, CacheId* aCacheIdOut)
 {
   MOZ_ASSERT(!NS_IsMainThread());
   MOZ_ASSERT(aConn);
@@ -1938,12 +1957,76 @@ CreateAndBindKeyStatement(mozIStorageCon
 
   state.forget(aStateOut);
 
   return rv;
 }
 
 } // 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
 } // namespace mozilla
--- a/dom/cache/DBSchema.h
+++ b/dom/cache/DBSchema.h
@@ -27,16 +27,17 @@ class CacheResponse;
 struct SavedRequest;
 struct SavedResponse;
 
 namespace db {
 
 nsresult
 CreateSchema(mozIStorageConnection* aConn);
 
+// Note, this cannot be executed within a transaction.
 nsresult
 InitializeConnection(mozIStorageConnection* aConn);
 
 nsresult
 CreateCacheId(mozIStorageConnection* aConn, CacheId* aCacheIdOut);
 
 nsresult
 DeleteCacheId(mozIStorageConnection* aConn, CacheId aCacheId,
@@ -99,16 +100,20 @@ StoragePutCache(mozIStorageConnection* a
 nsresult
 StorageForgetCache(mozIStorageConnection* aConn, Namespace aNamespace,
                    const nsAString& aKey);
 
 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;
 
 } // namespace db
 } // namespace cache
 } // namespace dom
 } // namespace mozilla