Changeset 209086 in webkit


Ignore:
Timestamp:
Nov 29, 2016 1:08:12 PM (7 years ago)
Author:
beidson@apple.com
Message:

IndexedDB 2.0: The client's transaction operation queue should flush as much to the server as possible.
​https://bugs.webkit.org/show_bug.cgi?id=164932

Reviewed by Alex Christensen.

No new tests (No new test necessary, covered extensively by all existing tests).

Profiles showed that on tests with lots of rapid IDBRequests in a row, both the main thread and database
threads were largely idle.

The explanation was simple. Currently the client IDBTransaction queues up operations and only vends them out
to the server 1 at a time, waiting for the previous operation to complete.

While some operations do need to wait for the server to reply, by making the change to send most operations
(all operations with an associated IDBRequest) to the server without waiting we get rid of most of the idleness.

It is possible we can find a few other types of operations to send without waiting, but we haven't yet seen any
test case where they would show up on profiles.

Sending more than one operation at a time was actually a very small part of this change.
As many "edge case" regression tests revealed, we also needed to start having IDBTransaction track all of their
"in progress" operations such that they could be aborted on the client side in exceptional circumstances.

  • Modules/indexeddb/IDBTransaction.cpp:

(WebCore::IDBTransaction::abortInProgressOperations): Abort's all in-progress operations (ones that have already

been sent to the server)

(WebCore::IDBTransaction::abortOnServerAndCancelRequests): Abort in-progress operations before pending ones.
(WebCore::IDBTransaction::operationTimerFired): If we just started an operation with an associated IDBRequest,

schedule the timer to send another one right away.

(WebCore::IDBTransaction::operationDidComplete):
(WebCore::IDBTransaction::connectionClosedFromServer): Abort in-progress operations before pending ones.

  • Modules/indexeddb/IDBTransaction.h:
  • Modules/indexeddb/client/TransactionOperation.cpp:

(WebCore::IDBClient::TransactionOperation::TransactionOperation):

  • Modules/indexeddb/client/TransactionOperation.h:

(WebCore::IDBClient::TransactionOperation::completed):
(WebCore::IDBClient::TransactionOperation::hasIDBRequest):

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r209085 r209086  
     12016-11-29  Brady Eidson  <beidson@apple.com>
     2
     3        IndexedDB 2.0: The client's transaction operation queue should flush as much to the server as possible.
     4        https://bugs.webkit.org/show_bug.cgi?id=164932
     5
     6        Reviewed by Alex Christensen.
     7
     8        No new tests (No new test necessary, covered extensively by all existing tests).
     9
     10        Profiles showed that on tests with lots of rapid IDBRequests in a row, both the main thread and database
     11        threads were largely idle.
     12
     13        The explanation was simple. Currently the client IDBTransaction queues up operations and only vends them out
     14        to the server 1 at a time, waiting for the previous operation to complete.
     15
     16        While some operations do need to wait for the server to reply, by making the change to send most operations
     17        (all operations with an associated IDBRequest) to the server without waiting we get rid of most of the idleness.
     18
     19        It is possible we can find a few other types of operations to send without waiting, but we haven't yet seen any
     20        test case where they would show up on profiles.
     21
     22        Sending more than one operation at a time was actually a very small part of this change.
     23        As many "edge case" regression tests revealed, we also needed to start having IDBTransaction track all of their
     24        "in progress" operations such that they could be aborted on the client side in exceptional circumstances.
     25
     26        * Modules/indexeddb/IDBTransaction.cpp:
     27        (WebCore::IDBTransaction::abortInProgressOperations): Abort's all in-progress operations (ones that have already
     28          been sent to the server)
     29        (WebCore::IDBTransaction::abortOnServerAndCancelRequests): Abort in-progress operations before pending ones.
     30        (WebCore::IDBTransaction::operationTimerFired): If we just started an operation with an associated IDBRequest,
     31          schedule the timer to send another one right away.
     32        (WebCore::IDBTransaction::operationDidComplete):
     33        (WebCore::IDBTransaction::connectionClosedFromServer): Abort in-progress operations before pending ones.
     34        * Modules/indexeddb/IDBTransaction.h:
     35
     36        * Modules/indexeddb/client/TransactionOperation.cpp:
     37        (WebCore::IDBClient::TransactionOperation::TransactionOperation):
     38        * Modules/indexeddb/client/TransactionOperation.h:
     39        (WebCore::IDBClient::TransactionOperation::completed):
     40        (WebCore::IDBClient::TransactionOperation::hasIDBRequest):
     41
    1422016-11-29  Dave Hyatt  <hyatt@apple.com>
    243

  • trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp

    r209069 r209086  
    252252}
    253253
     254void IDBTransaction::abortInProgressOperations(const IDBError& error)
     255{
     256    LOG(IndexedDB, "IDBTransaction::abortInProgressOperations");
     257
     258    Vector<RefPtr<IDBClient::TransactionOperation>> inProgressAbortVector;
     259    inProgressAbortVector.reserveInitialCapacity(m_transactionOperationsInProgressQueue.size());
     260    while (!m_transactionOperationsInProgressQueue.isEmpty())
     261        inProgressAbortVector.uncheckedAppend(m_transactionOperationsInProgressQueue.takeFirst());
     262
     263    for (auto& operation : inProgressAbortVector) {
     264        m_transactionOperationsInProgressQueue.append(operation.get());
     265        m_currentlyCompletingRequest = nullptr;
     266        operation->doComplete(IDBResultData::error(operation->identifier(), error));
     267    }
     268
     269    Vector<RefPtr<IDBClient::TransactionOperation>> completedOnServerAbortVector;
     270    completedOnServerAbortVector.reserveInitialCapacity(m_completedOnServerQueue.size());
     271    while (!m_completedOnServerQueue.isEmpty())
     272        completedOnServerAbortVector.uncheckedAppend(m_completedOnServerQueue.takeFirst().first);
     273
     274    for (auto& operation : completedOnServerAbortVector) {
     275        m_currentlyCompletingRequest = nullptr;
     276        operation->doComplete(IDBResultData::error(operation->identifier(), error));
     277    }
     278
     279    connectionProxy().forgetActiveOperations(inProgressAbortVector);
     280}
     281
    254282void IDBTransaction::abortOnServerAndCancelRequests(IDBClient::TransactionOperation& operation)
    255283{
    … …  
    261289
    262290    ASSERT(m_transactionOperationMap.contains(operation.identifier()));
     291    ASSERT(m_transactionOperationsInProgressQueue.last() == &operation);
    263292    m_transactionOperationMap.remove(operation.identifier());
     293    m_transactionOperationsInProgressQueue.removeLast();
    264294
    265295    m_currentlyCompletingRequest = nullptr;
    266296   
    267297    IDBError error(IDBDatabaseException::AbortError);
     298
     299    abortInProgressOperations(error);
     300
    268301    for (auto& operation : m_abortQueue) {
    269302        m_currentlyCompletingRequest = nullptr;
     303        m_transactionOperationsInProgressQueue.append(operation.get());
    270304        operation->doComplete(IDBResultData::error(operation->identifier(), error));
    271305    }
    … …  
    368402        return;
    369403
     404    // If the last in-progress operation we've sent to the server is not an IDBRequest operation,
     405    // then we have to wait until it completes before sending any more.
     406    if (!m_transactionOperationsInProgressQueue.isEmpty() && !m_transactionOperationsInProgressQueue.last()->nextRequestCanGoToServer())
     407        return;
     408
    370409    if (!m_pendingTransactionOperationQueue.isEmpty()) {
    371410        auto operation = m_pendingTransactionOperationQueue.takeFirst();
     411        m_transactionOperationsInProgressQueue.append(operation.get());
    372412        operation->perform();
     413
     414        // If this operation we just started has an associated IDBRequest, we might be able to send
     415        // another operation to the server before it completes.
     416        if (operation->idbRequest() && !m_pendingTransactionOperationQueue.isEmpty())
     417            schedulePendingOperationTimer();
    373418
    374419        return;
    … …  
    11481193    }
    11491194
     1195    // Since this request won't actually go to the server until the blob writes are complete,
     1196    // stop future requests from going to the server ahead of it.
     1197    operation.setNextRequestCanGoToServer(false);
     1198
    11501199    value->writeBlobsToDiskForIndexedDB([protectedThis = makeRef(*this), this, protectedOperation = Ref<IDBClient::TransactionOperation>(operation), keyData = IDBKeyData(key.get()).isolatedCopy(), overwriteMode](const IDBValue& idbValue) mutable {
    11511200        ASSERT(currentThread() == originThreadID());
    … …  
    12391288    LOG(IndexedDB, "IDBTransaction::operationCompletedOnClient");
    12401289
     1290    ASSERT(currentThread() == m_database->originThreadID());
     1291    ASSERT(currentThread() == operation.originThreadID());
    12411292    ASSERT(m_transactionOperationMap.get(operation.identifier()) == &operation);
    1242     ASSERT(currentThread() == m_database->originThreadID());
    1243     ASSERT(currentThread() == operation.originThreadID());
     1293    ASSERT(m_transactionOperationsInProgressQueue.first() == &operation);
    12441294
    12451295    m_transactionOperationMap.remove(operation.identifier());
     1296    m_transactionOperationsInProgressQueue.removeFirst();
    12461297
    12471298    schedulePendingOperationTimer();
    … …  
    12821333    m_state = IndexedDB::TransactionState::Aborting;
    12831334
     1335    abortInProgressOperations(error);
     1336
    12841337    Vector<RefPtr<IDBClient::TransactionOperation>> operations;
    12851338    copyValuesToVector(m_transactionOperationMap, operations);
    … …  
    12871340    for (auto& operation : operations) {
    12881341        m_currentlyCompletingRequest = nullptr;
     1342        m_transactionOperationsInProgressQueue.append(operation.get());
     1343        ASSERT(m_transactionOperationsInProgressQueue.first() == operation.get());
    12891344        operation->doComplete(IDBResultData::error(operation->identifier(), error));
    12901345    }

  • trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.h

    r209069 r209086  
    161161    void notifyDidAbort(const IDBError&);
    162162    void finishAbortOrCommit();
     163    void abortInProgressOperations(const IDBError&);
    163164
    164165    void scheduleOperation(RefPtr<IDBClient::TransactionOperation>&&);
    … …  
    244245
    245246    Deque<RefPtr<IDBClient::TransactionOperation>> m_pendingTransactionOperationQueue;
     247    Deque<IDBClient::TransactionOperation*> m_transactionOperationsInProgressQueue;
    246248    Deque<std::pair<RefPtr<IDBClient::TransactionOperation>, IDBResultData>> m_completedOnServerQueue;
    247249    Deque<RefPtr<IDBClient::TransactionOperation>> m_abortQueue;
     250
    248251    HashMap<IDBResourceIdentifier, RefPtr<IDBClient::TransactionOperation>> m_transactionOperationMap;
    249252

  • trunk/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h

    r209069 r209086  
    8484    {
    8585        ASSERT(m_originThreadID == currentThread());
    86         ASSERT(m_completeFunction);
     86
     87        // Due to race conditions between the server sending an "operation complete" message and the client
     88        // forcefully aborting an operation, it's unavoidable that this method might be called twice.
     89        // It's okay to handle that gracefully with an early return.
     90        if (!m_completeFunction)
     91            return;
     92
    8793        m_completeFunction(data);
    8894        m_transaction->operationCompletedOnClient(*this);
    … …  
    99105
    100106    IDBRequest* idbRequest() { return m_idbRequest.get(); }
     107
     108    bool nextRequestCanGoToServer() const { return m_nextRequestCanGoToServer && m_idbRequest; }
     109    void setNextRequestCanGoToServer(bool nextRequestCanGoToServer) { m_nextRequestCanGoToServer = nextRequestCanGoToServer; }
    101110
    102111protected:
    … …  
    128137    ThreadIdentifier m_originThreadID { currentThread() };
    129138    RefPtr<IDBRequest> m_idbRequest;
     139    bool m_nextRequestCanGoToServer { true };
    130140};
    131141
Note: See TracChangeset for help on using the changeset viewer.