Minor SimpleCacheBackend improvements.

net/disk_cache/simple/simple_index.cc

Index: net/disk_cache/simple/simple_index.cc
diff --git a/net/disk_cache/simple/simple_index.cc b/net/disk_cache/simple/simple_index.cc
index e0f927b782b96ecaf2e32bb8b9f8724546c17332..3c3a286cfc0ff7ace0d663db73e8355ae5325a55 100644
--- a/net/disk_cache/simple/simple_index.cc
+++ b/net/disk_cache/simple/simple_index.cc
@@ -165,6 +165,9 @@ SimpleIndex::SimpleIndex(base::SingleThreadTaskRunner* cache_thread,
       cache_thread_(cache_thread),
       io_thread_(io_thread),
       app_on_background_(false) {
+  // Creating the callback once so it is reused every time
+  // write_to_disk_timer_.Start() is called.
+  write_to_disk_cb_ = base::Bind(&SimpleIndex::WriteToDisk, AsWeakPtr());

[gavinp, 2013/05/21 19:59:21]

Why not place this in the ctor list?

[digit1, 2013/05/30 13:17:58]

Done.

 }
 
 SimpleIndex::~SimpleIndex() {
@@ -260,9 +263,12 @@ void SimpleIndex::Insert(const std::string& key) {
 
 void SimpleIndex::Remove(const std::string& key) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
-  UpdateEntrySize(key, 0);
   const uint64 hash_key = simple_util::GetEntryHashKey(key);
-  entries_set_.erase(hash_key);
+  EntrySet::iterator it = entries_set_.find(hash_key);
+  if (it != entries_set_.end()) {
+    UpdateEntryIteratorSize(&it, 0);
+    entries_set_.erase(it);
+  }
 
   if (!initialized_)
     removed_entries_.insert(hash_key);
@@ -273,7 +279,8 @@ bool SimpleIndex::Has(const std::string& key) const {
   DCHECK(io_thread_checker_.CalledOnValidThread());
   // If not initialized, always return true, forcing it to go to the disk.
   return !initialized_ ||
-      entries_set_.count(simple_util::GetEntryHashKey(key)) != 0;
+         entries_set_.find(simple_util::GetEntryHashKey(key)) !=

[gavinp, 2013/05/21 19:59:21]

 I don't think this is an improvement. count() is a lot easier to read than
find.

[digit1, 2013/05/30 13:17:58]

count() is actually slowed than find() != end() with STLport, but I agree the
difference should be minimal, so I've reverted this.

+             entries_set_.end();
 }
 
 bool SimpleIndex::UseIfExists(const std::string& key) {
@@ -344,11 +351,7 @@ bool SimpleIndex::UpdateEntrySize(const std::string& key, uint64 entry_size) {
   if (it == entries_set_.end())
     return false;
 
-  // Update the total cache size with the new entry size.
-  DCHECK(cache_size_ - it->second.GetEntrySize() <= cache_size_);
-  cache_size_ -= it->second.GetEntrySize();
-  cache_size_ += entry_size;
-  it->second.SetEntrySize(entry_size);
+  UpdateEntryIteratorSize(&it, entry_size);
   PostponeWritingToDisk();
   StartEvictionIfNeeded();
   return true;
@@ -382,9 +385,16 @@ void SimpleIndex::PostponeWritingToDisk() {
                                        : kWriteToDiskDelayMSecs;
   // If the timer is already active, Start() will just Reset it, postponing it.
   write_to_disk_timer_.Start(
-      FROM_HERE,
-      base::TimeDelta::FromMilliseconds(delay),
-      base::Bind(&SimpleIndex::WriteToDisk, AsWeakPtr()));
+      FROM_HERE, base::TimeDelta::FromMilliseconds(delay), write_to_disk_cb_);
+}
+
+void SimpleIndex::UpdateEntryIteratorSize(EntrySet::iterator* it,
+                                          uint64 entry_size) {

[gavinp, 2013/05/21 19:59:21]

Please add a thread checker.

[digit1, 2013/05/30 13:17:58]

Done.

+  // Update the total cache size with the new entry size.
+  DCHECK(cache_size_ - (*it)->second.GetEntrySize() <= cache_size_);

[gavinp, 2013/05/21 19:59:21]

DCHECK_GE(cache_size, ...);

[digit1, 2013/05/30 13:17:58]

Done.

+  cache_size_ -= (*it)->second.GetEntrySize();
+  cache_size_ += entry_size;
+  (*it)->second.SetEntrySize(entry_size);
 }
 
 // static
@@ -477,7 +487,7 @@ scoped_ptr<SimpleIndex::EntrySet> SimpleIndex::RestoreFromDisk(
   COMPILE_ASSERT(kSimpleEntryFileCount == 3,
                  file_pattern_must_match_file_count);
 
-  const int kFileSuffixLenght = std::string("_0").size();
+  const int kFileSuffixLength = strlen("_0");
   const base::FilePath::StringType file_pattern = FILE_PATH_LITERAL("*_[0-2]");
   FileEnumerator enumerator(index_filename.DirName(),
                             false /* recursive */,
@@ -488,14 +498,13 @@ scoped_ptr<SimpleIndex::EntrySet> SimpleIndex::RestoreFromDisk(
     const base::FilePath::StringType base_name = file_path.BaseName().value();
     // Converting to std::string is OK since we never use UTF8 wide chars in our
     // file names.
-    const std::string hash_name(base_name.begin(), base_name.end());
-    const std::string hash_key_string =
-        hash_name.substr(0, hash_name.size() - kFileSuffixLenght);
+    const std::string hash_key_string(base_name.begin(),
+                                      base_name.end() - kFileSuffixLength);
     uint64 hash_key = 0;
     if (!simple_util::GetEntryHashKeyFromHexString(
             hash_key_string, &hash_key)) {
       LOG(WARNING) << "Invalid Entry Hash Key filename while restoring "
-                   << "Simple Index from disk: " << hash_name;
+                   << "Simple Index from disk: " << base_name;
       // TODO(felipeg): Should we delete the invalid file here ?
       continue;
     }