[IndexedDB] Transaction limitting

content/browser/indexed_db/indexed_db_transaction_coordinator.cc

Index: content/browser/indexed_db/indexed_db_transaction_coordinator.cc
diff --git a/content/browser/indexed_db/indexed_db_transaction_coordinator.cc b/content/browser/indexed_db/indexed_db_transaction_coordinator.cc
index 80345303ab195efadd7a765b33fb8d8b154033b1..e6ba835375d96fc9bea921e10181493658d68fd7 100644
--- a/content/browser/indexed_db/indexed_db_transaction_coordinator.cc
+++ b/content/browser/indexed_db/indexed_db_transaction_coordinator.cc
@@ -5,10 +5,14 @@
 #include "content/browser/indexed_db/indexed_db_transaction_coordinator.h"
 
 #include "base/logging.h"
+#include "content/browser/indexed_db/indexed_db_tracing.h"
 #include "content/browser/indexed_db/indexed_db_transaction.h"
 #include "third_party/WebKit/public/platform/modules/indexeddb/WebIDBTypes.h"
 
 namespace content {
+namespace {
+static const size_t kMaxStartedTransactions = 10;

[cmumford, 2017/02/16 23:16:05]

Can you add a comment explaining why this has a value of 10?

[pwnall, 2017/02/16 23:19:40]

Can you explain that this limits the number of transactions per database? It'd
also help if you mention what happens when the limit is reached -- seems like
new transactions will remain queued?

[dmurph, 2017/02/16 23:32:34]

Done.

+}  // namespace
 
 IndexedDBTransactionCoordinator::IndexedDBTransactionCoordinator() {}
 
@@ -35,6 +39,7 @@ void IndexedDBTransactionCoordinator::DidCreateObserverTransaction(
   DCHECK_EQ(IndexedDBTransaction::CREATED, transaction->state());
 
   started_transactions_.insert_front(transaction);
+  RecordMetrics();
   ProcessQueuedTransactions();
 }
 
@@ -47,7 +52,7 @@ void IndexedDBTransactionCoordinator::DidFinishTransaction(
     DCHECK(started_transactions_.count(transaction));
     started_transactions_.erase(transaction);
   }
-
+  RecordMetrics();
   ProcessQueuedTransactions();
 }
 
@@ -84,6 +89,12 @@ IndexedDBTransactionCoordinator::GetTransactions() const {
   return result;
 }
 
+void IndexedDBTransactionCoordinator::RecordMetrics() const {
+  IDB_TRACE_COUNTER2("TransactionCoordinator", "StartedTransactions",
+                     started_transactions_.size(), "QueuedTransactions",
+                     queued_transactions_.size());
+}
+
 void IndexedDBTransactionCoordinator::ProcessQueuedTransactions() {
   if (queued_transactions_.empty())
     return;
@@ -110,7 +121,8 @@ void IndexedDBTransactionCoordinator::ProcessQueuedTransactions() {
   while (it != queued_transactions_.end()) {
     IndexedDBTransaction* transaction = *it;
     ++it;
-    if (CanStartTransaction(transaction, locked_scope)) {
+    if (CanStartTransaction(transaction, locked_scope,
+                            started_transactions_.size())) {
       DCHECK_EQ(IndexedDBTransaction::CREATED, transaction->state());
       queued_transactions_.erase(transaction);
       started_transactions_.insert(transaction);
@@ -125,6 +137,7 @@ void IndexedDBTransactionCoordinator::ProcessQueuedTransactions() {
                           transaction->scope().end());
     }
   }
+  RecordMetrics();
 }
 
 template<typename T>
@@ -145,7 +158,11 @@ static bool DoSetsIntersect(const std::set<T>& set1,
 
 bool IndexedDBTransactionCoordinator::CanStartTransaction(
     IndexedDBTransaction* const transaction,
-    const std::set<int64_t>& locked_scope) const {
+    const std::set<int64_t>& locked_scope,
+    size_t num_started_txns) const {

[cmumford, 2017/02/16 23:16:05]

This is a non-static member function. Why not just access started_transactions_
directly? Seems unnecessarily complex.

[dmurph, 2017/02/16 23:32:34]

Whoops, yes much better.

+  if (num_started_txns >= kMaxStartedTransactions) {
+    return false;
+  }
   DCHECK(queued_transactions_.count(transaction));
   switch (transaction->mode()) {
     case blink::WebIDBTransactionModeVersionChange: