Write the Simple Cache Index file to disk once in a while

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 c87b3c8a7019ca72061ae9e8d34686006ac6164b..04ff8de0808ecc86a1a121e2d7cc76ff9ebb87cb 100644
--- a/net/disk_cache/simple/simple_index.cc
+++ b/net/disk_cache/simple/simple_index.cc
@@ -19,6 +19,14 @@
 #include "net/disk_cache/simple/simple_index_file.h"
 #include "net/disk_cache/simple/simple_util.h"
 
+namespace {
+
+// How many seconds we delay writing the index to disk since the last cache
+// operation has happened.
+const int kWriteToDiskDelaySecs = 20;
+
+}  // namespace
+
 namespace disk_cache {
 
 EntryMetadata::EntryMetadata() :
@@ -27,7 +35,6 @@ EntryMetadata::EntryMetadata() :
     entry_size_(0)
 {}
 
-
 EntryMetadata::EntryMetadata(uint64 hash_key,
                              base::Time last_used_time,
                              uint64 entry_size) :
@@ -77,7 +84,8 @@ SimpleIndex::SimpleIndex(
       initialized_(false),
       index_filename_(path.AppendASCII("simple-index")),
       cache_thread_(cache_thread),
-      io_thread_(io_thread) {}
+      io_thread_(io_thread)

[gavinp, 2013/04/17 07:54:41]

I like:

      io_thread_(io_thread) {
}

best of all the options!

[Philippe, 2013/04/17 08:31:39]

Yes, this is the recommended way.

[felipeg, 2013/04/17 15:01:48]

Dude, in another CL someone told me to put {} in the next line.
Anyway I also like it more as you said.

Done.

[pasko-google - do not use, 2013/04/17 15:38:16]

The style guide says: "The close curly brace is either on the last line by
itself or (if other style rules permit) on the same line as the open curly
brace."

The one on lines 36 and 44 are wrong since: "The open curly brace is always at
the end of the same line as the last parameter."

[Philippe, 2013/04/17 15:39:18]

In the other CL I told you to put '}' on the next line, not '{}' :)

+{}
 
 SimpleIndex::~SimpleIndex() {
   DCHECK(io_thread_checker_.CalledOnValidThread());
@@ -106,6 +114,7 @@ void SimpleIndex::Insert(const std::string& key) {
                    &entries_set_);
   if (!initialized_)
     removed_entries_.erase(hash_key);
+  PostponeWriteToDisk();
 }
 
 void SimpleIndex::Remove(const std::string& key) {
@@ -116,6 +125,7 @@ void SimpleIndex::Remove(const std::string& key) {
 
   if (!initialized_)
     removed_entries_.insert(hash_key);
+  PostponeWriteToDisk();
 }
 
 bool SimpleIndex::Has(const std::string& key) const {
@@ -134,6 +144,7 @@ bool SimpleIndex::UseIfExists(const std::string& key) {
     // If not initialized, always return true, forcing it to go to the disk.
     return !initialized_;
   it->second.SetLastUsedTime(base::Time::Now());
+  PostponeWriteToDisk();
   return true;
 }
 
@@ -147,7 +158,7 @@ bool SimpleIndex::UpdateEntrySize(const std::string& key, uint64 entry_size) {
   cache_size_ -= it->second.GetEntrySize();
   cache_size_ += entry_size;
   it->second.SetEntrySize(entry_size);
-
+  PostponeWriteToDisk();
   return true;
 }
 
@@ -160,6 +171,14 @@ void SimpleIndex::InsertInEntrySet(
       std::make_pair(entry_metadata.GetHashKey(), entry_metadata));
 }
 
+void SimpleIndex::PostponeWriteToDisk() {
+  // If the timer is already active, Start() will just Reset it, postponing it.
+  write_to_disk_timer_.Start(
+      FROM_HERE,
+      base::TimeDelta::FromSeconds(kWriteToDiskDelaySecs),
+      base::Bind(&SimpleIndex::WriteToDisk, AsWeakPtr()));
+}
+
 // static
 void SimpleIndex::LoadFromDisk(
     const base::FilePath& index_filename,
@@ -168,12 +187,18 @@ void SimpleIndex::LoadFromDisk(
   scoped_ptr<EntrySet> index_file_entries =
       SimpleIndexFile::LoadFromDisk(index_filename);
 
-  if (!index_file_entries.get())
-      index_file_entries = SimpleIndex::RestoreFromDisk(index_filename);
+  bool must_write_to_disk = false;
+  if (!index_file_entries.get()) {
+    index_file_entries = SimpleIndex::RestoreFromDisk(index_filename);
+    // When we restore from disk we write the merged index file to disk right
+    // away, this might save us from having to restore again next time.
+    must_write_to_disk = true;
+  }
 
   io_thread->PostTask(FROM_HERE,
                       base::Bind(completion_callback,
-                                 base::Passed(&index_file_entries)));
+                                 base::Passed(&index_file_entries),
+                                 must_write_to_disk));
 }
 
 // static
@@ -243,8 +268,8 @@ void SimpleIndex::WriteToDiskInternal(const base::FilePath& index_filename,
   SimpleIndexFile::WriteToDisk(index_filename, *pickle);
 }
 
-void SimpleIndex::MergeInitializingSet(
-    scoped_ptr<EntrySet> index_file_entries) {
+void SimpleIndex::MergeInitializingSet(scoped_ptr<EntrySet> index_file_entries,
+                                       bool must_write_to_disk) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
   // First, remove the entries that are in the |removed_entries_| from both
   // sets.
@@ -270,12 +295,19 @@ void SimpleIndex::MergeInitializingSet(
       cache_size_ += it->second.GetEntrySize();
     }
   }
-
   initialized_ = true;
+  removed_entries_.clear();

[Philippe, 2013/04/17 08:31:39]

Is this line fixing another bug (which would be covered by a unit test :)?

[felipeg, 2013/04/17 15:01:48]

Nope. This is not a bug, it is just that we don't need to keep the removed
entries after we used it. It is just a waste.

[Philippe, 2013/04/17 15:39:18]

Great, thanks.

+
+  // The actual IO is asynchronous, so calling WriteToDisk() shouldn't slow down

[gavinp, 2013/04/17 07:54:41]

I'd rather rename the method to: "PostWriteToDisk" than have this comment.

[felipeg, 2013/04/17 15:01:48]

Done.

+  // much the merge.
+  if (must_write_to_disk)
+    WriteToDisk();
 }
 
 void SimpleIndex::WriteToDisk() {
   DCHECK(io_thread_checker_.CalledOnValidThread());
+  if (!initialized_)
+    return;
   SimpleIndexFile::IndexMetadata index_metadata(entries_set_.size(),
                                                 cache_size_);
   scoped_ptr<Pickle> pickle = SimpleIndexFile::Serialize(index_metadata,