In AsyncExecuteStatements::executeStatement(), there are some changes we could make to improve locking and how we handle SQLITE_BUSY.

1. If we get the SQLITE_BUSY return value from Connection::stepStatement(), we end up in a loop repeatedly calling Connection::stepStatement() until we get a different return value. We should check mCancelRequested during the loop and stop retrying the statement if we've been cancelled.

2. Instead of calling PR_Sleep(PR_INTERVAL_NO_WAIT) to yield between sqlite calls, we should refactor the code to use a condition variable and block until signaled to avoid wasting CPU cycles.

3. In the SQLITE_BUSY handling, we could eliminate one cycle of releasing the locking and acquiring it again to make the code slightly more efficient. When we handle the SQLITE_BUSY return value in AsyncExecuteStatements::executeStatement(), we first release mDBMutex to call PR_Sleep(), then we acquire the lock again to call sqlite_reset, then we release the mutex again when using 'continue' to go to the next loop iteration, then we acquire the once more before calling Connection::stepStatement(). We could refactor the code to release the mutex once.

Found while looking into bug 1567018. See also the discussion on bug 1435446 comment 51 and later.

(In reply to Haik Aftandilian [:haik] from comment #0)

2. Instead of calling PR_Sleep(PR_INTERVAL_NO_WAIT) to yield between sqlite calls, we should refactor the code to use a condition variable and block until signaled to avoid wasting CPU cycles.

After looking at the code more, I'm not sure if this makes sense. I was assuming we are waiting for another thread to finish executing a statement, but if we are potentially waiting for something in another process, this change wouldn't make sense.

Check for cancellation in SQLITE_BUSY loop and refactor SQLITE_BUSY handling to release/acquire mMutex only once.

https://hg.mozilla.org/mozilla-central/rev/fba4303d6219