Index initialization improvements for Simple Cache.

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 341 matching lines @@
 
   SimpleIndex::EntrySet* index_file_entries = &load_result->entries;
   // First, remove the entries that are in the |removed_entries_| from both
   // sets.
   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();
+  for (EntrySet::const_iterator it = entries_set_.begin();
+       it != entries_set_.end(); ++it) {
+    const uint64 entry_hash = it->first;
+    std::pair<EntrySet::iterator, bool> insert_result =
+        index_file_entries->insert(EntrySet::value_type(entry_hash,
+                                                        EntryMetadata()));
+    EntrySet::iterator& possibly_inserted_entry = insert_result.first;
+    possibly_inserted_entry->second = it->second;

[Randy Smith (Not in Mondays), 2013/08/12 17:02:20]

Suggestion: Why not test insert_result.second to decide whether to insert?  I
sorta assume it's in cache anyway since we're touching .first, and it'd save a
24 byte copy.  I don't see a reason to believe that that case would be
particularly common or uncommon.

(I believe your judgement will be better than mine as to whether this is a good
idea, thus "suggestion".)

[gavinp, 2013/08/13 11:11:05]

I think it's just a copy of EntrySet::data_type you're talking about saving, not
EntrySet::value_type, so it's 16 bytes, not 24. But that's a minor detail.

I originally coded it this way you mentioned. The constructor EntryMetadata() on
line 368 had to be replaced with |it->second|, and then recapitulated in the
case that insert_result.second is false; that repetition seemed a bit confusing,
so I went with the less conditional path.

I'm pretty sure, but I didn't check by compiling, that data flow analysis should
be able to toss out the redundant call to EntryMetadata(), making this just as
efficient. The constructor to EntryMetadata is inline able in this context. The
STL authors are actually fairly careful about this, because this code is the
same as:

  index_file_entries[entry_hash] = it->second;

And now that I think about it, perhaps we should replace lihnes 368-370 with
just that. I'll upload that CL later today.

+  }
+
+  uint64 merged_cache_size = 0;
+  for (EntrySet::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.
-    EntrySet::iterator current_entry = entries_set_.find(it->first);
-    // When Merging, existing data in the |current_entry| will prevail.
-    if (current_entry == entries_set_.end()) {
-      InsertInEntrySet(it->first, it->second, &entries_set_);
-      cache_size_ += it->second.GetEntrySize();
-    }
+    merged_cache_size += it->second.GetEntrySize();
   }
+
+  entries_set_.swap(*index_file_entries);
+  cache_size_ = merged_cache_size;
   initialized_ = true;
   removed_entries_.clear();
 
   // The actual IO is asynchronous, so calling WriteToDisk() shouldn't slow the
   // merge down much.
   if (load_result->flush_required)
     WriteToDisk();
 
   UMA_HISTOGRAM_CUSTOM_COUNTS("SimpleCache.IndexInitializationWaiters",
                               to_run_when_initialized_.size(), 0, 100, 20);
@@ skipping 62 matching lines @@
         entries_set_.erase(it++);
         continue;
       }
     }
     ++it;
   }
   return ret_hashes.Pass();
 }
 
 }  // namespace disk_cache