Bug 488148 - Share the mutex used by AsyncExecuteStatements on Connection
☠☠ backed out by 1e873ec2be87 ☠ ☠
authorShawn Wilsher <sdwilsh@shawnwilsher.com>
Wed, 17 Jun 2009 12:12:43 -0700
changeset 29299 0997bcc75daf217886ccf010cdb0b37dc3e30b26
parent 29298 bb7c02ccd0c4f8aed86f95dffae6306e459d6d7f
child 29300 d546bd2c46a4271215bbae1cbe87fdc6975afe21
child 29398 1e873ec2be87523bc8b5a77e581859eef40a6151
push idunknown
push userunknown
push dateunknown
bugs488148
milestone1.9.2a1pre
Bug 488148 - Share the mutex used by AsyncExecuteStatements on Connection Greatly reduces the number of mutexes used when using the asynchronous storage API. r=bent r=asuth
storage/src/mozStorageAsyncStatementExecution.cpp
storage/src/mozStorageAsyncStatementExecution.h
storage/src/mozStorageConnection.cpp
storage/src/mozStorageConnection.h
--- a/storage/src/mozStorageAsyncStatementExecution.cpp
+++ b/storage/src/mozStorageAsyncStatementExecution.cpp
@@ -32,17 +32,16 @@
  * use your version of this file under the terms of the MPL, indicate your
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
-#include "nsAutoLock.h"
 #include "nsAutoPtr.h"
 #include "prtime.h"
 
 #include "sqlite3.h"
 
 #include "mozIStorageStatementCallback.h"
 #include "mozStorageHelper.h"
 #include "mozStorageResultSet.h"
@@ -178,145 +177,99 @@ AsyncExecuteStatements::execute(sqlite3_
                                 mozIStorageStatementCallback *aCallback,
                                 mozIStoragePendingStatement **_stmt)
 {
   // Create our event to run in the background
   nsRefPtr<AsyncExecuteStatements> event =
     new AsyncExecuteStatements(aStatements, aConnection, aCallback);
   NS_ENSURE_TRUE(event, NS_ERROR_OUT_OF_MEMORY);
 
-  nsresult rv = event->initialize();
-  NS_ENSURE_SUCCESS(rv, rv);
-
   // Dispatch it to the background
   nsCOMPtr<nsIEventTarget> target(aConnection->getAsyncExecutionTarget());
   NS_ENSURE_TRUE(target, NS_ERROR_NOT_AVAILABLE);
-  rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
+  nsresult rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Return it as the pending statement object
   NS_ADDREF(*_stmt = event);
   return NS_OK;
 }
 
 AsyncExecuteStatements::AsyncExecuteStatements(sqlite3_stmt_array &aStatements,
-                                               mozIStorageConnection *aConnection,
+                                               Connection *aConnection,
                                                mozIStorageStatementCallback *aCallback)
 : mConnection(aConnection)
 , mTransactionManager(nsnull)
 , mCallback(aCallback)
 , mCallingThread(::do_GetCurrentThread())
 , mMaxIntervalWait(::PR_MicrosecondsToInterval(MAX_MILLISECONDS_BETWEEN_RESULTS))
 , mIntervalStart(::PR_IntervalNow())
 , mState(PENDING)
 , mCancelRequested(PR_FALSE)
-, mLock(nsAutoLock::NewLock("AsyncExecuteStatements::mLock"))
+, mMutex(aConnection->sharedAsyncExecutionMutex)
 {
   (void)mStatements.SwapElements(aStatements);
   NS_ASSERTION(mStatements.Length(), "We weren't given any statements!");
-}
-
-AsyncExecuteStatements::~AsyncExecuteStatements()
-{
-  nsAutoLock::DestroyLock(mLock);
-}
-
-nsresult
-AsyncExecuteStatements::initialize()
-{
-  NS_ENSURE_TRUE(mLock, NS_ERROR_OUT_OF_MEMORY);
   NS_IF_ADDREF(mCallback);
-  return NS_OK;
 }
 
 bool
 AsyncExecuteStatements::shouldNotify()
 {
 #ifdef DEBUG
   PRBool onCallingThread = PR_FALSE;
   (void)mCallingThread->IsOnCurrentThread(&onCallingThread);
   NS_ASSERTION(onCallingThread, "runEvent not running on the calling thread!");
 #endif
 
-  // We do not need to acquire mLock here because it can only ever be written
+  // We do not need to acquire mMutex here because it can only ever be written
   // to on the calling thread, and the only thread that can call us is the
   // calling thread, so we know that our access is serialized.
   return !mCancelRequested;
 }
 
 bool
 AsyncExecuteStatements::executeAndProcessStatement(sqlite3_stmt *aStatement,
                                                    bool aLastStatement)
 {
+  mMutex.AssertNotCurrentThreadOwns();
+
   // We need to hold the mutex for statement execution so we can properly
   // reflect state in case we are canceled.  We release the mutex in a few areas
   // in order to allow for cancelation to occur.
-  nsAutoLock mutex(mLock);
+  MutexAutoLock lockedScope(mMutex);
 
-  nsresult rv = NS_OK;
-  while (true) {
-    int rc = ::sqlite3_step(aStatement);
-    // Break out if we have no more results
-    if (rc == SQLITE_DONE)
-      break;
+  // Execute our statement
+  bool hasResults = executeStatement(aStatement);
 
-    // Some errors are not fatal, and we can handle them and continue.
-    if (rc != SQLITE_OK && rc != SQLITE_ROW) {
-      if (rc == SQLITE_BUSY) {
-        // We do not want to hold our mutex while we yield.
-        nsAutoUnlock cancelationScope(mLock);
+  // If we had an error, bail.
+  if (mState == ERROR)
+    return false;
 
-        // Yield, and try again
-        (void)::PR_Sleep(PR_INTERVAL_NO_WAIT);
-        continue;
-      }
-
-      // Set error state
-      mState = ERROR;
+  // If we have been canceled, there is no point in going on...
+  if (mCancelRequested) {
+    mState = CANCELED;
+    return false;
+  }
 
-      // Drop our mutex - notifyError doesn't want it held
-      mutex.unlock();
+  // Build our result set and notify if we got anything back and have a
+  // callback to notify.
+  if (mCallback && hasResults && NS_FAILED(buildAndNotifyResults(aStatement))) {
+    // We had an error notifying, so we notify on error and stop processing.
+    mState = ERROR;
 
-      // Notify
-      sqlite3 *db = ::sqlite3_db_handle(aStatement);
-      (void)notifyError(rc, ::sqlite3_errmsg(db));
+    {
+      // Drop our mutex because notifyError doesn't want it held.
+      MutexAutoUnlock unlockedScope(mMutex);
 
-      // And stop processing statements
-      return false;
+      // Notify, and stop processing statements.
+      (void)notifyError(mozIStorageError::ERROR,
+                        "An error occurred while notifying about results");
     }
 
-    // If we do not have a callback, there's no point in executing this
-    // statement anymore, but we wish to continue to execute statements.  We
-    // also need to update our state if we are finished, so break out of the
-    // while loop.
-    if (!mCallback)
-      break;
-
-    // If we have been canceled, there is no point in going on...
-    if (mCancelRequested) {
-      mState = CANCELED;
-      return false;
-    }
-
-    // Build our results and notify if it's time.
-    rv = buildAndNotifyResults(aStatement);
-    if (NS_FAILED(rv))
-      break;
-  }
-
-  // If we have an error that we have not already notified about, set our
-  // state accordingly, and notify.
-  if (NS_FAILED(rv)) {
-    mState = ERROR;
-
-    // Drop our mutex - notifyError doesn't want it held
-    mutex.unlock();
-
-    // Notify, and stop processing statements.
-    (void)notifyError(mozIStorageError::ERROR, "");
     return false;
   }
 
 #ifdef DEBUG
   // Check to make sure that this statement was smart about what it did.
   checkAndLogStatementPerformance(aStatement);
 #endif
 
@@ -324,26 +277,68 @@ AsyncExecuteStatements::executeAndProces
   // our mutex.  We would have already returned if we were canceled or had
   // an error at this point.
   if (aLastStatement)
     mState = COMPLETED;
 
   return true;
 }
 
+bool
+AsyncExecuteStatements::executeStatement(sqlite3_stmt *aStatement)
+{
+  mMutex.AssertCurrentThreadOwns();
+
+  while (true) {
+    int rc = ::sqlite3_step(aStatement);
+    // Stop if we have no more results.
+    if (rc == SQLITE_DONE)
+      return false;
+
+    // If we got results, we can return now.
+    if (rc == SQLITE_ROW)
+      return true;
+
+    // Some errors are not fatal, and we can handle them and continue.
+    if (rc == SQLITE_BUSY) {
+      // We do not want to hold our mutex while we yield.
+      MutexAutoUnlock cancelationScope(mMutex);
+
+      // Yield, and try again
+      (void)::PR_Sleep(PR_INTERVAL_NO_WAIT);
+      continue;
+    }
+
+    // Set an error state.
+    mState = ERROR;
+
+    {
+      // Drop our mutex because notifyError doesn't want it held.
+      MutexAutoUnlock unlockedScope(mMutex);
+
+      // And notify.
+      sqlite3 *db = ::sqlite3_db_handle(aStatement);
+      (void)notifyError(rc, ::sqlite3_errmsg(db));
+    }
+
+    // Finally, indicate that we should stop processing.
+    return false;
+  }
+}
+
 nsresult
 AsyncExecuteStatements::buildAndNotifyResults(sqlite3_stmt *aStatement)
 {
   NS_ASSERTION(mCallback, "Trying to dispatch results without a callback!");
-  PR_ASSERT_CURRENT_THREAD_OWNS_LOCK(mLock);
+  mMutex.AssertCurrentThreadOwns();
 
   // At this point, it is safe to not hold the mutex and allow for cancelation.
   // We may add an event to the calling thread, but that thread will not end
   // up running when it checks back with us to see if it should run.
-  nsAutoUnlock cancelationScope(mLock);
+  MutexAutoUnlock cancelationScope(mMutex);
 
   // Build result object if we need it.
   if (!mResultSet)
     mResultSet = new ResultSet();
   NS_ENSURE_TRUE(mResultSet, NS_ERROR_OUT_OF_MEMORY);
 
   nsRefPtr<Row> row(new Row());
   NS_ENSURE_TRUE(row, NS_ERROR_OUT_OF_MEMORY);
@@ -370,16 +365,17 @@ AsyncExecuteStatements::buildAndNotifyRe
   }
 
   return NS_OK;
 }
 
 nsresult
 AsyncExecuteStatements::notifyComplete()
 {
+  mMutex.AssertNotCurrentThreadOwns();
   NS_ASSERTION(mState != PENDING,
                "Still in a pending state when calling Complete!");
 
   // Finalize our statements before we try to commit or rollback.  If we are
   // canceling and have statements that think they have pending work, the
   // rollback will fail.
   for (PRUint32 i = 0; i < mStatements.Length(); i++) {
     (void)::sqlite3_finalize(mStatements[i]);
@@ -417,32 +413,35 @@ AsyncExecuteStatements::notifyComplete()
 
   return NS_OK;
 }
 
 nsresult
 AsyncExecuteStatements::notifyError(PRInt32 aErrorCode,
                                     const char *aMessage)
 {
+  mMutex.AssertNotCurrentThreadOwns();
+
   if (!mCallback)
     return NS_OK;
 
   nsCOMPtr<mozIStorageError> errorObj(new Error(aErrorCode, aMessage));
   NS_ENSURE_TRUE(errorObj, NS_ERROR_OUT_OF_MEMORY);
 
   nsRefPtr<ErrorNotifier> notifier =
     new ErrorNotifier(mCallback, errorObj, this);
   NS_ENSURE_TRUE(notifier, NS_ERROR_OUT_OF_MEMORY);
 
   return mCallingThread->Dispatch(notifier, NS_DISPATCH_NORMAL);
 }
 
 nsresult
 AsyncExecuteStatements::notifyResults()
 {
+  mMutex.AssertNotCurrentThreadOwns();
   NS_ASSERTION(mCallback, "notifyResults called without a callback!");
 
   nsRefPtr<CallbackResultNotifier> notifier =
     new CallbackResultNotifier(mCallback, mResultSet, this);
   NS_ENSURE_TRUE(notifier, NS_ERROR_OUT_OF_MEMORY);
 
   nsresult rv = mCallingThread->Dispatch(notifier, NS_DISPATCH_NORMAL);
   if (NS_SUCCEEDED(rv))
@@ -468,17 +467,17 @@ AsyncExecuteStatements::Cancel(PRBool *_
   NS_ASSERTION(onCallingThread, "Not canceling from the calling thread!");
 #endif
 
   // If we have already canceled, we have an error, but always indicate that
   // we are trying to cancel.
   NS_ENSURE_FALSE(mCancelRequested, NS_ERROR_UNEXPECTED);
 
   {
-    nsAutoLock mutex(mLock);
+    MutexAutoLock lockedScope(mMutex);
 
     // We need to indicate that we want to try and cancel now.
     mCancelRequested = true;
 
     // Establish if we can cancel
     *_successful = (mState == PENDING);
   }
 
@@ -492,25 +491,26 @@ AsyncExecuteStatements::Cancel(PRBool *_
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// nsIRunnable
 
 NS_IMETHODIMP
 AsyncExecuteStatements::Run()
 {
-  // do not run if we have been canceled
+  // Do not run if we have been canceled.
+  bool cancelRequested;
   {
-    nsAutoLock mutex(mLock);
-    if (mCancelRequested) {
+    MutexAutoLock lockedScope(mMutex);
+    cancelRequested = mCancelRequested;
+    if (cancelRequested)
       mState = CANCELED;
-      mutex.unlock();
-      return notifyComplete();
-    }
   }
+  if (cancelRequested)
+    return notifyComplete();
 
   // If there is more than one statement, run it in a transaction.  We assume
   // that we have been given write statements since getting a batch of read
   // statements doesn't make a whole lot of sense.
   if (mStatements.Length() > 1) {
     // We don't error if this failed because it's not terrible if it does.
     mTransactionManager = new mozStorageTransaction(mConnection, PR_FALSE,
                                                     mozIStorageConnection::TRANSACTION_IMMEDIATE);
--- a/storage/src/mozStorageAsyncStatementExecution.h
+++ b/storage/src/mozStorageAsyncStatementExecution.h
@@ -39,16 +39,17 @@
 
 #ifndef _mozStorageAsyncStatementExecution_h_
 #define _mozStorageAsyncStatementExecution_h_
 
 #include "nscore.h"
 #include "nsTArray.h"
 #include "nsAutoPtr.h"
 #include "nsThreadUtils.h"
+#include "mozilla/Mutex.h"
 
 #include "mozIStoragePendingStatement.h"
 #include "mozIStorageStatementCallback.h"
 
 struct sqlite3_stmt;
 class mozStorageTransaction;
 
 namespace mozilla {
@@ -102,81 +103,85 @@ public:
    * should run or not.
    *
    * @returns true if the event should notify still, false otherwise.
    */
   bool shouldNotify();
 
 private:
   AsyncExecuteStatements(sqlite3_stmt_array &aStatements,
-                         mozIStorageConnection *aConnection,
+                         Connection *aConnection,
                          mozIStorageStatementCallback *aCallback);
 
   /**
-   * Initializes the object so it can be run on the background thread.
-   */
-  nsresult initialize();
-
-  ~AsyncExecuteStatements();
-
-  /**
    * Executes a given statement until completion, an error occurs, or we are
    * canceled.  If aLastStatement is true, we should set mState accordingly.
    *
-   * @pre mLock is not held
+   * @pre mMutex is not held
    *
    * @param aStatement
    *        The statement to execute and then process.
    * @param aLastStatement
    *        Indicates if this is the last statement or not.  If it is, we have
    *        to set the proper state.
    * @returns true if we should continue to process statements, false otherwise.
    */
   bool executeAndProcessStatement(sqlite3_stmt *aStatement,
                                   bool aLastStatement);
 
   /**
+   * Executes one step of a statement, properly handling any error conditions.
+   *
+   * @pre mMutex is held
+   *
+   * @param aStatement
+   *        The statement to execute a step on.
+   * @returns true if results were obtained, false otherwise.
+   */
+  bool executeStatement(sqlite3_stmt *aStatement);
+
+  /**
    * Builds a result set up with a row from a given statement.  If we meet the
    * right criteria, go ahead and notify about this results too.
    *
-   * @pre mLock is held
+   * @pre mMutex is held
    *
    * @param aStatement
    *        The statement to get the row data from.
    */
   nsresult buildAndNotifyResults(sqlite3_stmt *aStatement);
 
   /**
    * Notifies callback about completion, and does any necessary cleanup.
    *
-   * @pre mLock is not held
+   * @pre mMutex is not held
    */
   nsresult notifyComplete();
 
   /**
    * Notifies callback about an error.
    *
-   * @pre mLock is not held
+   * @pre mMutex is not held
    *
    * @param aErrorCode
    *        The error code defined in mozIStorageError for the error.
    * @param aMessage
    *        The error string, if any.
    */
   nsresult notifyError(PRInt32 aErrorCode, const char *aMessage);
 
   /**
    * Notifies the callback about a result set.
    *
-   * @pre mLock is not held
+   * @pre mMutex is not held
    */
   nsresult notifyResults();
 
   sqlite3_stmt_array mStatements;
-  mozIStorageConnection *mConnection;
+  nsRefPtr<Connection> mConnection;
   mozStorageTransaction *mTransactionManager;
   mozIStorageStatementCallback *mCallback;
   nsCOMPtr<nsIThread> mCallingThread;
   nsRefPtr<ResultSet> mResultSet;
 
   /**
    * The maximum amount of time we want to wait between results.  Defined by
    * MAX_MILLISECONDS_BETWEEN_RESULTS and set at construction.
@@ -201,15 +206,15 @@ private:
   /**
    * This is the mutex tat protects our state from changing between threads.
    * This includes the following variables:
    *   - mState
    *   - mCancelRequested is only set on the calling thread while the lock is
    *     held.  It is always read from within the lock on the background thread,
    *     but not on the calling thread (see shouldNotify for why).
    */
-  PRLock *mLock;
+  Mutex &mMutex;
 };
 
 } // namespace storage
 } // namespace mozilla
 
 #endif // _mozStorageAsyncStatementExecution_h_
--- a/storage/src/mozStorageConnection.cpp
+++ b/storage/src/mozStorageConnection.cpp
@@ -45,16 +45,17 @@
 #include "nsError.h"
 #include "nsIMutableArray.h"
 #include "nsHashSets.h"
 #include "nsAutoPtr.h"
 #include "nsIFile.h"
 #include "nsIPrefService.h"
 #include "nsIPrefBranch.h"
 #include "nsThreadUtils.h"
+#include "nsAutoLock.h"
 
 #include "mozIStorageAggregateFunction.h"
 #include "mozIStorageFunction.h"
 
 #include "mozStorageAsyncStatementExecution.h"
 #include "mozStorageSQLFunctions.h"
 #include "mozStorageConnection.h"
 #include "mozStorageService.h"
@@ -236,17 +237,18 @@ sqlite3_T_blob(sqlite3_context *aCtx,
   ::sqlite3_result_blob(aCtx, aData, aSize, NS_Free);
   return SQLITE_OK;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Connection
 
 Connection::Connection(mozIStorageService *aService)
-: mDBConn(nsnull)
+: sharedAsyncExecutionMutex("Connection::sharedAsyncExecutionMutex")
+, mDBConn(nsnull)
 , mAsyncExecutionMutex(nsAutoLock::NewLock("AsyncExecutionMutex"))
 , mAsyncExecutionThreadShuttingDown(PR_FALSE)
 , mTransactionMutex(nsAutoLock::NewLock("TransactionMutex"))
 , mTransactionInProgress(PR_FALSE)
 , mFunctionsMutex(nsAutoLock::NewLock("FunctionsMutex"))
 , mProgressHandlerMutex(nsAutoLock::NewLock("ProgressHandlerMutex"))
 , mProgressHandler(nsnull)
 , mStorageService(aService)
--- a/storage/src/mozStorageConnection.h
+++ b/storage/src/mozStorageConnection.h
@@ -37,27 +37,28 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef _MOZSTORAGECONNECTION_H_
 #define _MOZSTORAGECONNECTION_H_
 
 #include "nsCOMPtr.h"
-#include "nsAutoLock.h"
+#include "mozilla/Mutex.h"
 
 #include "nsString.h"
 #include "nsInterfaceHashtable.h"
 #include "mozIStorageProgressHandler.h"
 #include "mozIStorageConnection.h"
 
 #include "nsIMutableArray.h"
 
 #include <sqlite3.h>
 
+struct PRLock;
 class nsIFile;
 class nsIEventTarget;
 class nsIThread;
 class mozIStorageService;
 
 namespace mozilla {
 namespace storage {
 
@@ -81,21 +82,28 @@ public:
 
   // fetch the native handle
   sqlite3 *GetNativeConnection() { return 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.
-   * 
+   *
    * @returns an event target suitable for asynchronous statement execution.
    */
   already_AddRefed<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).
+   */
+  Mutex sharedAsyncExecutionMutex;
+
 private:
   ~Connection();
 
   /**
    * Describes a certain primitive type in the database.
    *
    * Possible Values Are:
    *  INDEX - To check for the existence of an index
@@ -154,17 +162,16 @@ private:
 
   PRLock *mFunctionsMutex;
   nsInterfaceHashtable<nsCStringHashKey, nsISupports> mFunctions;
 
   PRLock *mProgressHandlerMutex;
   nsCOMPtr<mozIStorageProgressHandler> mProgressHandler;
 
   // This isn't accessed but is used to make sure that the connections do
-  // not outlive the service. The service, for example, owns certain locks
-  // in mozStorageAsyncIO file that the connections depend on.
+  // not outlive the service.
   nsCOMPtr<mozIStorageService> mStorageService;
 };
 
 } // namespace storage
 } // namespace mozilla
 
 #endif /* _MOZSTORAGECONNECTION_H_ */