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

net/disk_cache/simple/simple_index.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/disk_cache/simple/simple_index.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 88 matching lines @@
   if (!file_util::GetFileInfo(path, &file_info))
     return false;
   *out_mtime = file_info.last_modified;
   return true;
 }
 
 }  // namespace
 
 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_);
 }
 
 void EntryMetadata::SetLastUsedTime(const base::Time& last_used_time) {
   last_used_time_ = last_used_time.ToInternalValue();
 }
 
 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)
     entry_size_ = from.entry_size_;
 }
 
 SimpleIndex::SimpleIndex(base::SingleThreadTaskRunner* cache_thread,
                          base::SingleThreadTaskRunner* io_thread,
                          const base::FilePath& path)
     : cache_size_(0),
@@ skipping 61 matching lines @@
 
 scoped_ptr<std::vector<uint64> > SimpleIndex::RemoveEntriesBetween(
     const base::Time initial_time, const base::Time end_time) {
   DCHECK_EQ(true, initialized_);
   const base::Time extended_end_time =
       end_time.is_null() ? base::Time::Max() : end_time;
   DCHECK(extended_end_time >= initial_time);
   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++;
     }
   }
   return ret_hashes.Pass();
 }
 
 int32 SimpleIndex::GetEntryCount() const {
   // TODO(pasko): return a meaningful initial estimate before initialized.
   return entries_set_.size();
 }
 
 void SimpleIndex::Insert(const std::string& key) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
   // Upon insert we don't know yet the size of the entry.
   // 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();
 }
 
 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);
@@ skipping 29 matching lines @@
     return;
 
   // Take all live key hashes from the index and sort them by time.
   eviction_in_progress_ = true;
   eviction_start_time_ = base::TimeTicks::Now();
   UMA_HISTOGRAM_COUNTS("SimpleCache.Eviction.CacheSizeOnStart", cache_size_);
   UMA_HISTOGRAM_COUNTS("SimpleCache.Eviction.MaxCacheSizeOnStart", max_size_);
   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_));
 
   // Remove as many entries from the index to get below |low_watermark_|.
   std::vector<uint64>::iterator it = entry_hashes->begin();
   uint64 evicted_so_far_size = 0;
   while (evicted_so_far_size < cache_size_ - low_watermark_) {
     DCHECK(it != entry_hashes->end());
     EntrySet::iterator found_meta = entries_set_.find(*it);
@@ skipping 45 matching lines @@
   // Ignore the result of eviction. We did our best.
   eviction_in_progress_ = false;
   UMA_HISTOGRAM_BOOLEAN("SimpleCache.Eviction.Result", *result == net::OK);
   UMA_HISTOGRAM_TIMES("SimpleCache.Eviction.TimeToDone",
                       base::TimeTicks::Now() - eviction_start_time_);
   UMA_HISTOGRAM_COUNTS("SimpleCache.Eviction.SizeWhenDone", cache_size_);
 }
 
 // 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() {
   if (!initialized_)
     return;
   const int delay = app_on_background_ ? kWriteToDiskOnBackgroundDelayMSecs
                                        : kWriteToDiskDelayMSecs;
   // If the timer is already active, Start() will just Reset it, postponing it.
   write_to_disk_timer_.Start(
       FROM_HERE,
@@ skipping 119 matching lines @@
     base::Time last_used_time;
 #if defined(OS_POSIX)
     // For POSIX systems, a last access time is available. However, it's not
     // guaranteed to be more accurate than mtime. It is no worse though.
     last_used_time = base::Time::FromTimeT(find_info.stat.st_atime);
 #endif
     if (last_used_time.is_null())
       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();
 }
 
 
 // static
 void SimpleIndex::WriteToDiskInternal(const base::FilePath& index_filename,
                                       scoped_ptr<Pickle> pickle,
                                       const base::TimeTicks& start_time,
@@ skipping 17 matching lines @@
   for (base::hash_set<uint64>::const_iterator it =
            removed_entries_.begin(); it != removed_entries_.end(); ++it) {
     entries_set_.erase(*it);
     index_file_entries->erase(*it);
   }
 
   // Recalculate the cache size while merging the two sets.
   for (EntrySet::const_iterator it = index_file_entries->begin();
        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();
 
   // The actual IO is asynchronous, so calling WriteToDisk() shouldn't slow down
   // much the merge.
   if (force_index_flush)
     WriteToDisk();
 
   UMA_HISTOGRAM_CUSTOM_COUNTS("SimpleCache.IndexInitializationWaiters",
@@ skipping 46 matching lines @@
                                                          entries_set_);
   cache_thread_->PostTask(FROM_HERE, base::Bind(
       &SimpleIndex::WriteToDiskInternal,
       index_filename_,
       base::Passed(&pickle),
       start,
       app_on_background_));
 }
 
 }  // namespace disk_cache