<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[209086] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/209086">209086</a></dd>
<dt>Author</dt> <dd>beidson@apple.com</dd>
<dt>Date</dt> <dd>2016-11-29 13:08:12 -0800 (Tue, 29 Nov 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreModulesindexeddbIDBTransactioncpp">trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp</a></li>
<li><a href="#trunkSourceWebCoreModulesindexeddbIDBTransactionh">trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.h</a></li>
<li><a href="#trunkSourceWebCoreModulesindexeddbclientTransactionOperationh">trunk/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (209085 => 209086)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-11-29 21:04:05 UTC (rev 209085)
+++ trunk/Source/WebCore/ChangeLog 2016-11-29 21:08:12 UTC (rev 209086)
</span><span class="lines">@@ -1,3 +1,44 @@
</span><ins>+2016-11-29 Brady Eidson <beidson@apple.com>
+
+ 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):
+
</ins><span class="cx"> 2016-11-29 Dave Hyatt <hyatt@apple.com>
</span><span class="cx">
</span><span class="cx"> [CSS Parser] Fix ::cue parsing
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesindexeddbIDBTransactioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp (209085 => 209086)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp 2016-11-29 21:04:05 UTC (rev 209085)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp 2016-11-29 21:08:12 UTC (rev 209086)
</span><span class="lines">@@ -251,6 +251,34 @@
</span><span class="cx"> scheduleOperation(IDBClient::createTransactionOperation(*this, nullptr, &IDBTransaction::abortOnServerAndCancelRequests));
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void IDBTransaction::abortInProgressOperations(const IDBError& error)
+{
+ LOG(IndexedDB, "IDBTransaction::abortInProgressOperations");
+
+ Vector<RefPtr<IDBClient::TransactionOperation>> inProgressAbortVector;
+ inProgressAbortVector.reserveInitialCapacity(m_transactionOperationsInProgressQueue.size());
+ while (!m_transactionOperationsInProgressQueue.isEmpty())
+ inProgressAbortVector.uncheckedAppend(m_transactionOperationsInProgressQueue.takeFirst());
+
+ for (auto& operation : inProgressAbortVector) {
+ m_transactionOperationsInProgressQueue.append(operation.get());
+ m_currentlyCompletingRequest = nullptr;
+ operation->doComplete(IDBResultData::error(operation->identifier(), error));
+ }
+
+ Vector<RefPtr<IDBClient::TransactionOperation>> completedOnServerAbortVector;
+ completedOnServerAbortVector.reserveInitialCapacity(m_completedOnServerQueue.size());
+ while (!m_completedOnServerQueue.isEmpty())
+ completedOnServerAbortVector.uncheckedAppend(m_completedOnServerQueue.takeFirst().first);
+
+ for (auto& operation : completedOnServerAbortVector) {
+ m_currentlyCompletingRequest = nullptr;
+ operation->doComplete(IDBResultData::error(operation->identifier(), error));
+ }
+
+ connectionProxy().forgetActiveOperations(inProgressAbortVector);
+}
+
</ins><span class="cx"> void IDBTransaction::abortOnServerAndCancelRequests(IDBClient::TransactionOperation& operation)
</span><span class="cx"> {
</span><span class="cx"> LOG(IndexedDB, "IDBTransaction::abortOnServerAndCancelRequests");
</span><span class="lines">@@ -260,13 +288,19 @@
</span><span class="cx"> m_database->connectionProxy().abortTransaction(*this);
</span><span class="cx">
</span><span class="cx"> ASSERT(m_transactionOperationMap.contains(operation.identifier()));
</span><ins>+ ASSERT(m_transactionOperationsInProgressQueue.last() == &operation);
</ins><span class="cx"> m_transactionOperationMap.remove(operation.identifier());
</span><ins>+ m_transactionOperationsInProgressQueue.removeLast();
</ins><span class="cx">
</span><span class="cx"> m_currentlyCompletingRequest = nullptr;
</span><span class="cx">
</span><span class="cx"> IDBError error(IDBDatabaseException::AbortError);
</span><ins>+
+ abortInProgressOperations(error);
+
</ins><span class="cx"> for (auto& operation : m_abortQueue) {
</span><span class="cx"> m_currentlyCompletingRequest = nullptr;
</span><ins>+ m_transactionOperationsInProgressQueue.append(operation.get());
</ins><span class="cx"> operation->doComplete(IDBResultData::error(operation->identifier(), error));
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -367,10 +401,21 @@
</span><span class="cx"> if (!m_startedOnServer)
</span><span class="cx"> return;
</span><span class="cx">
</span><ins>+ // 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;
+
</ins><span class="cx"> if (!m_pendingTransactionOperationQueue.isEmpty()) {
</span><span class="cx"> auto operation = m_pendingTransactionOperationQueue.takeFirst();
</span><ins>+ m_transactionOperationsInProgressQueue.append(operation.get());
</ins><span class="cx"> operation->perform();
</span><span class="cx">
</span><ins>+ // If this operation we just started has an associated IDBRequest, we might be able to send
+ // another operation to the server before it completes.
+ if (operation->idbRequest() && !m_pendingTransactionOperationQueue.isEmpty())
+ schedulePendingOperationTimer();
+
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1147,6 +1192,10 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ // Since this request won't actually go to the server until the blob writes are complete,
+ // stop future requests from going to the server ahead of it.
+ operation.setNextRequestCanGoToServer(false);
+
</ins><span class="cx"> value->writeBlobsToDiskForIndexedDB([protectedThis = makeRef(*this), this, protectedOperation = Ref<IDBClient::TransactionOperation>(operation), keyData = IDBKeyData(key.get()).isolatedCopy(), overwriteMode](const IDBValue& idbValue) mutable {
</span><span class="cx"> ASSERT(currentThread() == originThreadID());
</span><span class="cx"> ASSERT(isMainThread());
</span><span class="lines">@@ -1238,11 +1287,13 @@
</span><span class="cx"> {
</span><span class="cx"> LOG(IndexedDB, "IDBTransaction::operationCompletedOnClient");
</span><span class="cx">
</span><del>- ASSERT(m_transactionOperationMap.get(operation.identifier()) == &operation);
</del><span class="cx"> ASSERT(currentThread() == m_database->originThreadID());
</span><span class="cx"> ASSERT(currentThread() == operation.originThreadID());
</span><ins>+ ASSERT(m_transactionOperationMap.get(operation.identifier()) == &operation);
+ ASSERT(m_transactionOperationsInProgressQueue.first() == &operation);
</ins><span class="cx">
</span><span class="cx"> m_transactionOperationMap.remove(operation.identifier());
</span><ins>+ m_transactionOperationsInProgressQueue.removeFirst();
</ins><span class="cx">
</span><span class="cx"> schedulePendingOperationTimer();
</span><span class="cx"> }
</span><span class="lines">@@ -1281,11 +1332,15 @@
</span><span class="cx">
</span><span class="cx"> m_state = IndexedDB::TransactionState::Aborting;
</span><span class="cx">
</span><ins>+ abortInProgressOperations(error);
+
</ins><span class="cx"> Vector<RefPtr<IDBClient::TransactionOperation>> operations;
</span><span class="cx"> copyValuesToVector(m_transactionOperationMap, operations);
</span><span class="cx">
</span><span class="cx"> for (auto& operation : operations) {
</span><span class="cx"> m_currentlyCompletingRequest = nullptr;
</span><ins>+ m_transactionOperationsInProgressQueue.append(operation.get());
+ ASSERT(m_transactionOperationsInProgressQueue.first() == operation.get());
</ins><span class="cx"> operation->doComplete(IDBResultData::error(operation->identifier(), error));
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesindexeddbIDBTransactionh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.h (209085 => 209086)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.h 2016-11-29 21:04:05 UTC (rev 209085)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.h 2016-11-29 21:08:12 UTC (rev 209086)
</span><span class="lines">@@ -160,6 +160,7 @@
</span><span class="cx"> void internalAbort();
</span><span class="cx"> void notifyDidAbort(const IDBError&);
</span><span class="cx"> void finishAbortOrCommit();
</span><ins>+ void abortInProgressOperations(const IDBError&);
</ins><span class="cx">
</span><span class="cx"> void scheduleOperation(RefPtr<IDBClient::TransactionOperation>&&);
</span><span class="cx"> void pendingOperationTimerFired();
</span><span class="lines">@@ -243,8 +244,10 @@
</span><span class="cx"> RefPtr<IDBOpenDBRequest> m_openDBRequest;
</span><span class="cx">
</span><span class="cx"> Deque<RefPtr<IDBClient::TransactionOperation>> m_pendingTransactionOperationQueue;
</span><ins>+ Deque<IDBClient::TransactionOperation*> m_transactionOperationsInProgressQueue;
</ins><span class="cx"> Deque<std::pair<RefPtr<IDBClient::TransactionOperation>, IDBResultData>> m_completedOnServerQueue;
</span><span class="cx"> Deque<RefPtr<IDBClient::TransactionOperation>> m_abortQueue;
</span><ins>+
</ins><span class="cx"> HashMap<IDBResourceIdentifier, RefPtr<IDBClient::TransactionOperation>> m_transactionOperationMap;
</span><span class="cx">
</span><span class="cx"> mutable Lock m_referencedObjectStoreLock;
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesindexeddbclientTransactionOperationh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h (209085 => 209086)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h 2016-11-29 21:04:05 UTC (rev 209085)
+++ trunk/Source/WebCore/Modules/indexeddb/client/TransactionOperation.h 2016-11-29 21:08:12 UTC (rev 209086)
</span><span class="lines">@@ -83,7 +83,13 @@
</span><span class="cx"> void doComplete(const IDBResultData& data)
</span><span class="cx"> {
</span><span class="cx"> ASSERT(m_originThreadID == currentThread());
</span><del>- ASSERT(m_completeFunction);
</del><ins>+
+ // Due to race conditions between the server sending an "operation complete" message and the client
+ // forcefully aborting an operation, it's unavoidable that this method might be called twice.
+ // It's okay to handle that gracefully with an early return.
+ if (!m_completeFunction)
+ return;
+
</ins><span class="cx"> m_completeFunction(data);
</span><span class="cx"> m_transaction->operationCompletedOnClient(*this);
</span><span class="cx">
</span><span class="lines">@@ -99,6 +105,9 @@
</span><span class="cx">
</span><span class="cx"> IDBRequest* idbRequest() { return m_idbRequest.get(); }
</span><span class="cx">
</span><ins>+ bool nextRequestCanGoToServer() const { return m_nextRequestCanGoToServer && m_idbRequest; }
+ void setNextRequestCanGoToServer(bool nextRequestCanGoToServer) { m_nextRequestCanGoToServer = nextRequestCanGoToServer; }
+
</ins><span class="cx"> protected:
</span><span class="cx"> TransactionOperation(IDBTransaction& transaction)
</span><span class="cx"> : m_transaction(transaction)
</span><span class="lines">@@ -127,6 +136,7 @@
</span><span class="cx">
</span><span class="cx"> ThreadIdentifier m_originThreadID { currentThread() };
</span><span class="cx"> RefPtr<IDBRequest> m_idbRequest;
</span><ins>+ bool m_nextRequestCanGoToServer { true };
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> template <typename... Arguments>
</span></span></pre>
</div>
</div>
</body>
</html>