Bug 1166166 - Shrink storage cache off main thread r?mak
Per bug: it can take around 200ms to shrink memory for
Places.sqlite. This can end up janking the browser if we hit a
memory-pressure. This patch simply removes the #if DEBUG guards
around the mAsyncExecutionThreadIsAlive boolean which is already
being updated and exposes it so that we can shrink memory off
the main thread when possible.
MozReview-Commit-ID: LoDGKrOXs8u
--- a/storage/StorageBaseStatementInternal.cpp
+++ b/storage/StorageBaseStatementInternal.cpp
@@ -131,35 +131,37 @@ StorageBaseStatementInternal::asyncFinal
}
void
StorageBaseStatementInternal::destructorAsyncFinalize()
{
if (!mAsyncStatement)
return;
- // If we reach this point, our owner has not finalized this
- // statement, yet we are being destructed. If possible, we want to
- // auto-finalize it early, to release the resources early.
- nsIEventTarget *target = mDBConnection->getAsyncExecutionTarget();
- if (target) {
- // If we can get the async execution target, we can indeed finalize
- // the statement, as the connection is still open.
- bool isAsyncThread = false;
- (void)target->IsOnCurrentThread(&isAsyncThread);
-
+ bool isOwningThread = false;
+ (void)mDBConnection->threadOpenedOn->IsOnCurrentThread(&isOwningThread);
+ if (isOwningThread) {
+ // If we are the owning thread (currently that means we're also the
+ // main thread), then we can get the async target and just dispatch
+ // to it.
+ nsIEventTarget *target = mDBConnection->getAsyncExecutionTarget();
+ if (target) {
+ nsCOMPtr<nsIRunnable> event =
+ new LastDitchSqliteStatementFinalizer(mDBConnection, mAsyncStatement);
+ (void)target->Dispatch(event, NS_DISPATCH_NORMAL);
+ }
+ } else {
+ // If we're not the owning thread, assume we're the async thread, and
+ // just run the statement.
nsCOMPtr<nsIRunnable> event =
new LastDitchSqliteStatementFinalizer(mDBConnection, mAsyncStatement);
- if (isAsyncThread) {
- (void)event->Run();
- } else {
- (void)target->Dispatch(event, NS_DISPATCH_NORMAL);
- }
+ (void)event->Run();
}
+
// We might not be able to dispatch to the background thread,
// presumably because it is being shutdown. Since said shutdown will
// finalize the statement, we just need to clean-up around here.
mAsyncStatement = nullptr;
}
NS_IMETHODIMP
StorageBaseStatementInternal::NewBindingParamsArray(
--- a/storage/mozStorageConnection.cpp
+++ b/storage/mozStorageConnection.cpp
@@ -451,18 +451,17 @@ public:
, mClone(aClone)
, mReadOnly(aReadOnly)
, mCallback(aCallback)
{
MOZ_ASSERT(NS_IsMainThread());
}
NS_IMETHOD Run() override {
- MOZ_ASSERT (NS_GetCurrentThread() == mConnection->getAsyncExecutionTarget());
-
+ MOZ_ASSERT(!NS_IsMainThread());
nsresult rv = mConnection->initializeClone(mClone, mReadOnly);
if (NS_FAILED(rv)) {
return Dispatch(rv, nullptr);
}
return Dispatch(NS_OK,
NS_ISUPPORTS_CAST(mozIStorageAsyncConnection*, mClone));
}
@@ -577,17 +576,17 @@ Connection::getSqliteRuntimeStatus(int32
if (aMaxValue)
*aMaxValue = max;
return curr;
}
nsIEventTarget *
Connection::getAsyncExecutionTarget()
{
- MutexAutoLock lockedScope(sharedAsyncExecutionMutex);
+ NS_ENSURE_TRUE(NS_IsMainThread(), nullptr);
// If we are shutting down the asynchronous thread, don't hand out any more
// references to the thread.
if (mAsyncExecutionThreadShuttingDown)
return nullptr;
if (!mAsyncExecutionThread) {
static nsThreadPoolNaming naming;
@@ -936,16 +935,23 @@ Connection::isClosing()
bool
Connection::isClosed()
{
MutexAutoLock lockedScope(sharedAsyncExecutionMutex);
return mConnectionClosed;
}
+bool
+Connection::isAsyncExecutionThreadAvailable()
+{
+ MOZ_ASSERT(NS_IsMainThread());
+ return !!mAsyncExecutionThread;
+}
+
void
Connection::shutdownAsyncThread(nsIThread *aThread) {
MOZ_ASSERT(!mAsyncExecutionThread);
MOZ_ASSERT(mAsyncExecutionThreadIsAlive);
MOZ_ASSERT(mAsyncExecutionThreadShuttingDown);
DebugOnly<nsresult> rv = aThread->Shutdown();
MOZ_ASSERT(NS_SUCCEEDED(rv));
@@ -1211,40 +1217,45 @@ Connection::GetInterface(const nsIID &aI
//// mozIStorageConnection
NS_IMETHODIMP
Connection::Close()
{
if (!mDBConn)
return NS_ERROR_NOT_INITIALIZED;
- { // Make sure we have not executed any asynchronous statements.
- // 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);
- }
+#ifdef DEBUG
+ // Since we're accessing mAsyncExecutionThread, we need to be on the opener thread.
+ // We make this check outside of debug code below in setClosedState, but this is
+ // here to be explicit.
+ bool onOpenerThread = false;
+ (void)threadOpenedOn->IsOnCurrentThread(&onOpenerThread);
+ MOZ_ASSERT(onOpenerThread);
+#endif // DEBUG
+
+ // Make sure we have not executed any asynchronous statements.
+ // 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.
+ bool asyncCloseWasCalled = !mAsyncExecutionThread;
+ NS_ENSURE_TRUE(asyncCloseWasCalled, NS_ERROR_UNEXPECTED);
// setClosedState nullifies our connection pointer, so we take a raw pointer
// off it, to pass it through the close procedure.
sqlite3 *nativeConn = mDBConn;
nsresult rv = setClosedState();
NS_ENSURE_SUCCESS(rv, rv);
return internalClose(nativeConn);
}
NS_IMETHODIMP
Connection::AsyncClose(mozIStorageCompletionCallback *aCallback)
{
- if (!NS_IsMainThread()) {
- return NS_ERROR_NOT_SAME_THREAD;
- }
+ NS_ENSURE_TRUE(NS_IsMainThread(), NS_ERROR_NOT_SAME_THREAD);
// The two relevant factors at this point are whether we have a database
// connection and whether we have an async execution thread. Here's what the
// states mean and how we handle them:
//
// - (mDBConn && asyncThread): The expected case where we are either an
// async connection or a sync connection that has been used asynchronously.
// Either way the caller must call us and not Close(). Nothing surprising
@@ -1316,42 +1327,35 @@ Connection::AsyncClose(mozIStorageComple
// setClosedState nullifies our connection pointer, so we take a raw pointer
// off it, to pass it through the close procedure.
sqlite3 *nativeConn = mDBConn;
nsresult rv = setClosedState();
NS_ENSURE_SUCCESS(rv, rv);
// Create and dispatch our close event to the background thread.
- nsCOMPtr<nsIRunnable> closeEvent;
- {
- // We need to lock because we're modifying mAsyncExecutionThread
- MutexAutoLock lockedScope(sharedAsyncExecutionMutex);
- closeEvent = new AsyncCloseConnection(this,
- nativeConn,
- completeEvent,
- mAsyncExecutionThread.forget());
- }
+ nsCOMPtr<nsIRunnable> closeEvent = new AsyncCloseConnection(this,
+ nativeConn,
+ completeEvent,
+ mAsyncExecutionThread.forget());
rv = asyncThread->Dispatch(closeEvent, NS_DISPATCH_NORMAL);
NS_ENSURE_SUCCESS(rv, rv);
return NS_OK;
}
NS_IMETHODIMP
Connection::AsyncClone(bool aReadOnly,
mozIStorageCompletionCallback *aCallback)
{
PROFILER_LABEL("mozStorageConnection", "AsyncClone",
js::ProfileEntry::Category::STORAGE);
- if (!NS_IsMainThread()) {
- return NS_ERROR_NOT_SAME_THREAD;
- }
+ NS_ENSURE_TRUE(NS_IsMainThread(), NS_ERROR_NOT_SAME_THREAD);
if (!mDBConn)
return NS_ERROR_NOT_INITIALIZED;
if (!mDatabaseFile)
return NS_ERROR_UNEXPECTED;
int flags = mFlags;
if (aReadOnly) {
// Turn off SQLITE_OPEN_READWRITE, and set SQLITE_OPEN_READONLY.
@@ -1674,19 +1678,17 @@ Connection::ExecuteAsync(mozIStorageBase
_handle);
}
NS_IMETHODIMP
Connection::ExecuteSimpleSQLAsync(const nsACString &aSQLStatement,
mozIStorageStatementCallback *aCallback,
mozIStoragePendingStatement **_handle)
{
- if (!NS_IsMainThread()) {
- return NS_ERROR_NOT_SAME_THREAD;
- }
+ NS_ENSURE_TRUE(NS_IsMainThread(), NS_ERROR_NOT_SAME_THREAD);
nsCOMPtr<mozIStorageAsyncStatement> stmt;
nsresult rv = CreateAsyncStatement(aSQLStatement, getter_AddRefs(stmt));
if (NS_FAILED(rv)) {
return rv;
}
nsCOMPtr<mozIStoragePendingStatement> pendingStatement;
--- a/storage/mozStorageConnection.h
+++ b/storage/mozStorageConnection.h
@@ -134,27 +134,28 @@ public:
return mDBConn && static_cast<bool>(::sqlite3_get_autocommit(mDBConn));
};
/**
* Lazily creates and returns a background execution thread. In the future,
* the thread may be re-claimed if left idle, so you should call this
* method just before you dispatch and not save the reference.
*
+ * This must be called from the main thread.
+ *
* @returns an event target suitable for asynchronous statement execution.
*/
nsIEventTarget *getAsyncExecutionTarget();
/**
* Mutex used by asynchronous statements to protect state. The mutex is
* declared on the connection object because there is no contention between
* asynchronous statements (they are serialized on mAsyncExecutionThread).
* Currently protects:
* - Connection.mAsyncExecutionThreadShuttingDown
- * - Connection.mAsyncExecutionThread
* - Connection.mConnectionClosed
* - AsyncExecuteStatements.mCancelRequested
*/
Mutex sharedAsyncExecutionMutex;
/**
* Wraps the mutex that SQLite gives us from sqlite3_db_mutex. This is public
* because we already expose the sqlite3* native connection and proper
@@ -229,16 +230,24 @@ public:
/**
* True if the underlying connection is closed.
* Any sqlite resources may be lost when this returns true, so nothing should
* try to use them.
*/
bool isClosed();
+ /**
+ * True if the async execution thread is alive and able to be used (i.e., it
+ * is not in the process of shutting down.)
+ *
+ * This must be called from the main thread.
+ */
+ bool isAsyncExecutionThreadAvailable();
+
nsresult initializeClone(Connection *aClone, bool aReadOnly);
private:
~Connection();
nsresult initializeInternal();
/**
* Sets the database into a closed state so no further actions can be
@@ -301,16 +310,18 @@ private:
* default this will be the leaf of the path to the database file.
*/
nsCString mTelemetryFilename;
/**
* Lazily created thread for asynchronous statement execution. Consumers
* should use getAsyncExecutionTarget rather than directly accessing this
* field.
+ *
+ * This must be accessed only on the main thread.
*/
nsCOMPtr<nsIThread> mAsyncExecutionThread;
/**
* Set to true by Close() or AsyncClose() prior to shutdown.
*
* If false, we guarantee both that the underlying sqlite3 database
* connection is still open and that getAsyncExecutionTarget() can
--- a/storage/mozStorageService.cpp
+++ b/storage/mozStorageService.cpp
@@ -360,18 +360,24 @@ Service::minimizeMemory()
// This is a mozIStorageAsyncConnection, it can only be used on the main
// thread, so we can do a straight API call.
nsCOMPtr<mozIStoragePendingStatement> ps;
DebugOnly<nsresult> rv =
conn->ExecuteSimpleSQLAsync(shrinkPragma, nullptr, getter_AddRefs(ps));
MOZ_ASSERT(NS_SUCCEEDED(rv), "Should have purged sqlite caches");
} else if (NS_SUCCEEDED(conn->threadOpenedOn->IsOnCurrentThread(&onOpenedThread)) &&
onOpenedThread) {
- // We are on the opener thread, so we can just proceed.
- conn->ExecuteSimpleSQL(shrinkPragma);
+ if (conn->isAsyncExecutionThreadAvailable()) {
+ nsCOMPtr<mozIStoragePendingStatement> ps;
+ DebugOnly<nsresult> rv =
+ conn->ExecuteSimpleSQLAsync(shrinkPragma, nullptr, getter_AddRefs(ps));
+ MOZ_ASSERT(NS_SUCCEEDED(rv), "Should have purged sqlite caches");
+ } else {
+ conn->ExecuteSimpleSQL(shrinkPragma);
+ }
} else {
// We are on the wrong thread, the query should be executed on the
// opener thread, so we must dispatch to it.
nsCOMPtr<nsIRunnable> event =
NewRunnableMethod<const nsCString>(
conn, &Connection::ExecuteSimpleSQL, shrinkPragma);
conn->threadOpenedOn->Dispatch(event, NS_DISPATCH_NORMAL);
}
--- a/storage/mozStorageStatementData.h
+++ b/storage/mozStorageStatementData.h
@@ -73,29 +73,16 @@ public:
/**
* NULLs out our sqlite3_stmt (it is held by the owner) after reseting it and
* clear all bindings to it. This is expected to occur on the async thread.
*/
inline void reset()
{
NS_PRECONDITION(mStatementOwner, "Must have a statement owner!");
-#ifdef DEBUG
- {
- nsCOMPtr<nsIEventTarget> asyncThread =
- mStatementOwner->getOwner()->getAsyncExecutionTarget();
- // It's possible that we are shutting down the async thread, and this
- // method would return nullptr as a result.
- if (asyncThread) {
- bool onAsyncThread;
- NS_ASSERTION(NS_SUCCEEDED(asyncThread->IsOnCurrentThread(&onAsyncThread)) && onAsyncThread,
- "This should only be running on the async thread!");
- }
- }
-#endif
// In the AsyncStatement case we may never have populated mStatement if the
// AsyncExecuteStatements got canceled or a failure occurred in constructing
// the statement.
if (mStatement) {
(void)::sqlite3_reset(mStatement);
(void)::sqlite3_clear_bindings(mStatement);
mStatement = nullptr;
}