IndexedDB: remove timer for pending operations in IDBTransaction
authorsihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Dec 2019 02:04:53 +0000 (02:04 +0000)
committersihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Dec 2019 02:04:53 +0000 (02:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=205312

Reviewed by Brady Eidson.

When pendingOperationTimer fired, IDBTransasction would try processing pending operations or commiting
automatically.
pendingOperationTimer was scheduled when some conditions changed and IDBTransaction could start processing
pending operations or start commiting, for example, when new pending operations was created.

For better performance, we may start processing right away after the condition change, without using a Timer.
This patch gives us about 10% speed up on test: PerformanceTests/IndexedDB/basic/objectstore-cursor.html.

* Modules/indexeddb/IDBRequest.cpp:
(WebCore::IDBRequest::dispatchEvent):
* Modules/indexeddb/IDBTransaction.cpp:
(WebCore::IDBTransaction::IDBTransaction):
(WebCore::IDBTransaction::abortInProgressOperations):
(WebCore::IDBTransaction::removeRequest):
(WebCore::IDBTransaction::scheduleOperation):
(WebCore::IDBTransaction::finishedDispatchEventForRequest):
(WebCore::IDBTransaction::didStart):
(WebCore::IDBTransaction::operationCompletedOnClient):
(WebCore::IDBTransaction::deactivate):
(WebCore::IDBTransaction::connectionClosedFromServer):
(WebCore::IDBTransaction::handlePendingOperations):
(WebCore::IDBTransaction::autoCommit):
(WebCore::IDBTransaction::trySchedulePendingOperationTimer): Deleted.
(WebCore::IDBTransaction::pendingOperationTimerFired): Deleted.
* Modules/indexeddb/IDBTransaction.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@253807 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBRequest.cpp
Source/WebCore/Modules/indexeddb/IDBTransaction.cpp
Source/WebCore/Modules/indexeddb/IDBTransaction.h
WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme

index 6a7b4d4..71d1b22 100644 (file)
@@ -1,3 +1,36 @@
+2019-12-19  Sihui Liu  <sihui_liu@apple.com>
+
+        IndexedDB: remove timer for pending operations in IDBTransaction
+        https://bugs.webkit.org/show_bug.cgi?id=205312
+
+        Reviewed by Brady Eidson.
+
+        When pendingOperationTimer fired, IDBTransasction would try processing pending operations or commiting 
+        automatically.
+        pendingOperationTimer was scheduled when some conditions changed and IDBTransaction could start processing 
+        pending operations or start commiting, for example, when new pending operations was created.
+
+        For better performance, we may start processing right away after the condition change, without using a Timer.
+        This patch gives us about 10% speed up on test: PerformanceTests/IndexedDB/basic/objectstore-cursor.html.
+
+        * Modules/indexeddb/IDBRequest.cpp:
+        (WebCore::IDBRequest::dispatchEvent):
+        * Modules/indexeddb/IDBTransaction.cpp:
+        (WebCore::IDBTransaction::IDBTransaction):
+        (WebCore::IDBTransaction::abortInProgressOperations):
+        (WebCore::IDBTransaction::removeRequest):
+        (WebCore::IDBTransaction::scheduleOperation):
+        (WebCore::IDBTransaction::finishedDispatchEventForRequest):
+        (WebCore::IDBTransaction::didStart):
+        (WebCore::IDBTransaction::operationCompletedOnClient):
+        (WebCore::IDBTransaction::deactivate):
+        (WebCore::IDBTransaction::connectionClosedFromServer):
+        (WebCore::IDBTransaction::handlePendingOperations):
+        (WebCore::IDBTransaction::autoCommit):
+        (WebCore::IDBTransaction::trySchedulePendingOperationTimer): Deleted.
+        (WebCore::IDBTransaction::pendingOperationTimerFired): Deleted.
+        * Modules/indexeddb/IDBTransaction.h:
+
 2019-12-19  Jack Lee  <shihchieh_lee@apple.com>
 
         Nullptr crash in WebCore::RenderTreeBuilder::attach
index 80ce424..30f6e5c 100644 (file)
@@ -330,10 +330,6 @@ void IDBRequest::dispatchEvent(Event& event)
     if (!m_transaction)
         return;
 
-    // The request should only remain in the transaction's request list if it represents a pending cursor operation, or this is an open request that was blocked.
-    if (!m_pendingCursor && event.type() != eventNames().blockedEvent)
-        m_transaction->removeRequest(*this);
-
     if (m_hasUncaughtException)
         m_transaction->abortDueToFailedRequest(DOMException::create(AbortError, "IDBTransaction will abort due to uncaught exception in an event handler"_s));
     else if (!event.defaultPrevented() && event.type() == eventNames().errorEvent && !m_transaction->isFinishedOrFinishing()) {
@@ -342,6 +338,10 @@ void IDBRequest::dispatchEvent(Event& event)
     }
 
     m_transaction->finishedDispatchEventForRequest(*this);
+
+    // The request should only remain in the transaction's request list if it represents a pending cursor operation, or this is an open request that was blocked.
+    if (!m_pendingCursor && event.type() != eventNames().blockedEvent)
+        m_transaction->removeRequest(*this);
 }
 
 void IDBRequest::uncaughtExceptionInEventHandler()
index 7c763e1..a567093 100644 (file)
@@ -78,7 +78,6 @@ IDBTransaction::IDBTransaction(IDBDatabase& database, const IDBTransactionInfo&
     : IDBActiveDOMObject(database.scriptExecutionContext())
     , m_database(database)
     , m_info(info)
-    , m_pendingOperationTimer(*this, &IDBTransaction::pendingOperationTimerFired)
     , m_openDBRequest(request)
     , m_currentlyCompletingRequest(request)
 
@@ -283,6 +282,7 @@ void IDBTransaction::abortInProgressOperations(const IDBError& error)
         operation->doComplete(IDBResultData::error(operation->identifier(), error));
     }
 
+    m_currentlyCompletingRequest = nullptr;
     connectionProxy().forgetActiveOperations(inProgressAbortVector);
 }
 
@@ -377,7 +377,7 @@ void IDBTransaction::removeRequest(IDBRequest& request)
     ASSERT(canCurrentThreadAccessThreadLocalData(m_database->originThread()));
     m_openRequests.remove(&request);
 
-    trySchedulePendingOperationTimer();
+    autoCommit();
 }
 
 void IDBTransaction::scheduleOperation(Ref<IDBClient::TransactionOperation>&& operation)
@@ -389,53 +389,7 @@ void IDBTransaction::scheduleOperation(Ref<IDBClient::TransactionOperation>&& op
     m_pendingTransactionOperationQueue.append(operation.copyRef());
     m_transactionOperationMap.set(identifier, WTFMove(operation));
 
-    trySchedulePendingOperationTimer();
-}
-
-void IDBTransaction::trySchedulePendingOperationTimer()
-{
-    ASSERT(canCurrentThreadAccessThreadLocalData(m_database->originThread()));
-
-    if (!m_startedOnServer)
-        return;
-
-    // If the last in-progress operation we've sent to the server is not an IDBRequest operation,
-    // then we have to wait until it completes before sending any more.
-    if (!m_transactionOperationsInProgressQueue.isEmpty() && !m_transactionOperationsInProgressQueue.last()->nextRequestCanGoToServer())
-        return;
-
-    if (m_pendingTransactionOperationQueue.isEmpty() && (!m_transactionOperationMap.isEmpty() || !m_openRequests.isEmpty() || isFinishedOrFinishing()))
-        return;
-
-    if (!m_pendingOperationTimer.isActive())
-        m_pendingOperationTimer.startOneShot(0_s);
-}
-
-void IDBTransaction::pendingOperationTimerFired()
-{
-    LOG(IndexedDB, "IDBTransaction::pendingOperationTimerFired (%p)", this);
-    ASSERT(canCurrentThreadAccessThreadLocalData(m_database->originThread()));
-
-    // We want to batch operations together without spinning the runloop for performance,
-    // but don't want to affect responsiveness of the main thread.
-    // This number is a good compromise in ad-hoc testing.
-    static const size_t operationBatchLimit = 128;
-
-    for (size_t iterations = 0; !m_pendingTransactionOperationQueue.isEmpty() && iterations < operationBatchLimit; ++iterations) {
-        auto operation = m_pendingTransactionOperationQueue.takeFirst();
-        m_transactionOperationsInProgressQueue.append(operation.get());
-        operation->perform();
-
-        if (!operation->nextRequestCanGoToServer())
-            break;
-
-    }
-
-    if (!m_transactionOperationMap.isEmpty() || !m_openRequests.isEmpty())
-        return;
-
-    if (!isFinishedOrFinishing())
-        commit();
+    handlePendingOperations();
 }
 
 void IDBTransaction::operationCompletedOnServer(const IDBResultData& data, IDBClient::TransactionOperation& operation)
@@ -480,7 +434,7 @@ void IDBTransaction::completeCursorRequest(IDBRequest& request, const IDBResultD
 
 void IDBTransaction::finishedDispatchEventForRequest(IDBRequest& request)
 {
-    if (isFinishedOrFinishing())
+    if (isFinished())
         return;
 
     ASSERT_UNUSED(request, !m_currentlyCompletingRequest || m_currentlyCompletingRequest == &request);
@@ -543,7 +497,12 @@ void IDBTransaction::didStart(const IDBError& error)
         return;
     }
 
-    trySchedulePendingOperationTimer();
+    handlePendingOperations();
+
+    // It's possible transaction does not create requests (or creates but finishes them early
+    // because of error) during intialization. In this case, since the transaction will
+    // not be active any more, we can end it.
+    autoCommit();
 }
 
 void IDBTransaction::notifyDidAbort(const IDBError& error)
@@ -1394,7 +1353,10 @@ void IDBTransaction::operationCompletedOnClient(IDBClient::TransactionOperation&
     m_transactionOperationMap.remove(operation.identifier());
     m_transactionOperationsInProgressQueue.removeFirst();
 
-    trySchedulePendingOperationTimer();
+    if (m_transactionOperationsInProgressQueue.isEmpty())
+        handlePendingOperations();
+
+    autoCommit();
 }
 
 void IDBTransaction::establishOnServer()
@@ -1422,7 +1384,7 @@ void IDBTransaction::deactivate()
     if (m_state == IndexedDB::TransactionState::Active)
         m_state = IndexedDB::TransactionState::Inactive;
 
-    trySchedulePendingOperationTimer();
+    autoCommit();
 }
 
 void IDBTransaction::connectionClosedFromServer(const IDBError& error)
@@ -1432,6 +1394,11 @@ void IDBTransaction::connectionClosedFromServer(const IDBError& error)
     m_database->willAbortTransaction(*this);
     m_state = IndexedDB::TransactionState::Aborting;
 
+    // Move operations out of m_pendingTransactionOperationQueue, otherwise we may start handling
+    // them after we forcibly complete in-progress transactions.
+    Deque<RefPtr<IDBClient::TransactionOperation>> pendingTransactionOperationQueue;
+    pendingTransactionOperationQueue.swap(m_pendingTransactionOperationQueue);
+
     abortInProgressOperations(error);
 
     auto operations = copyToVector(m_transactionOperationMap.values());
@@ -1442,11 +1409,11 @@ void IDBTransaction::connectionClosedFromServer(const IDBError& error)
         operation->doComplete(IDBResultData::error(operation->identifier(), error));
     }
     m_currentlyCompletingRequest = nullptr;
+    pendingTransactionOperationQueue.clear();
 
     connectionProxy().forgetActiveOperations(operations);
     connectionProxy().forgetTransaction(*this);
 
-    m_pendingTransactionOperationQueue.clear();
     m_abortQueue.clear();
     m_transactionOperationMap.clear();
 
@@ -1465,6 +1432,46 @@ void IDBTransaction::visitReferencedObjectStores(JSC::SlotVisitor& visitor) cons
         visitor.addOpaqueRoot(objectStore.get());
 }
 
+void IDBTransaction::handlePendingOperations()
+{
+    ASSERT(canCurrentThreadAccessThreadLocalData(m_database->originThread()));
+
+    if (!m_startedOnServer)
+        return;
+
+    if (!m_transactionOperationsInProgressQueue.isEmpty() && !m_transactionOperationsInProgressQueue.last()->nextRequestCanGoToServer())
+        return;
+
+    while (!m_pendingTransactionOperationQueue.isEmpty()) {
+        auto operation = m_pendingTransactionOperationQueue.takeFirst();
+        m_transactionOperationsInProgressQueue.append(operation.get());
+        operation->perform();
+
+        if (!operation->nextRequestCanGoToServer())
+            break;
+    }
+}
+
+void IDBTransaction::autoCommit()
+{
+    // If transaction is not inactive, it's active, finished or finishing.
+    // If it's active, it may create new requests, so we cannot commit it.
+    if (m_state != IndexedDB::TransactionState::Inactive)
+        return;
+
+    if (!m_startedOnServer)
+        return;
+
+    if (!m_transactionOperationMap.isEmpty())
+        return;
+
+    if (!m_openRequests.isEmpty())
+        return;
+    ASSERT(!m_currentlyCompletingRequest);
+
+    commit();
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(INDEXED_DATABASE)
index aaa255f..358b01b 100644 (file)
@@ -166,8 +166,9 @@ private:
     void abortInProgressOperations(const IDBError&);
 
     void scheduleOperation(Ref<IDBClient::TransactionOperation>&&);
-    void pendingOperationTimerFired();
     void handleOperationsCompletedOnServer();
+    void handlePendingOperations();
+    void autoCommit();
 
     void fireOnComplete();
     void fireOnAbort();
@@ -239,8 +240,6 @@ private:
     IDBError m_idbError;
     RefPtr<DOMException> m_domError;
 
-    Timer m_pendingOperationTimer;
-
     RefPtr<IDBOpenDBRequest> m_openDBRequest;
 
     Deque<RefPtr<IDBClient::TransactionOperation>> m_pendingTransactionOperationQueue;
index e1c974c..b75f4bd 100644 (file)
@@ -57,7 +57,7 @@
             <BuildableReference
                BuildableIdentifier = "primary"
                BlueprintIdentifier = "FB39D0D01200F0E300088E69"
-               BuildableName = "libANGLE-shared.dylib"
+               BuildableName = "libANGLE.a"
                BlueprintName = "ANGLE"
                ReferencedContainer = "container:Source/ThirdParty/ANGLE/ANGLE.xcodeproj">
             </BuildableReference>