Disk cache: Make sure that we don't keep looping trying

to evict entries from the cache.

Also, consider the number of requests and returned data
when deciding if the cache is under load, and not just the
number of pending operations.

BUG=68894
TEST=none
Review URL: http://codereview.chromium.org/8392020

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

3 files changed, 55 insertions, 45 deletions

diff --git a/net/disk_cache/backend_impl.cc b/net/disk_cache/backend_impl.cc
index 0d9c7e3..d482386 100644
--- a/net/disk_cache/backend_impl.cc
+++ b/net/disk_cache/backend_impl.cc
@@ -183,25 +183,25 @@ bool InitExperiment(disk_cache::IndexHeader* header, uint32 mask) {
   }
 
   if (!header->create_time || !header->lru.filled)
-    return true; // Wait untill we fill up the cache.
+    return true;  // Wait untill we fill up the cache.
 
   int index_load = header->num_entries * 100 / (mask + 1);
   if (index_load > 25) {
-   // Out of the experiment (~18% users).
-   header->experiment = disk_cache::EXPERIMENT_DELETED_LIST_OUT2;
-   return true;
+    // Out of the experiment (~18% users).
+    header->experiment = disk_cache::EXPERIMENT_DELETED_LIST_OUT2;
+    return true;
   }
 
   int option = base::RandInt(0, 4);
   if (option > 1) {
-   // 60% out (49% of the total).
-   header->experiment = disk_cache::EXPERIMENT_DELETED_LIST_OUT2;
+    // 60% out (49% of the total).
+    header->experiment = disk_cache::EXPERIMENT_DELETED_LIST_OUT2;
   } else if (!option) {
-   // About 16% of the total.
-   header->experiment = disk_cache::EXPERIMENT_DELETED_LIST_CONTROL;
+    // About 16% of the total.
+    header->experiment = disk_cache::EXPERIMENT_DELETED_LIST_CONTROL;
   } else {
-   // About 16% of the total.
-   header->experiment = disk_cache::EXPERIMENT_DELETED_LIST_IN;
+    // About 16% of the total.
+    header->experiment = disk_cache::EXPERIMENT_DELETED_LIST_IN;
   }
 
   SetFieldTrialInfo(header->experiment);
@@ -381,6 +381,7 @@ BackendImpl::BackendImpl(const FilePath& path,
       disabled_(false),
       new_eviction_(false),
       first_timer_(true),
+      user_load_(false),
       net_log_(net_log),
       done_(true, false),
       ALLOW_THIS_IN_INITIALIZER_LIST(ptr_factory_(this)) {
@@ -406,6 +407,7 @@ BackendImpl::BackendImpl(const FilePath& path,
       disabled_(false),
       new_eviction_(false),
       first_timer_(true),
+      user_load_(false),
       net_log_(net_log),
       done_(true, false),
       ALLOW_THIS_IN_INITIALIZER_LIST(ptr_factory_(this)) {
@@ -1108,7 +1110,7 @@ bool BackendImpl::IsLoaded() const {
   if (user_flags_ & kNoLoadProtection)
     return false;
 
-  return num_pending_io_ > 5;
+  return (num_pending_io_ > 5 || user_load_);
 }
 
 std::string BackendImpl::HistogramName(const char* name, int experiment) const {
@@ -1260,6 +1262,9 @@ void BackendImpl::OnStatsTimer() {
 
   CACHE_UMA(COUNTS_10000, "EntryAccessRate", 0, entry_count_);
   CACHE_UMA(COUNTS, "ByteIORate", 0, byte_count_ / 1024);
+
+  // These values cover about 99.5% of the population (Oct 2011).
+  user_load_ = (entry_count_ > 300 || byte_count_ > 7 * 1024 * 1024);
   entry_count_ = 0;
   byte_count_ = 0;
   up_ticks_++;
diff --git a/net/disk_cache/backend_impl.h b/net/disk_cache/backend_impl.h
index c637791..a1624dd 100644
--- a/net/disk_cache/backend_impl.h
+++ b/net/disk_cache/backend_impl.h
@@ -377,7 +377,7 @@ class NET_EXPORT_PRIVATE BackendImpl : public Backend {
   bool disabled_;
   bool new_eviction_;  // What eviction algorithm should be used.
   bool first_timer_;  // True if the timer has not been called.
-  bool throttle_requests_;
+  bool user_load_;  // True if we see a high load coming from the caller.
 
   net::NetLog* net_log_;
 
diff --git a/net/disk_cache/eviction.cc b/net/disk_cache/eviction.cc
index fa0425d..07028ed 100644
--- a/net/disk_cache/eviction.cc
+++ b/net/disk_cache/eviction.cc
@@ -114,8 +114,9 @@ void Eviction::TrimCache(bool empty) {
   trimming_ = true;
   TimeTicks start = TimeTicks::Now();
   Rankings::ScopedRankingsBlock node(rankings_);
-  Rankings::ScopedRankingsBlock next(rankings_,
-      rankings_->GetPrev(node.get(), Rankings::NO_USE));
+  Rankings::ScopedRankingsBlock next(
+      rankings_, rankings_->GetPrev(node.get(), Rankings::NO_USE));
+  int deleted_entries = 0;
   int target_size = empty ? 0 : max_size_;
   while ((header_->num_bytes > target_size || test_mode_) && next.get()) {
     // The iterator could be invalidated within EvictEntry().
@@ -127,28 +128,26 @@ void Eviction::TrimCache(bool empty) {
       // This entry is not being used by anybody.
       // Do NOT use node as an iterator after this point.
       rankings_->TrackRankingsBlock(node.get(), false);
-      if (!EvictEntry(node.get(), empty, Rankings::NO_USE) && !test_mode_)
-        continue;
+      if (EvictEntry(node.get(), empty, Rankings::NO_USE) && !test_mode_)
+        deleted_entries++;
 
-      if (!empty) {
-        backend_->OnEvent(Stats::TRIM_ENTRY);
-        if (test_mode_)
-          break;
-
-        if ((TimeTicks::Now() - start).InMilliseconds() > 20) {
-          MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
-              &Eviction::TrimCache, ptr_factory_.GetWeakPtr(), false));
-          break;
-        }
-      }
+      if (!empty && test_mode_)
+        break;
+    }
+    if (!empty && (deleted_entries > 20 ||
+                   (TimeTicks::Now() - start).InMilliseconds() > 20)) {
+      MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
+          &Eviction::TrimCache, ptr_factory_.GetWeakPtr(), false));
+      break;
     }
   }
 
   if (empty) {
     CACHE_UMA(AGE_MS, "TotalClearTimeV1", 0, start);
   } else {
-    CACHE_UMA(AGE_MS, "TotalTrimTimeV1", backend_->GetSizeGroup(), start);
+    CACHE_UMA(AGE_MS, "TotalTrimTimeV1", 0, start);
   }
+  CACHE_UMA(COUNTS, "TrimItemsV1", 0, deleted_entries);
 
   trimming_ = false;
   Trace("*** Trim Cache end ***");
@@ -269,6 +268,8 @@ bool Eviction::EvictEntry(CacheRankingsBlock* node, bool empty,
   ReportTrimTimes(entry);
   if (empty || !new_eviction_) {
     entry->DoomImpl();
+    if (!empty)
+      backend_->OnEvent(Stats::TRIM_ENTRY);
   } else {
     entry->DeleteEntryData(false);
     EntryStore* info = entry->entry()->Data();
@@ -317,8 +318,9 @@ void Eviction::TrimCacheV2(bool empty) {
     list = 0;
 
   Rankings::ScopedRankingsBlock node(rankings_);
-
+  int deleted_entries = 0;
   int target_size = empty ? 0 : max_size_;
+
   for (; list < kListsToSearch; list++) {
     while ((header_->num_bytes > target_size || test_mode_) &&
            next[list].get()) {
@@ -332,18 +334,17 @@ void Eviction::TrimCacheV2(bool empty) {
         // This entry is not being used by anybody.
         // Do NOT use node as an iterator after this point.
         rankings_->TrackRankingsBlock(node.get(), false);
-        if (!EvictEntry(node.get(), empty, static_cast<Rankings::List>(list)) &&
-            !test_mode_)
-          continue;
+        if (EvictEntry(node.get(), empty, static_cast<Rankings::List>(list)))
+          deleted_entries++;
 
         if (!empty && test_mode_)
           break;
-
-        if (!empty && (TimeTicks::Now() - start).InMilliseconds() > 20) {
-          MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
-              &Eviction::TrimCache, ptr_factory_.GetWeakPtr(), false));
-          break;
-        }
+      }
+      if (!empty && (deleted_entries > 20 ||
+                     (TimeTicks::Now() - start).InMilliseconds() > 20)) {
+        MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
+            &Eviction::TrimCache, ptr_factory_.GetWeakPtr(), false));
+        break;
       }
     }
     if (!empty)
@@ -361,8 +362,9 @@ void Eviction::TrimCacheV2(bool empty) {
   if (empty) {
     CACHE_UMA(AGE_MS, "TotalClearTimeV2", 0, start);
   } else {
-    CACHE_UMA(AGE_MS, "TotalTrimTimeV2", backend_->GetSizeGroup(), start);
+    CACHE_UMA(AGE_MS, "TotalTrimTimeV2", 0, start);
   }
+  CACHE_UMA(COUNTS, "TrimItemsV2", 0, deleted_entries);
 
   Trace("*** Trim Cache end ***");
   trimming_ = false;
@@ -470,14 +472,16 @@ void Eviction::TrimDeleted(bool empty) {
 
   TimeTicks start = TimeTicks::Now();
   Rankings::ScopedRankingsBlock node(rankings_);
-  Rankings::ScopedRankingsBlock next(rankings_,
-    rankings_->GetPrev(node.get(), Rankings::DELETED));
-  bool deleted = false;
+  Rankings::ScopedRankingsBlock next(
+    rankings_, rankings_->GetPrev(node.get(), Rankings::DELETED));
+  int deleted_entries = 0;
   while (next.get() &&
-         (empty || (TimeTicks::Now() - start).InMilliseconds() < 20)) {
+         (empty || (deleted_entries < 20 &&
+                    (TimeTicks::Now() - start).InMilliseconds() < 20))) {
     node.reset(next.release());
     next.reset(rankings_->GetPrev(node.get(), Rankings::DELETED));
-    deleted |= RemoveDeletedNode(node.get());
+    if (RemoveDeletedNode(node.get()))
+      deleted_entries++;
     if (test_mode_)
       break;
   }
@@ -488,13 +492,14 @@ void Eviction::TrimDeleted(bool empty) {
   // lists intact.
   int max_length = in_experiment_ ? header_->num_entries * 2 / 5 :
                                     header_->num_entries / 4;
-  if (deleted && !empty && !test_mode_ &&
+  if (deleted_entries && !empty && !test_mode_ &&
       header_->lru.sizes[Rankings::DELETED] > max_length) {
     MessageLoop::current()->PostTask(FROM_HERE,
         base::Bind(&Eviction::TrimDeleted, ptr_factory_.GetWeakPtr(), false));
   }
 
   CACHE_UMA(AGE_MS, "TotalTrimDeletedTime", 0, start);
+  CACHE_UMA(COUNTS, "TrimDeletedItems", 0, deleted_entries);
   Trace("*** Trim Deleted end ***");
   return;
 }