Minor SimpleCacheBackend improvements.

net/disk_cache/simple/simple_backend_impl.h

 // 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.
 
 #ifndef NET_DISK_CACHE_SIMPLE_SIMPLE_BACKEND_IMPL_H_
 #define NET_DISK_CACHE_SIMPLE_SIMPLE_BACKEND_IMPL_H_
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
+#include "base/hash_tables.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/task_runner.h"
 #include "net/base/cache_type.h"
 #include "net/disk_cache/disk_cache.h"
 
 namespace base {
 class SingleThreadTaskRunner;
 }
@@ skipping 52 matching lines @@
   virtual int DoomEntriesSince(base::Time initial_time,
                                const CompletionCallback& callback) OVERRIDE;
   virtual int OpenNextEntry(void** iter, Entry** next_entry,
                             const CompletionCallback& callback) OVERRIDE;
   virtual void EndEnumeration(void** iter) OVERRIDE;
   virtual void GetStats(
       std::vector<std::pair<std::string, std::string> >* stats) OVERRIDE;
   virtual void OnExternalCacheHit(const std::string& key) OVERRIDE;
 
  private:
-  typedef std::map<uint64, base::WeakPtr<SimpleEntryImpl> > EntryMap;
+  typedef base::hash_map<uint64, base::WeakPtr<SimpleEntryImpl> > EntryMap;

[pasko, 2013/05/17 10:31:33]

This change cannot make any significant improvement: the amount of active
entries is not more than 300 at all times. Even worse, it can degrade
performance if a 'malicious' website attacks our hash function making buckets
for google.com main resource very lengthy, for example.

I would rather leave it as is.

[digit1, 2013/05/17 14:36:18]

I feel that these statements are contradicting each other :)

If there are no more than 300 entries at all times, then a degenerate map will
result in a linked list with 300 items in it. Even a mobile CPU should be able
to scan such a list and perform 64-bit comparisons in each node in a _very_
reasonable amount of time, so the effect of such denial of service would
probably not be noticeable.

Moreover, I though we were using a cryptographic hash like SHA-1 to make this
DOS attack extremely difficult to pull. I.e. I don't think there is an easy way
to produce hundreds or even thousands of input strings that all hash to the same
SHA-1. Even if there was, this would require several hundreds / thousands
concurrent active requests, right?

If you still think a std::map() is better, I propose adding a comment on top of
this typedef explaining why this container type is preferred.

[pasko, 2013/05/17 16:57:19]

You are right, there is practically no difference. Keep it whatever you like,
just remove "include <map>" if you decide to stay with hash_map.

 
   typedef base::Callback<void(uint64 max_size, int result)>
       InitializeIndexCallback;
 
   // Must run on IO Thread.
   void InitializeIndex(const CompletionCallback& callback,
                        uint64 suggested_max_size,
                        int result);
 
   // Dooms all entries previously accessed between |initial_time| and
@@ skipping 23 matching lines @@
   int orig_max_size_;
 
   // TODO(gavinp): Store the entry_hash in SimpleEntryImpl, and index this map
   // by hash. This will save memory, and make IndexReadyForDoom easier.
   EntryMap active_entries_;
 };
 
 }  // namespace disk_cache
 
 #endif  // NET_DISK_CACHE_SIMPLE_SIMPLE_BACKEND_IMPL_H_