net/disk_cache/simple: Reduce size of EntrySet nodes.

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..1ee7d5dd0fa29fd7f9729d3ebf770869eb0d9a3f 100644
--- a/net/disk_cache/simple/simple_index.cc
+++ b/net/disk_cache/simple/simple_index.cc
@@ -106,18 +106,11 @@ bool GetMTime(const base::FilePath& path, base::Time* out_mtime) {
 
 namespace disk_cache {
 
-EntryMetadata::EntryMetadata() : hash_key_(0),
-                                 last_used_time_(0),
-                                 entry_size_(0) {
-}
+EntryMetadata::EntryMetadata() : last_used_time_(0), entry_size_(0) {}
 
-EntryMetadata::EntryMetadata(uint64 hash_key,
-                             base::Time last_used_time,
-                             uint64 entry_size) :
-    hash_key_(hash_key),
-    last_used_time_(last_used_time.ToInternalValue()),
-    entry_size_(entry_size) {
-}
+EntryMetadata::EntryMetadata(base::Time last_used_time, uint64 entry_size)
+    : last_used_time_(last_used_time.ToInternalValue()),
+      entry_size_(entry_size) {}
 
 base::Time EntryMetadata::GetLastUsedTime() const {
   return base::Time::FromInternalValue(last_used_time_);
@@ -129,23 +122,18 @@ void EntryMetadata::SetLastUsedTime(const base::Time& last_used_time) {
 
 void EntryMetadata::Serialize(Pickle* pickle) const {
   DCHECK(pickle);
-  COMPILE_ASSERT(sizeof(EntryMetadata) ==
-                 (sizeof(uint64) + sizeof(int64) + sizeof(uint64)),
-                 EntryMetadata_has_three_member_variables);
-  pickle->WriteUInt64(hash_key_);
+  COMPILE_ASSERT(sizeof(EntryMetadata) == (sizeof(int64) + sizeof(uint64)),
+                 EntryMetadata_has_two_member_variables);
   pickle->WriteInt64(last_used_time_);
   pickle->WriteUInt64(entry_size_);
 }
 
 bool EntryMetadata::Deserialize(PickleIterator* it) {
   DCHECK(it);
-  return it->ReadUInt64(&hash_key_) &&
-      it->ReadInt64(&last_used_time_) &&
-      it->ReadUInt64(&entry_size_);
+  return it->ReadInt64(&last_used_time_) && it->ReadUInt64(&entry_size_);
 }
 
 void EntryMetadata::MergeWith(const EntryMetadata& from) {
-  DCHECK_EQ(hash_key_, from.hash_key_);
   if (last_used_time_ == 0)
     last_used_time_ = from.last_used_time_;
   if (entry_size_ == 0)
@@ -227,12 +215,11 @@ scoped_ptr<std::vector<uint64> > SimpleIndex::RemoveEntriesBetween(
   scoped_ptr<std::vector<uint64> > ret_hashes(new std::vector<uint64>());
   for (EntrySet::iterator it = entries_set_.begin(), end = entries_set_.end();
        it != end;) {
-    EntryMetadata metadata = it->second;

[pasko, 2013/05/16 18:13:21]

I would prefer to leave it with a named variable for the purpose of reading. If
you are concerned with copying, can make a const reference, although the
compiler should be able to eliminate the copying.

[digit1, 2013/05/17 08:45:38]

Sure, no problem. Done.

-    base::Time entry_time = metadata.GetLastUsedTime();
+    base::Time entry_time = it->second.GetLastUsedTime();
     if (initial_time <= entry_time && entry_time < extended_end_time) {
-      ret_hashes->push_back(metadata.GetHashKey());
+      ret_hashes->push_back(it->first);
+      cache_size_ -= it->second.GetEntrySize();
       entries_set_.erase(it++);
-      cache_size_ -= metadata.GetEntrySize();
     } else {
       it++;
     }
@@ -251,8 +238,8 @@ void SimpleIndex::Insert(const std::string& key) {
   // It will be updated later when the SimpleEntryImpl finishes opening or
   // creating the new entry, and then UpdateEntrySize will be called.
   const uint64 hash_key = simple_util::GetEntryHashKey(key);
-  InsertInEntrySet(EntryMetadata(hash_key, base::Time::Now(), 0),
-                   &entries_set_);
+  InsertInEntrySet(

[gavinp, 2013/05/16 13:38:01]

This is the only call site for this function now. Might as well elide it and
remove the method, I think?

[digit1, 2013/05/16 15:31:02]

It's a public function that is also called from simple_index_file.cc and a unit
test I believe.

+      hash_key, EntryMetadata(base::Time::Now(), 0), &entries_set_);
   if (!initialized_)
     removed_entries_.erase(hash_key);
   PostponeWritingToDisk();
@@ -302,7 +289,7 @@ void SimpleIndex::StartEvictionIfNeeded() {
   scoped_ptr<std::vector<uint64> > entry_hashes(new std::vector<uint64>());
   for (EntrySet::const_iterator it = entries_set_.begin(),
        end = entries_set_.end(); it != end; ++it) {
-    entry_hashes->push_back(it->second.GetHashKey());
+    entry_hashes->push_back(it->first);
   }
   std::sort(entry_hashes->begin(), entry_hashes->end(),
             CompareHashesForTimestamp(entries_set_));
@@ -368,11 +355,11 @@ void SimpleIndex::EvictionDone(scoped_ptr<int> result) {
 
 // static
 void SimpleIndex::InsertInEntrySet(
+    uint64 hash_key,
     const disk_cache::EntryMetadata& entry_metadata,
     EntrySet* entry_set) {
   DCHECK(entry_set);
-  entry_set->insert(
-      std::make_pair(entry_metadata.GetHashKey(), entry_metadata));
+  entry_set->insert(std::make_pair(hash_key, entry_metadata));
 }
 
 void SimpleIndex::PostponeWritingToDisk() {
@@ -512,13 +499,11 @@ scoped_ptr<SimpleIndex::EntrySet> SimpleIndex::RestoreFromDisk(
       last_used_time = FileEnumerator::GetLastModifiedTime(find_info);
 
     int64 file_size = FileEnumerator::GetFilesize(find_info);
-    EntrySet::iterator it = index_file_entries->find(hash_key);
-    if (it == index_file_entries->end()) {
-      InsertInEntrySet(EntryMetadata(hash_key, last_used_time, file_size),
-                       index_file_entries.get());
-    } else {
-      // Summing up the total size of the entry through all the *_[0-2] files
-      it->second.SetEntrySize(it->second.GetEntrySize() + file_size);
+    std::pair<EntrySet::iterator, bool> ret = index_file_entries->insert(
+        std::make_pair(hash_key, EntryMetadata(last_used_time, file_size)));
+    if (ret.second == false) {
+      EntryMetadata* current_entry = &ret.first->second;
+      current_entry->SetEntrySize(current_entry->GetEntrySize() + file_size);
     }
   }
   return index_file_entries.Pass();
@@ -557,16 +542,14 @@ void SimpleIndex::MergeInitializingSet(scoped_ptr<EntrySet> index_file_entries,
        it != index_file_entries->end(); ++it) {
     // If there is already an entry in the current entries_set_, we need to
     // merge the new data there with the data loaded in the initialization.

[pasko, 2013/05/16 18:13:21]

while we are here, please delete this comment, it is too obvious.

[digit1, 2013/05/17 08:46:59]

Done.

-    EntrySet::iterator current_entry = entries_set_.find(it->first);
-    if (current_entry != entries_set_.end()) {
+    std::pair<EntrySet::iterator, bool> ret = entries_set_.insert(*it);
+    EntryMetadata& current_entry = ret.first->second;
+    if (ret.second == false) {
       // When Merging, existing valid data in the |current_entry| will prevail.
-      cache_size_ -= current_entry->second.GetEntrySize();
-      current_entry->second.MergeWith(it->second);
-      cache_size_ += current_entry->second.GetEntrySize();
-    } else {
-      InsertInEntrySet(it->second, &entries_set_);
-      cache_size_ += it->second.GetEntrySize();
+      cache_size_ -= current_entry.GetEntrySize();
+      current_entry.MergeWith(it->second);
     }
+    cache_size_ += current_entry.GetEntrySize();
   }
   initialized_ = true;
   removed_entries_.clear();