Bug 703143 - Use a memory multi-reporter for SQLite's per-connection reporting (attempt 2). r=sdwilsh.

--HG--
extra : rebase_source : c0bf639394d78efa1cd962d27433050f442eaf2b
This commit is contained in:
Nicholas Nethercote 2012-01-03 00:27:05 -08:00
parent a32ae1c040
commit 5f13f50fe4
8 changed files with 237 additions and 167 deletions

View File

@ -550,17 +550,6 @@ Connection::Connection(Service *aService,
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)
@ -722,28 +711,6 @@ Connection::initialize(nsIFile *aDatabaseFile,
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;
}
@ -880,11 +847,6 @@ Connection::internalClose()
}
#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!");
@ -1013,8 +975,9 @@ Connection::Close()
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);

View File

@ -60,13 +60,10 @@ struct PRLock;
class nsIFile;
class nsIEventTarget;
class nsIThread;
class nsIMemoryReporter;
namespace mozilla {
namespace storage {
class StorageMemoryReporter;
class Connection : public mozIStorageConnection
, public nsIInterfaceRequestor
{
@ -225,8 +222,6 @@ private:
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

View File

@ -144,23 +144,171 @@ 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];
// Someone may have closed the Connection, in which case we skip it.
bool isReady;
(void)conn->GetConnectionReady(&isReady);
if (!isReady) {
continue;
}
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;
}
NS_IMETHOD GetExplicitNonHeap(PRInt64 *aAmount)
{
// This reporter doesn't do any non-heap measurements.
*aAmount = 0;
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)
{
@ -195,14 +343,18 @@ public:
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;
@ -288,11 +440,16 @@ Service::Service()
, 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.");
@ -478,7 +635,7 @@ Service::initialize()
// 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();
}

View File

@ -53,6 +53,8 @@
#include "mozIStorageService.h"
#include "mozIStorageServiceQuotaManagement.h"
class nsIMemoryReporter;
class nsIMemoryMultiReporter;
class nsIXPConnect;
struct sqlite3_vfs;
@ -127,7 +129,9 @@ public:
void unregisterConnection(Connection *aConnection);
/**
* Gets the list of open connections.
* Gets the list of open connections. Note that you must test each
* connection with mozIStorageConnection::connectionReady before doing
* anything with it, and skip it if it's not ready.
*
* @pre mRegistrationMutex is not held
*
@ -187,11 +191,16 @@ private:
nsCOMPtr<nsIFile> mProfileStorageFile;
nsCOMPtr<nsIMemoryReporter> mStorageSQLiteReporter;
nsCOMPtr<nsIMemoryMultiReporter> mStorageSQLiteMultiReporter;
static Service *gService;
static nsIXPConnect *sXPConnect;
static PRInt32 sSynchronousPref;
friend class ServiceMainThreadInitializer;
};
} // namespace storage

View File

@ -46,7 +46,7 @@ include $(topsrcdir)/config/rules.mk
_CHROME_FILES = \
test_aboutmemory.xul \
test_asyncClose_leak.xul \
test_sqliteMultiReporter.xul \
$(NULL)
libs:: $(_CHROME_FILES)

View File

@ -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>

View File

@ -0,0 +1,56 @@
<?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[
// Test for bug 708248, where the SQLite memory multi-reporter was
// crashing when a DB was closed.
// 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("test_sqliteMultiReporter-fake-DB-tmp.sqlite");
// Open and close the DB.
var storage = Cc["@mozilla.org/storage/service;1"].
getService(Ci.mozIStorageService);
var db = storage.openDatabase(file);
db.close();
// Invoke all the multi-reporters. The SQLite multi-reporter is among
// them. It shouldn't crash.
var mgr = Cc["@mozilla.org/memory-reporter-manager;1"].
getService(Ci.nsIMemoryReporterManager);
e = mgr.enumerateMultiReporters();
while (e.hasMoreElements()) {
var r = e.getNext().QueryInterface(Ci.nsIMemoryMultiReporter);
r.collectReports(function(){}, null);
}
// If we haven't crashed, we've passed, but the test harness requires that
// we explicitly check something.
ok(true);
SimpleTest.finish();
]]>
</script>
</window>

View File

@ -59,7 +59,7 @@ const MEM_HISTOGRAMS = {
"js-compartments-user": "MEMORY_JS_COMPARTMENTS_USER",
"explicit": "MEMORY_EXPLICIT",
"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",