UrlPatternIndex: Lazily compute rule count for UrlPatternIndexMatcher.

IndexedRuleset.median_match_time_avg/MatchAll perftest shows a
regression after r759462 due to computation of rule count in the
UrlPatternIndexMatcher constructor. This CL changes
UrlPatternIndexMatcher rule count to be lazily computed which should
ensure that the cost of computation is only paid by clients which use
the rule count.

When the perf test is run locally on my machine:

Without this CL: *RESULT IndexedRuleset.median_match_time: MatchAll=
231364 us
With this CL: *RESULT IndexedRuleset.median_match_time:
MatchAll= 80248 us

Note that the test regression is exaggerated since the test creates a
UrlPatternIndexMatcher for each request.

BUG=1071599

Change-Id: I6364e4b9043494d18389a41c517b90756ba4c8f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2153693
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Auto-Submit: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759905}

components/url_pattern_index/url_pattern_index.cc

@@ -114,23 +114,6 @@ int GetKeysMask(const T& map) {
   return mask;
 }
 
-size_t GetRulesCount(const flat::UrlPatternIndex* index) {
-  if (!index)
-    return 0;
-
-  size_t rules_count = index->fallback_rules()->size();
-
-  // Iterate over all ngrams and check their corresponding rules.
-  for (auto* ngram_to_rules : *index->ngram_index()) {
-    if (ngram_to_rules == index->ngram_index_empty_slot())
-      continue;
-
-    rules_count += ngram_to_rules->rule_list()->size();
-  }
-
-  return rules_count;
-}
-
 // Checks whether a URL |rule| can be converted to its FlatBuffers equivalent,
 // and performs the actual conversion.
 class UrlRuleFlatBufferConverter {
@@ -779,7 +762,7 @@ bool DoesRuleFlagsMatch(const flat::UrlRule& rule,
 
 UrlPatternIndexMatcher::UrlPatternIndexMatcher(
     const flat::UrlPatternIndex* flat_index)
-    : flat_index_(flat_index), rules_count_(GetRulesCount(flat_index)) {
+    : flat_index_(flat_index) {
   DCHECK(!flat_index || flat_index->n() == kNGramSize);
 }
 
@@ -789,6 +772,28 @@ UrlPatternIndexMatcher::UrlPatternIndexMatcher(UrlPatternIndexMatcher&&) =
 UrlPatternIndexMatcher& UrlPatternIndexMatcher::operator=(
     UrlPatternIndexMatcher&&) = default;
 
+size_t UrlPatternIndexMatcher::GetRulesCount() const {
+  if (rules_count_)
+    return *rules_count_;
+
+  if (!flat_index_) {
+    rules_count_ = 0;
+    return 0;
+  }
+
+  rules_count_ = flat_index_->fallback_rules()->size();
+
+  // Iterate over all ngrams and check their corresponding rules.
+  for (auto* ngram_to_rules : *flat_index_->ngram_index()) {
+    if (ngram_to_rules == flat_index_->ngram_index_empty_slot())
+      continue;
+
+    *rules_count_ += ngram_to_rules->rule_list()->size();
+  }
+
+  return *rules_count_;
+}
+
 const flat::UrlRule* UrlPatternIndexMatcher::FindMatch(
     const GURL& url,
     const url::Origin& first_party_origin,

components/url_pattern_index/url_pattern_index.h

@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/strings/string_piece_forward.h"
 #include "components/url_pattern_index/closed_hash_map.h"
 #include "components/url_pattern_index/flat/url_pattern_index_generated.h"
@@ -175,8 +176,9 @@ class UrlPatternIndexMatcher {
   UrlPatternIndexMatcher(UrlPatternIndexMatcher&&);
   UrlPatternIndexMatcher& operator=(UrlPatternIndexMatcher&&);
 
-  // Returns the number of rules in this index.
-  size_t rules_count() const { return rules_count_; }
+  // Returns the number of rules in this index. Lazily computed, the first call
+  // to this method will scan the entire index.
+  size_t GetRulesCount() const;
 
   // If the index contains one or more UrlRules that match the request, returns
   // one of them, depending on the |strategy|. Otherwise, returns nullptr.
@@ -244,7 +246,8 @@ class UrlPatternIndexMatcher {
   // Must outlive this instance.
   const flat::UrlPatternIndex* flat_index_;
 
-  size_t rules_count_ = 0;
+  // The number of rules in this index. Mutable since this is lazily computed.
+  mutable base::Optional<size_t> rules_count_;
 
   DISALLOW_COPY_AND_ASSIGN(UrlPatternIndexMatcher);
 };

components/url_pattern_index/url_pattern_index_unittest.cc

@@ -78,7 +78,7 @@ class UrlPatternIndexTest : public ::testing::Test {
         flat::GetUrlPatternIndex(flat_builder_->GetBufferPointer());
     index_matcher_.reset(new UrlPatternIndexMatcher(flat_index));
 
-    ASSERT_EQ(indexed_rules_count_, index_matcher_->rules_count());
+    ASSERT_EQ(indexed_rules_count_, index_matcher_->GetRulesCount());
   }
 
   const flat::UrlRule* FindMatch(

extensions/browser/api/declarative_net_request/extension_url_pattern_index_matcher.cc

@@ -53,7 +53,7 @@ bool IsExtraHeadersMatcherInternal(
   };
 
   for (flat::IndexType index : extra_header_indices) {
-    if (matchers[index].rules_count() > 0)
+    if (matchers[index].GetRulesCount() > 0)
       return true;
   }
 
@@ -64,7 +64,7 @@ size_t GetRulesCountInternal(
     const std::vector<url_pattern_index::UrlPatternIndexMatcher>& matchers) {
   size_t rules_count = 0;
   for (const auto& matcher : matchers)
-    rules_count += matcher.rules_count();
+    rules_count += matcher.GetRulesCount();
 
   return rules_count;
 }