[safe-browsing] Make the perftest less allocate-y

This CL makes the V4 Store perftest a bit faster by:
1. Using a smaller # of hashes in debug builds
2. Using one big string instead of a vector of full hashes, to avoid
   allocating and de-allocating many small strings. To get this working,
   needed to slightly modify V4Store to allow querying by StringPiece.

(2) improved local performance on my release build by 25-30%

Bug: 822624
Change-Id: I646af2b0ccfb6447ccbb94ce68b661bd235587d1
Reviewed-on: https://chromium-review.googlesource.com/965931
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543796}

components/safe_browsing/db/v4_store.cc

@@ -777,6 +777,10 @@ StoreWriteResult V4Store::WriteToDisk(const Checksum& checksum) {
 }
 
 HashPrefix V4Store::GetMatchingHashPrefix(const FullHash& full_hash) {
+  return GetMatchingHashPrefix(base::StringPiece(full_hash));
+}
+
+HashPrefix V4Store::GetMatchingHashPrefix(base::StringPiece full_hash) {
   // It should never be the case that more than one hash prefixes match a given
   // full hash. However, if that happens, this method returns any one of them.
   // It does not guarantee which one of those will be returned.
@@ -784,8 +788,7 @@ HashPrefix V4Store::GetMatchingHashPrefix(const FullHash& full_hash) {
   checks_attempted_++;
   for (const auto& pair : hash_prefix_map_) {
     const PrefixSize& prefix_size = pair.first;
-    base::StringPiece hash_prefix =
-        base::StringPiece(full_hash).substr(0, prefix_size);
+    base::StringPiece hash_prefix = full_hash.substr(0, prefix_size);
     if (HashPrefixMatches(hash_prefix, pair.second, prefix_size))
       return hash_prefix.as_string();
   }

components/safe_browsing/db/v4_store.h

@@ -293,6 +293,8 @@ class V4Store {
   FRIEND_TEST_ALL_PREFIXES(V4StoreTest, TestChecksumErrorOnStartup);
   FRIEND_TEST_ALL_PREFIXES(V4StoreTest, WriteToDiskFails);
   FRIEND_TEST_ALL_PREFIXES(V4StoreTest, FullUpdateFailsChecksumSynchronously);
+  FRIEND_TEST_ALL_PREFIXES(V4StorePerftest, StressTest);
+
   friend class V4StoreTest;
 
   // If |prefix_size| is within expected range, and |raw_hashes_length| is a
@@ -338,6 +340,10 @@ class V4Store {
   static void ReserveSpaceInPrefixMap(const HashPrefixMap& other_prefixes_map,
                                       HashPrefixMap* prefix_map_to_update);
 
+  // Same as the public GetMatchingHashPrefix method, but takes a StringPiece,
+  // for performance reasons.
+  HashPrefix GetMatchingHashPrefix(base::StringPiece full_hash);
+
   // Merges the prefix map from the old store (|old_hash_prefix_map|) and the
   // update (additions_map) to populate the prefix map for the current store.
   // The indices in the |raw_removals| list, which may be NULL, are not merged.

components/safe_browsing/db/v4_store_perftest.cc

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/numerics/checked_math.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/timer/elapsed_timer.h"
@@ -23,14 +24,28 @@ namespace safe_browsing {
 class V4StorePerftest : public testing::Test {};
 
 TEST_F(V4StorePerftest, StressTest) {
-  const int kNumPrefixes = 2000000;
+// Debug builds can be quite slow. Use a smaller number of prefixes to test.
+#if defined(NDEBUG)
+  const size_t kNumPrefixes = 2000000;
+#else
+  const size_t kNumPrefixes = 20000;
+#endif
+
+  static_assert(kMaxHashPrefixLength == crypto::kSHA256Length,
+                "SHA256 produces a valid FullHash");
+  CHECK(base::IsValidForType<size_t>(
+      base::CheckMul(kNumPrefixes, kMaxHashPrefixLength)));
+
+  // Keep the full hashes as one big string to avoid tons of allocations /
+  // deallocations in the test.
+  std::string full_hashes(kNumPrefixes * kMaxHashPrefixLength, 0);
+  base::StringPiece full_hashes_piece = base::StringPiece(full_hashes);
   std::vector<std::string> prefixes;
-  std::vector<std::string> full_hashes;
   for (size_t i = 0; i < kNumPrefixes; i++) {
-    std::string sha256 = crypto::SHA256HashString(base::StringPrintf("%zu", i));
-    DCHECK_EQ(crypto::kSHA256Length, kMaxHashPrefixLength);
-    full_hashes.push_back(sha256);
-    prefixes.push_back(sha256.substr(0, kMinHashPrefixLength));
+    size_t index = i * kMaxHashPrefixLength;
+    crypto::SHA256HashString(base::StringPrintf("%zu", i), &full_hashes[index],
+                             kMaxHashPrefixLength);
+    prefixes.push_back(full_hashes.substr(index, kMinHashPrefixLength));
   }
 
   auto store = std::make_unique<TestV4Store>(
@@ -39,13 +54,16 @@ TEST_F(V4StorePerftest, StressTest) {
 
   size_t matches = 0;
   base::ElapsedTimer timer;
-  for (const auto& full_hash : full_hashes) {
+  for (size_t i = 0; i < kNumPrefixes; i++) {
+    size_t index = i * kMaxHashPrefixLength;
+    base::StringPiece full_hash =
+        full_hashes_piece.substr(index, kMaxHashPrefixLength);
     matches += !store->GetMatchingHashPrefix(full_hash).empty();
   }
-  perf_test::PrintResult("GetMachingHashPrefix", "", "",
+  perf_test::PrintResult("GetMatchingHashPrefix", "", "",
                          timer.Elapsed().InMillisecondsF(), "ms", true);
 
-  EXPECT_EQ(matches, full_hashes.size());
+  EXPECT_EQ(kNumPrefixes, matches);
 }
 
 }  // namespace safe_browsing