Disk cache: End the throttling experiment and remove

the throttling queue

BUG=54338
TEST=none

Review URL: http://codereview.chromium.org/6602073

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76830 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 28 insertions, 233 deletions

diff --git a/net/disk_cache/backend_impl.cc b/net/disk_cache/backend_impl.cc
index 5491268..cac4656 100644
--- a/net/disk_cache/backend_impl.cc
+++ b/net/disk_cache/backend_impl.cc
@@ -166,12 +166,11 @@ bool InitExperiment(disk_cache::IndexHeader* header) {
 }
 
 // Initializes the field trial structures to allow performance measurements
-// for the current cache configuration. Returns true if we are active part of
-// the CacheThrottle field trial.
-bool SetFieldTrialInfo(int size_group) {
+// for the current cache configuration.
+void SetFieldTrialInfo(int size_group) {
   static bool first = true;
   if (!first)
-    return false;
+    return;
 
   // Field trials involve static objects so we have to do this only once.
   first = false;
@@ -180,15 +179,6 @@ bool SetFieldTrialInfo(int size_group) {
   scoped_refptr<base::FieldTrial> trial1(
       new base::FieldTrial("CacheSize", totalProbability, group1, 2011, 6, 30));
   trial1->AppendGroup(group1, totalProbability);
-
-  // After June 30, 2011 builds, it will always be in default group.
-  scoped_refptr<base::FieldTrial> trial2(
-      new base::FieldTrial(
-          "CacheThrottle", 100, "CacheThrottle_Default", 2011, 6, 30));
-  int group2a = trial2->AppendGroup("CacheThrottle_On", 10);  // 10 % in.
-  trial2->AppendGroup("CacheThrottle_Off", 10);  // 10 % control.
-
-  return trial2->group() == group2a;
 }
 
 // ------------------------------------------------------------------------
@@ -364,7 +354,6 @@ BackendImpl::BackendImpl(const FilePath& path,
       disabled_(false),
       new_eviction_(false),
       first_timer_(true),
-      throttle_requests_(false),
       net_log_(net_log),
       done_(true, false),
       ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)),
@@ -391,7 +380,6 @@ BackendImpl::BackendImpl(const FilePath& path,
       disabled_(false),
       new_eviction_(false),
       first_timer_(true),
-      throttle_requests_(false),
       net_log_(net_log),
       done_(true, false),
       ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)),
@@ -510,8 +498,8 @@ int BackendImpl::SyncInit() {
   }
 
   // Setup load-time data only for the main cache.
-  if (!throttle_requests_ && cache_type() == net::DISK_CACHE)
-    throttle_requests_ = SetFieldTrialInfo(GetSizeGroup());
+  if (cache_type() == net::DISK_CACHE)
+    SetFieldTrialInfo(GetSizeGroup());
 
   eviction_.Init(this);
 
@@ -1146,31 +1134,6 @@ void BackendImpl::OnWrite(int32 bytes) {
   OnRead(bytes);
 }
 
-void BackendImpl::OnOperationCompleted(base::TimeDelta elapsed_time) {
-  CACHE_UMA(TIMES, "TotalIOTime", 0, elapsed_time);
-
-  if (cache_type() != net::DISK_CACHE)
-    return;
-
-  UMA_HISTOGRAM_TIMES(base::FieldTrial::MakeName("DiskCache.TotalIOTime",
-                                                 "CacheThrottle").data(),
-                      elapsed_time);
-
-  if (!throttle_requests_)
-    return;
-
-  const int kMaxNormalDelayMS = 460;
-
-  bool throttling = io_delay_ > kMaxNormalDelayMS;
-
-  // We keep a simple exponential average of elapsed_time.
-  io_delay_ = (io_delay_ + static_cast<int>(elapsed_time.InMilliseconds())) / 2;
-  if (io_delay_ > kMaxNormalDelayMS && !throttling)
-    background_queue_.StartQueingOperations();
-  else if (io_delay_ <= kMaxNormalDelayMS && throttling)
-    background_queue_.StopQueingOperations();
-}
-
 void BackendImpl::OnStatsTimer() {
   stats_.OnEvent(Stats::TIMER);
   int64 time = stats_.GetCounter(Stats::TIMER);
@@ -1248,13 +1211,6 @@ int BackendImpl::RunTaskForTest(Task* task, CompletionCallback* callback) {
   return net::ERR_IO_PENDING;
 }
 
-void BackendImpl::ThrottleRequestsForTest(bool throttle) {
-  if (throttle)
-    background_queue_.StartQueingOperations();
-  else
-    background_queue_.StopQueingOperations();
-}
-
 void BackendImpl::TrimForTest(bool empty) {
   eviction_.SetTestMode();
   eviction_.TrimCache(empty);
diff --git a/net/disk_cache/backend_impl.h b/net/disk_cache/backend_impl.h
index 9be5d4a..92d207b 100644
--- a/net/disk_cache/backend_impl.h
+++ b/net/disk_cache/backend_impl.h
@@ -205,9 +205,6 @@ class BackendImpl : public Backend {
   void OnRead(int bytes);
   void OnWrite(int bytes);
 
-  // Keeps track of the time needed to complete some IO operations.
-  void OnOperationCompleted(base::TimeDelta elapsed_time);
-
   // Timer callback to calculate usage statistics.
   void OnStatsTimer();
 
@@ -237,9 +234,6 @@ class BackendImpl : public Backend {
   // deleted after it runs.
   int RunTaskForTest(Task* task, CompletionCallback* callback);
 
-  // Starts or stops throttling requests.
-  void ThrottleRequestsForTest(bool throttle);
-
   // Trims an entry (all if |empty| is true) from the list of deleted
   // entries. This method should be called directly on the cache thread.
   void TrimForTest(bool empty);
diff --git a/net/disk_cache/entry_unittest.cc b/net/disk_cache/entry_unittest.cc
index 21d5839..9c6b28a 100644
--- a/net/disk_cache/entry_unittest.cc
+++ b/net/disk_cache/entry_unittest.cc
@@ -476,75 +476,6 @@ TEST_F(DiskCacheEntryTest, MemoryOnlyExternalAsyncIO) {
   ExternalAsyncIO();
 }
 
-TEST_F(DiskCacheEntryTest, RequestThrottling) {
-  SetDirectMode();
-  InitCache();
-  disk_cache::Entry* entry = NULL;
-  ASSERT_EQ(net::OK, CreateEntry("the first key", &entry));
-  ASSERT_TRUE(NULL != entry);
-
-  // Let's verify that each IO goes to the right callback object.
-  CallbackTest cb(true);
-
-  g_cache_tests_error = false;
-  g_cache_tests_received = 0;
-
-  MessageLoopHelper helper;
-
-  const int kSize = 200;
-  scoped_refptr<net::IOBuffer> buffer = new net::IOBuffer(kSize);
-  CacheTestFillBuffer(buffer->data(), kSize, false);
-
-  int expected = 0;
-  // Start with no throttling.
-  for (; expected < 10; expected++) {
-    int ret = entry->WriteData(0, 0, buffer, kSize, &cb, false);
-    EXPECT_EQ(net::ERR_IO_PENDING, ret);
-  }
-  EXPECT_TRUE(helper.WaitUntilCacheIoFinished(expected));
-
-  // And now with full throttling.
-  cache_impl_->ThrottleRequestsForTest(true);
-  for (; expected < 20; expected++) {
-    int ret = entry->WriteData(0, 0, buffer, kSize, &cb, false);
-    EXPECT_EQ(net::ERR_IO_PENDING, ret);
-  }
-  EXPECT_TRUE(helper.WaitUntilCacheIoFinished(expected));
-
-  for (; expected < 30; expected++) {
-    int ret = entry->WriteData(0, 0, buffer, kSize, &cb, false);
-    EXPECT_EQ(net::ERR_IO_PENDING, ret);
-  }
-  // We have 9 queued requests, let's dispatch them all.
-  cache_impl_->ThrottleRequestsForTest(false);
-  EXPECT_TRUE(helper.WaitUntilCacheIoFinished(expected));
-
-  cache_impl_->ThrottleRequestsForTest(true);
-  for (; expected < 40; expected++) {
-    int ret = entry->WriteData(0, 0, buffer, kSize, &cb, false);
-    EXPECT_EQ(net::ERR_IO_PENDING, ret);
-  }
-
-  // We can close the entry and keep receiving notifications.
-  entry->Close();
-  EXPECT_TRUE(helper.WaitUntilCacheIoFinished(expected));
-
-  ASSERT_EQ(net::OK, OpenEntry("the first key", &entry));
-  for (; expected < 50; expected++) {
-    int ret = entry->WriteData(0, 0, buffer, kSize, &cb, false);
-    EXPECT_EQ(net::ERR_IO_PENDING, ret);
-  }
-
-  // ... and even close the cache.
-  entry->Close();
-  delete cache_impl_;
-  cache_ = cache_impl_ = NULL;
-  EXPECT_TRUE(helper.WaitUntilCacheIoFinished(expected));
-
-  EXPECT_FALSE(g_cache_tests_error);
-  EXPECT_EQ(expected, g_cache_tests_received);
-}
-
 void DiskCacheEntryTest::StreamAccess() {
   disk_cache::Entry* entry = NULL;
   ASSERT_EQ(net::OK, CreateEntry("the first key", &entry));
diff --git a/net/disk_cache/in_flight_backend_io.cc b/net/disk_cache/in_flight_backend_io.cc
index 5752353..5d4be18 100644
--- a/net/disk_cache/in_flight_backend_io.cc
+++ b/net/disk_cache/in_flight_backend_io.cc
@@ -275,8 +275,6 @@ void BackendIO::ExecuteEntryOperation() {
       NOTREACHED() << "Invalid Operation";
       result_ = net::ERR_UNEXPECTED;
   }
-  // We added a reference to protect the queued operation.
-  entry_->Release();
   if (result_ != net::ERR_IO_PENDING)
     controller_->OnIOComplete(this);
 }
@@ -286,9 +284,7 @@ void BackendIO::ExecuteEntryOperation() {
 InFlightBackendIO::InFlightBackendIO(BackendImpl* backend,
                     base::MessageLoopProxy* background_thread)
     : backend_(backend),
-      background_thread_(background_thread),
-      max_queue_len_(0),
-      queue_entry_ops_(false) {
+      background_thread_(background_thread) {
 }
 
 InFlightBackendIO::~InFlightBackendIO() {
@@ -297,34 +293,34 @@ InFlightBackendIO::~InFlightBackendIO() {
 void InFlightBackendIO::Init(CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->Init();
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::OpenEntry(const std::string& key, Entry** entry,
                                   CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->OpenEntry(key, entry);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::CreateEntry(const std::string& key, Entry** entry,
                                     CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->CreateEntry(key, entry);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::DoomEntry(const std::string& key,
                                   CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->DoomEntry(key);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::DoomAllEntries(CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->DoomAllEntries();
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::DoomEntriesBetween(const base::Time initial_time,
@@ -332,58 +328,58 @@ void InFlightBackendIO::DoomEntriesBetween(const base::Time initial_time,
                         CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->DoomEntriesBetween(initial_time, end_time);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::DoomEntriesSince(const base::Time initial_time,
                                          CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->DoomEntriesSince(initial_time);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::OpenNextEntry(void** iter, Entry** next_entry,
                                       CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->OpenNextEntry(iter, next_entry);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::OpenPrevEntry(void** iter, Entry** prev_entry,
                                       CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->OpenPrevEntry(iter, prev_entry);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::EndEnumeration(void* iterator) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, NULL));
   operation->EndEnumeration(iterator);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::CloseEntryImpl(EntryImpl* entry) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, NULL));
   operation->CloseEntryImpl(entry);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::DoomEntryImpl(EntryImpl* entry) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, NULL));
   operation->DoomEntryImpl(entry);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::FlushQueue(net::CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->FlushQueue();
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::RunTask(Task* task, net::CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->RunTask(task);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::ReadData(EntryImpl* entry, int index, int offset,
@@ -391,7 +387,7 @@ void InFlightBackendIO::ReadData(EntryImpl* entry, int index, int offset,
                                  CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->ReadData(entry, index, offset, buf, buf_len);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::WriteData(EntryImpl* entry, int index, int offset,
@@ -400,7 +396,7 @@ void InFlightBackendIO::WriteData(EntryImpl* entry, int index, int offset,
                                   CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->WriteData(entry, index, offset, buf, buf_len, truncate);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::ReadSparseData(EntryImpl* entry, int64 offset,
@@ -408,7 +404,7 @@ void InFlightBackendIO::ReadSparseData(EntryImpl* entry, int64 offset,
                                        CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->ReadSparseData(entry, offset, buf, buf_len);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::WriteSparseData(EntryImpl* entry, int64 offset,
@@ -416,7 +412,7 @@ void InFlightBackendIO::WriteSparseData(EntryImpl* entry, int64 offset,
                                         CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->WriteSparseData(entry, offset, buf, buf_len);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::GetAvailableRange(EntryImpl* entry, int64 offset,
@@ -424,112 +420,42 @@ void InFlightBackendIO::GetAvailableRange(EntryImpl* entry, int64 offset,
                                           CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->GetAvailableRange(entry, offset, len, start);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::CancelSparseIO(EntryImpl* entry) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, NULL));
   operation->CancelSparseIO(entry);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::ReadyForSparseIO(EntryImpl* entry,
                                          CompletionCallback* callback) {
   scoped_refptr<BackendIO> operation(new BackendIO(this, backend_, callback));
   operation->ReadyForSparseIO(entry);
-  QueueOperation(operation);
+  PostOperation(operation);
 }
 
 void InFlightBackendIO::WaitForPendingIO() {
   InFlightIO::WaitForPendingIO();
 }
 
-void InFlightBackendIO::StartQueingOperations() {
-  queue_entry_ops_ = true;
-  CACHE_UMA(COUNTS_10000, "InitialQueuedOperations", 0, pending_ops_.size());
-  if (!pending_ops_.size()) {
-    queueing_start_ = base::TimeTicks::Now();
-    max_queue_len_ = 0;
-  }
-}
-
-void InFlightBackendIO::StopQueingOperations() {
-  queue_entry_ops_ = false;
-  CACHE_UMA(COUNTS_10000, "FinalQueuedOperations", 0, pending_ops_.size());
-}
-
 void InFlightBackendIO::OnOperationComplete(BackgroundIO* operation,
                                             bool cancel) {
   BackendIO* op = static_cast<BackendIO*>(operation);
 
   if (op->IsEntryOperation()) {
-    backend_->OnOperationCompleted(op->ElapsedTime());
-    if (!pending_ops_.empty() && RemoveFirstQueuedOperation(op)) {
-      // Process the next request. Note that invoking the callback may result
-      // in the backend destruction (and with it this object), so we should deal
-      // with the next operation before invoking the callback.
-      PostQueuedOperation();
-      if (!pending_ops_.size() && !queue_entry_ops_) {
-        CACHE_UMA(AGE_MS, "ThrottleTime", 0, queueing_start_);
-        CACHE_UMA(COUNTS_10000, "MaxQueuedOperations", 0, max_queue_len_);
-      }
-    }
+    CACHE_UMA(TIMES, "TotalIOTime", 0, op->ElapsedTime());
   }
 
   if (op->callback() && (!cancel || op->IsEntryOperation()))
     op->callback()->Run(op->result());
 }
 
-void InFlightBackendIO::QueueOperation(BackendIO* operation) {
-  if (!operation->IsEntryOperation())
-    return PostOperation(operation);
-
-  // We have to protect the entry from deletion while it is on the queue.
-  // If the caller closes the entry right after writing to it, and the write is
-  // waiting on the queue, we could end up deleting the entry before the write
-  // operation is actually posted. Sending a task to reference the entry we make
-  // sure that there is an extra reference before the caller can post a task to
-  // release its reference.
-  background_thread_->PostTask(FROM_HERE,
-      NewRunnableMethod(operation, &BackendIO::ReferenceEntry));
-
-  bool empty_list = pending_ops_.empty();
-  if (!queue_entry_ops_ && empty_list)
-    return PostOperation(operation);
-
-  CACHE_UMA(COUNTS_10000, "QueuedOperations", 0, pending_ops_.size());
-
-  // We keep the operation that we are executing in the list so that we know
-  // when it completes.
-  pending_ops_.push_back(operation);
-  max_queue_len_ = std::max(max_queue_len_, pending_ops_.size());
-  if (empty_list)
-    PostOperation(operation);
-}
-
 void InFlightBackendIO::PostOperation(BackendIO* operation) {
   background_thread_->PostTask(FROM_HERE,
       NewRunnableMethod(operation, &BackendIO::ExecuteOperation));
   OnOperationPosted(operation);
 }
 
-void InFlightBackendIO::PostQueuedOperation() {
-  if (pending_ops_.empty())
-    return;
-
-  // Keep it in the list until it's done.
-  scoped_refptr<BackendIO> next_op = pending_ops_.front();
-  PostOperation(next_op);
-}
-
-bool InFlightBackendIO::RemoveFirstQueuedOperation(BackendIO* operation) {
-  DCHECK(!pending_ops_.empty());
-  scoped_refptr<BackendIO> next_op = pending_ops_.front();
-  if (operation != next_op)
-    return false;
-
-  pending_ops_.pop_front();
-  return true;
-}
-
 }  // namespace
diff --git a/net/disk_cache/in_flight_backend_io.h b/net/disk_cache/in_flight_backend_io.h
index 7b4b111..89fe3c1 100644
--- a/net/disk_cache/in_flight_backend_io.h
+++ b/net/disk_cache/in_flight_backend_io.h
@@ -188,26 +188,14 @@ class InFlightBackendIO : public InFlightIO {
     return background_thread_->BelongsToCurrentThread();
   }
 
-  // Controls the queing of entry (async) operations.
-  void StartQueingOperations();
-  void StopQueingOperations();
-
  protected:
   virtual void OnOperationComplete(BackgroundIO* operation, bool cancel);
 
  private:
-  typedef std::list<scoped_refptr<BackendIO> > OperationList;
-  void QueueOperation(BackendIO* operation);
   void PostOperation(BackendIO* operation);
-  void PostQueuedOperation();
-  bool RemoveFirstQueuedOperation(BackendIO* operation);
 
   BackendImpl* backend_;
   scoped_refptr<base::MessageLoopProxy> background_thread_;
-  OperationList pending_ops_;  // Entry (async) operations to be posted.
-  base::TimeTicks queueing_start_;
-  size_t max_queue_len_;
-  bool queue_entry_ops_;  // True if we are queuing entry (async) operations.
 
   DISALLOW_COPY_AND_ASSIGN(InFlightBackendIO);
 };