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 idunknown
push userunknown
push dateunknown
reviewerssdwilsh
bugs703143
milestone11.0a1
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.