Bug 703143 - Use a memory multi-reporter for SQLite's per-connection reporting. r=sdwilsh.
authorNicholas Nethercote <nnethercote@mozilla.com>
Mon, 05 Dec 2011 19:19:47 -0800
changeset 83127 1f093067e982dff68a1b6ef8d2617e965f84d493
parent 83126 bfb517a374fc041f8f599910d58317bd79403e0e
child 83128 c9eee0711ca6c1d8b9a152af2f9372fef4cd5f31
push id519
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 00:38:35 +0000
treeherdermozilla-beta@788ea1ef610b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssdwilsh
bugs703143
milestone11.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 703143 - Use a memory multi-reporter for SQLite's per-connection reporting. r=sdwilsh.
storage/src/mozStorageConnection.cpp
storage/src/mozStorageConnection.h
storage/src/mozStorageService.cpp
storage/src/mozStorageService.h
toolkit/components/aboutmemory/tests/chrome/Makefile.in
toolkit/components/aboutmemory/tests/chrome/test_asyncClose_leak.xul
toolkit/components/telemetry/TelemetryPing.js
--- a/storage/src/mozStorageConnection.cpp
+++ b/storage/src/mozStorageConnection.cpp
@@ -468,27 +468,16 @@ Connection::Connection(Service *aService
 {
   mFunctions.Init();
   mStorageService->registerConnection(this);
 }
 
 Connection::~Connection()
 {
   (void)Close();
-
-  // The memory reporters should have been already unregistered if the APIs
-  // have been used properly.  But if an async connection hasn't been closed
-  // with asyncClose(), the connection is about to leak and it's too late to do
-  // anything about it.  So we mark the memory reporters accordingly so that
-  // the leak will be obvious in about:memory.
-  for (PRUint32 i = 0; i < mMemoryReporters.Length(); i++) {
-    if (mMemoryReporters[i]) {
-      mMemoryReporters[i]->markAsLeaked();
-    }
-  }
 }
 
 NS_IMPL_THREADSAFE_ADDREF(Connection)
 NS_IMPL_THREADSAFE_QUERY_INTERFACE2(
   Connection,
   mozIStorageConnection,
   nsIInterfaceRequestor
 )
@@ -640,38 +629,16 @@ Connection::initialize(nsIFile *aDatabas
       break;
     case 1:
     default:
       (void)ExecuteSimpleSQL(NS_LITERAL_CSTRING(
           "PRAGMA synchronous = NORMAL;"));
       break;
   }
 
-  nsRefPtr<StorageMemoryReporter> reporter;
-  nsCString filename = this->getFilename();
-
-  reporter =
-    new StorageMemoryReporter(this->mDBConn, filename,
-                              StorageMemoryReporter::Cache_Used);
-  mMemoryReporters.AppendElement(reporter);
-
-  reporter =
-    new StorageMemoryReporter(this->mDBConn, filename,
-                              StorageMemoryReporter::Schema_Used);
-  mMemoryReporters.AppendElement(reporter);
-
-  reporter =
-    new StorageMemoryReporter(this->mDBConn, filename,
-                              StorageMemoryReporter::Stmt_Used);
-  mMemoryReporters.AppendElement(reporter);
-
-  for (PRUint32 i = 0; i < mMemoryReporters.Length(); i++) {
-    (void)::NS_RegisterMemoryReporter(mMemoryReporters[i]);
-  }
-
   return NS_OK;
 }
 
 nsresult
 Connection::databaseElementExists(enum DatabaseElementType aElementType,
                                   const nsACString &aElementName,
                                   bool *_exists)
 {
@@ -798,21 +765,16 @@ Connection::internalClose()
   while ((stmt = ::sqlite3_next_stmt(mDBConn, stmt))) {
     char *msg = ::PR_smprintf("SQL statement '%s' was not finalized",
                               ::sqlite3_sql(stmt));
     NS_WARNING(msg);
     ::PR_smprintf_free(msg);
   }
 #endif
 
-  for (PRUint32 i = 0; i < mMemoryReporters.Length(); i++) {
-    (void)::NS_UnregisterMemoryReporter(mMemoryReporters[i]);
-    mMemoryReporters[i] = nsnull;
-  }
-
   int srv = ::sqlite3_close(mDBConn);
   NS_ASSERTION(srv == SQLITE_OK,
                "sqlite3_close failed. There are probably outstanding statements that are listed above!");
 
   mDBConn = NULL;
   return convertResultCode(srv);
 }
 
@@ -847,18 +809,19 @@ Connection::GetInterface(const nsIID &aI
 
 NS_IMETHODIMP
 Connection::Close()
 {
   if (!mDBConn)
     return NS_ERROR_NOT_INITIALIZED;
 
   { // Make sure we have not executed any asynchronous statements.
-    // If this fails, the connection will be left open!  See ~Connection() for
-    // more details.
+    // If this fails, the mDBConn will be left open, resulting in a leak.
+    // Ideally we'd schedule some code to destroy the mDBConn once all its
+    // async statements have finished executing;  see bug 704030.
     MutexAutoLock lockedScope(sharedAsyncExecutionMutex);
     bool asyncCloseWasCalled = !mAsyncExecutionThread;
     NS_ENSURE_TRUE(asyncCloseWasCalled, NS_ERROR_UNEXPECTED);
   }
 
   nsresult rv = setClosedState();
   NS_ENSURE_SUCCESS(rv, rv);
 
--- a/storage/src/mozStorageConnection.h
+++ b/storage/src/mozStorageConnection.h
@@ -55,23 +55,20 @@
 #include "nsIMutableArray.h"
 
 #include "sqlite3.h"
 
 struct PRLock;
 class nsIFile;
 class nsIEventTarget;
 class nsIThread;
-class nsIMemoryReporter;
 
 namespace mozilla {
 namespace storage {
 
-class StorageMemoryReporter;
-
 class Connection : public mozIStorageConnection
                  , public nsIInterfaceRequestor
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_MOZISTORAGECONNECTION
   NS_DECL_NSIINTERFACEREQUESTOR
 
@@ -199,18 +196,16 @@ private:
   // Generic progress handler
   // Dispatch call to registered progress handler,
   // if there is one. Do nothing in other cases.
   int progressHandler();
 
   sqlite3 *mDBConn;
   nsCOMPtr<nsIFile> mDatabaseFile;
 
-  nsTArray<nsRefPtr<StorageMemoryReporter> > mMemoryReporters;
-
   /**
    * Lazily created thread for asynchronous statement execution.  Consumers
    * should use getAsyncExecutionTarget rather than directly accessing this
    * field.
    */
   nsCOMPtr<nsIThread> mAsyncExecutionThread;
   /**
    * Set to true by Close() prior to actually shutting down the thread.  This
--- a/storage/src/mozStorageService.cpp
+++ b/storage/src/mozStorageService.cpp
@@ -133,33 +133,167 @@ namespace storage {
 //// Memory Reporting
 
 static PRInt64
 GetStorageSQLiteMemoryUsed()
 {
   return ::sqlite3_memory_used();
 }
 
-NS_MEMORY_REPORTER_IMPLEMENT(StorageSQLiteMemoryUsed,
-    "explicit/storage/sqlite",
-    KIND_HEAP,
+// We don't need an "explicit" reporter for total SQLite memory usage, because
+// the multi-reporter provides reports that add up to the total.  But it's
+// useful to have the total in the "Other Measurements" list in about:memory,
+// and more importantly, we also gather the total via telemetry.
+NS_MEMORY_REPORTER_IMPLEMENT(StorageSQLite,
+    "storage-sqlite",
+    KIND_OTHER,
     UNITS_BYTES,
     GetStorageSQLiteMemoryUsed,
     "Memory used by SQLite.")
 
+class StorageSQLiteMultiReporter : public nsIMemoryMultiReporter
+{
+private:
+  Service *mService;    // a weakref because Service contains a strongref to this
+  nsCString mStmtDesc;
+  nsCString mCacheDesc;
+  nsCString mSchemaDesc;
+
+public:
+  NS_DECL_ISUPPORTS
+
+  StorageSQLiteMultiReporter(Service *aService) 
+  : mService(aService)
+  {
+    NS_NAMED_LITERAL_CSTRING(mStmtDesc,
+      "Memory (approximate) used by all prepared statements used by "
+      "connections to this database.");
+
+    NS_NAMED_LITERAL_CSTRING(mCacheDesc,
+      "Memory (approximate) used by all pager caches used by connections "
+      "to this database.");
+
+    NS_NAMED_LITERAL_CSTRING(mSchemaDesc,
+      "Memory (approximate) used to store the schema for all databases "
+      "associated with connections to this database.");
+  }
+
+  // Warning: To get a Connection's measurements requires holding its lock.
+  // There may be a delay getting the lock if another thread is accessing the
+  // Connection.  This isn't very nice if CollectReports is called from the
+  // main thread!  But at the time of writing this function is only called when
+  // about:memory is loaded (not, for example, when telemetry pings occur) and
+  // any delays in that case aren't so bad.
+  NS_IMETHOD CollectReports(nsIMemoryMultiReporterCallback *aCallback,
+                            nsISupports *aClosure)
+  {
+    size_t totalConnSize = 0;
+    {
+      nsTArray<nsRefPtr<Connection> > connections;
+      mService->getConnections(connections);
+
+      for (PRUint32 i = 0; i < connections.Length(); i++) {
+        nsRefPtr<Connection> &conn = connections[i];
+
+        nsCString pathHead("explicit/storage/sqlite/");
+        pathHead.Append(conn->getFilename());
+        pathHead.AppendLiteral("/");
+
+        SQLiteMutexAutoLock lockedScope(conn->sharedDBMutex);
+
+        totalConnSize +=
+          doConnMeasurement(aCallback, aClosure, *conn.get(), pathHead,
+                            NS_LITERAL_CSTRING("stmt"), mStmtDesc,
+                            SQLITE_DBSTATUS_STMT_USED);
+        totalConnSize +=
+          doConnMeasurement(aCallback, aClosure, *conn.get(), pathHead,
+                            NS_LITERAL_CSTRING("cache"), mCacheDesc,
+                            SQLITE_DBSTATUS_CACHE_USED);
+        totalConnSize +=
+          doConnMeasurement(aCallback, aClosure, *conn.get(), pathHead,
+                            NS_LITERAL_CSTRING("schema"), mSchemaDesc,
+                            SQLITE_DBSTATUS_SCHEMA_USED);
+      }
+    }
+
+    PRInt64 other = ::sqlite3_memory_used() - totalConnSize;
+
+    aCallback->Callback(NS_LITERAL_CSTRING(""),
+                        NS_LITERAL_CSTRING("explicit/storage/sqlite/other"),
+                        nsIMemoryReporter::KIND_HEAP,
+                        nsIMemoryReporter::UNITS_BYTES, other,
+                        NS_LITERAL_CSTRING("All unclassified sqlite memory."),
+                        aClosure);
+
+    return NS_OK;
+  }
+
+private:
+  /**
+   * Passes a single SQLite memory statistic to a memory multi-reporter
+   * callback.
+   *
+   * @param aCallback
+   *        The callback.
+   * @param aClosure
+   *        The closure for the callback.
+   * @param aConn
+   *        The SQLite connection.
+   * @param aPathHead
+   *        Head of the path for the memory report.
+   * @param aKind
+   *        The memory report statistic kind, one of "stmt", "cache" or
+   *        "schema".
+   * @param aDesc
+   *        The memory report description.
+   * @param aOption
+   *        The SQLite constant for getting the measurement.
+   */
+  size_t doConnMeasurement(nsIMemoryMultiReporterCallback *aCallback,
+                           nsISupports *aClosure,
+                           sqlite3 *aConn,
+                           const nsACString &aPathHead,
+                           const nsACString &aKind,
+                           const nsACString &aDesc,
+                           int aOption)
+  {
+    nsCString path(aPathHead);
+    path.Append(aKind);
+    path.AppendLiteral("-used");
+
+    int curr = 0, max = 0;
+    int rc = ::sqlite3_db_status(aConn, aOption, &curr, &max, 0);
+    nsresult rv = convertResultCode(rc);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    aCallback->Callback(NS_LITERAL_CSTRING(""), path,
+                        nsIMemoryReporter::KIND_HEAP,
+                        nsIMemoryReporter::UNITS_BYTES, PRInt64(curr),
+                        aDesc, aClosure);
+    return curr;
+  }
+};
+
+NS_IMPL_THREADSAFE_ISUPPORTS1(
+  StorageSQLiteMultiReporter,
+  nsIMemoryMultiReporter
+)
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Helpers
 
 class ServiceMainThreadInitializer : public nsRunnable
 {
 public:
-  ServiceMainThreadInitializer(nsIObserver *aObserver,
+  ServiceMainThreadInitializer(Service *aService,
+                               nsIObserver *aObserver,
                                nsIXPConnect **aXPConnectPtr,
                                PRInt32 *aSynchronousPrefValPtr)
-  : mObserver(aObserver)
+  : mService(aService)
+  , mObserver(aObserver)
   , mXPConnectPtr(aXPConnectPtr)
   , mSynchronousPrefValPtr(aSynchronousPrefValPtr)
   {
   }
 
   NS_IMETHOD Run()
   {
     NS_PRECONDITION(NS_IsMainThread(), "Must be running on the main thread!");
@@ -184,24 +318,28 @@ public:
 
     // We need to obtain the toolkit.storage.synchronous preferences on the main
     // thread because the preference service can only be accessed there.  This
     // is cached in the service for all future Open[Unshared]Database calls.
     PRInt32 synchronous =
       Preferences::GetInt(PREF_TS_SYNCHRONOUS, PREF_TS_SYNCHRONOUS_DEFAULT);
     ::PR_ATOMIC_SET(mSynchronousPrefValPtr, synchronous);
 
-    // Register our SQLite memory reporter.  Registration can only happen on
-    // the main thread (otherwise you'll get cryptic crashes).
-    NS_RegisterMemoryReporter(new NS_MEMORY_REPORTER_NAME(StorageSQLiteMemoryUsed));
+    // Create and register our SQLite memory reporters.  Registration can only
+    // happen on the main thread (otherwise you'll get cryptic crashes).
+    mService->mStorageSQLiteReporter = new NS_MEMORY_REPORTER_NAME(StorageSQLite);
+    mService->mStorageSQLiteMultiReporter = new StorageSQLiteMultiReporter(mService);
+    (void)::NS_RegisterMemoryReporter(mService->mStorageSQLiteReporter);
+    (void)::NS_RegisterMemoryMultiReporter(mService->mStorageSQLiteMultiReporter);
 
     return NS_OK;
   }
 
 private:
+  Service *mService;
   nsIObserver *mObserver;
   nsIXPConnect **mXPConnectPtr;
   PRInt32 *mSynchronousPrefValPtr;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Service
 
@@ -277,21 +415,26 @@ Service::getSynchronousPref()
   return sSynchronousPref;
 }
 
 Service::Service()
 : mMutex("Service::mMutex")
 , mSqliteVFS(nsnull)
 , mRegistrationMutex("Service::mRegistrationMutex")
 , mConnections()
+, mStorageSQLiteReporter(nsnull)
+, mStorageSQLiteMultiReporter(nsnull)
 {
 }
 
 Service::~Service()
 {
+  (void)::NS_UnregisterMemoryReporter(mStorageSQLiteReporter);
+  (void)::NS_UnregisterMemoryMultiReporter(mStorageSQLiteMultiReporter);
+
   int rc = sqlite3_vfs_unregister(mSqliteVFS);
   if (rc != SQLITE_OK)
     NS_WARNING("Failed to unregister sqlite vfs wrapper.");
 
   // Shutdown the sqlite3 API.  Warn if shutdown did not turn out okay, but
   // there is nothing actionable we can do in that case.
   rc = ::sqlite3_quota_shutdown();
   if (rc != SQLITE_OK)
@@ -467,17 +610,17 @@ Service::initialize()
     return convertResultCode(rc);
 
   // Set the default value for the toolkit.storage.synchronous pref.  It will be
   // updated with the user preference on the main thread.
   sSynchronousPref = PREF_TS_SYNCHRONOUS_DEFAULT;
 
   // Run the things that need to run on the main thread there.
   nsCOMPtr<nsIRunnable> event =
-    new ServiceMainThreadInitializer(this, &sXPConnect, &sSynchronousPref);
+    new ServiceMainThreadInitializer(this, this, &sXPConnect, &sSynchronousPref);
   if (event && ::NS_IsMainThread()) {
     (void)event->Run();
   }
   else {
     (void)::NS_DispatchToMainThread(event);
   }
 
   return NS_OK;
--- a/storage/src/mozStorageService.h
+++ b/storage/src/mozStorageService.h
@@ -48,16 +48,18 @@
 #include "nsIFile.h"
 #include "nsIObserver.h"
 #include "nsTArray.h"
 #include "mozilla/Mutex.h"
 
 #include "mozIStorageService.h"
 #include "mozIStorageServiceQuotaManagement.h"
 
+class nsIMemoryReporter;
+class nsIMemoryMultiReporter;
 class nsIXPConnect;
 struct sqlite3_vfs;
 
 namespace mozilla {
 namespace storage {
 
 class Connection;
 class Service : public mozIStorageService
@@ -182,19 +184,24 @@ private:
    *
    * @note Collation implementations are platform-dependent and in general not
    *       thread-safe.  Access to this collation should be synchronized.
    */
   nsCOMPtr<nsICollation> mLocaleCollation;
 
   nsCOMPtr<nsIFile> mProfileStorageFile;
 
+  nsCOMPtr<nsIMemoryReporter> mStorageSQLiteReporter;
+  nsCOMPtr<nsIMemoryMultiReporter> mStorageSQLiteMultiReporter;
+
   static Service *gService;
 
   static nsIXPConnect *sXPConnect;
 
   static PRInt32 sSynchronousPref;
+
+  friend class ServiceMainThreadInitializer;
 };
 
 } // namespace storage
 } // namespace mozilla
 
 #endif /* MOZSTORAGESERVICE_H */
--- a/toolkit/components/aboutmemory/tests/chrome/Makefile.in
+++ b/toolkit/components/aboutmemory/tests/chrome/Makefile.in
@@ -41,14 +41,13 @@ srcdir		= @srcdir@
 VPATH		= @srcdir@
 relativesrcdir  = toolkit/components/aboutmemory/tests/chrome
 
 include $(DEPTH)/config/autoconf.mk
 include $(topsrcdir)/config/rules.mk
 
 _CHROME_FILES	= \
 		test_aboutmemory.xul \
-		test_asyncClose_leak.xul \
 		$(NULL)
 
 libs:: $(_CHROME_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/chrome/$(relativesrcdir)
 
deleted file mode 100644
--- a/toolkit/components/aboutmemory/tests/chrome/test_asyncClose_leak.xul
+++ /dev/null
@@ -1,110 +0,0 @@
-<?xml version="1.0"?>
-<?xml-stylesheet type="text/css" href="chrome://global/skin"?>
-<?xml-stylesheet type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"?>
-<window title="about:memory"
-        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
-  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
-  <script type="text/javascript" src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
-
-  <!-- test results are displayed in the html:body -->
-  <body xmlns="http://www.w3.org/1999/xhtml"></body>
-
-  <!-- test code goes here -->
-  <script type="application/javascript">
-  <![CDATA[
-
-  // Nb: this test is all JS and so should be done with an xpcshell test,
-  // but bug 671753 is preventing the memory-reporter-manager from being
-  // accessed from xpcshell.
-
-  const Cc = Components.classes;
-  const Ci = Components.interfaces;
-  const Cu = Components.utils;
-
-  // Make a fake DB file.
-  var file = Cc["@mozilla.org/file/directory_service;1"].
-             getService(Ci.nsIProperties).
-             get("ProfD", Ci.nsIFile);
-  file.append("asyncClose_leak-fake-DB-tmp.sqlite");
-
-  var storage = Cc["@mozilla.org/storage/service;1"].
-                getService(Ci.mozIStorageService);
-  var db = storage.openDatabase(file);
-
-  // A statement, the exact form doesn't matter, it just has to be
-  // asynchronous.
-  var stmt = db.createAsyncStatement("SELECT * FROM sqlite_master");
-  try {
-    stmt.executeAsync({
-      handleResult: function(aResults) {
-      },
-      handleError: function(aError) {
-        Cu.reportError("db error: " + aError);
-      },
-      handleCompletion: function(aReason) {
-      }
-    });
-  } catch (e) {
-  } finally {
-    stmt.finalize();
-  }
-  // Correct code would call db.asyncClose() here.  But the point of this
-  // test is to see what happens when we forget this call.
-  //db.asyncClose();
-
-  // We need db to be collected, otherwise this test will fail.  Schedule
-  // three GC+CC runs on the main thread followed by the final check.
-  stmt = null;
-  db = null;
-  function forceCollectionsThenCheck()
-  {
-    function runSoon(f)
-    {
-      var tm = Cc["@mozilla.org/thread-manager;1"]
-                .getService(Ci.nsIThreadManager);
-
-      tm.mainThread.dispatch({ run: f }, Ci.nsIThread.DISPATCH_NORMAL);
-    }
-
-    function doGCandCC()
-    {
-      window.QueryInterface(Ci.nsIInterfaceRequestor)
-            .getInterface(Ci.nsIDOMWindowUtils)
-            .garbageCollect();
-
-      if (++j < 3)
-        runSoon(doGCandCC);
-      else
-        runSoon(doCheck);
-    }
-
-    var j = 0;
-    doGCandCC();
-  }
-
-  // Because we forgot to asyncClose the db, we end up with three "LEAKED"
-  // reporters.
-  function doCheck() {
-    var numLeaked = 0;
-    var mgr = Cc["@mozilla.org/memory-reporter-manager;1"].
-              getService(Ci.nsIMemoryReporterManager);
-    var e = mgr.enumerateReporters(); 
-    while (e.hasMoreElements()) {
-      var r = e.getNext().QueryInterface(Ci.nsIMemoryReporter);
-      if (r.path.search(/sqlite-LEAKED/) != -1) {
-        numLeaked++;
-        // Unregister the leaked reporter, so that if the test is run more
-        // than once, we don't end up with too many of them.
-        mgr.unregisterReporter(r);
-      }
-    }
-    is(numLeaked, 3, "Looking for sqlite-LEAKED reporters");
-    SimpleTest.finish();
-  }
-
-  forceCollectionsThenCheck();
-
-  SimpleTest.waitForExplicitFinish();
-  ]]>
-  </script>
-</window>
--- a/toolkit/components/telemetry/TelemetryPing.js
+++ b/toolkit/components/telemetry/TelemetryPing.js
@@ -52,17 +52,17 @@ const TELEMETRY_INTERVAL = 60000;
 // Delay before intializing telemetry (ms)
 const TELEMETRY_DELAY = 60000;
 // about:memory values to turn into histograms
 const MEM_HISTOGRAMS = {
   "js-gc-heap": "MEMORY_JS_GC_HEAP",
   "js-compartments-system": "MEMORY_JS_COMPARTMENTS_SYSTEM",
   "js-compartments-user": "MEMORY_JS_COMPARTMENTS_USER",
   "resident": "MEMORY_RESIDENT",
-  "explicit/storage/sqlite": "MEMORY_STORAGE_SQLITE",
+  "storage-sqlite": "MEMORY_STORAGE_SQLITE",
   "explicit/images/content/used/uncompressed":
     "MEMORY_IMAGES_CONTENT_USED_UNCOMPRESSED",
   "heap-allocated": "MEMORY_HEAP_ALLOCATED",
   "page-faults-hard": "PAGE_FAULTS_HARD"
 };
 // Seconds of idle time before pinging.
 // On idle-daily a gather-telemetry notification is fired, during it probes can
 // start asynchronous tasks to gather data.  On the next idle the data is sent.