stats: optimize stats-matchers instantiated with "prefix." pattern, to avoid having to do toString() mapping and caching

envoy/stats/stats_matcher.h

@@ -9,16 +9,58 @@
 namespace Envoy {
 namespace Stats {
 
+class StatName;
+
 class StatsMatcher {
 public:
+  // Holds the result of fastRejects(). This contains state that must be then
+  // passed to slowRejects() for the final 2-phase determination of whether a
+  // stat can be rejected.
+  enum class FastResult {
+    NoMatch,
+    Rejects,
+    Matches,
+  };
+
   virtual ~StatsMatcher() = default;
 
   /**
    * Take a metric name and report whether or not it should be instantiated.
-   * @param the name of a Stats::Metric.
+   * This may need to convert the StatName to a string. This is equivalent to
+   * calling fastRejects() and then calling slowResults() if necessary.
+   *
+   * @param name the name of a Stats::Metric.
+   * @return bool true if that stat should not be instantiated.
+   */
+  virtual bool rejects(StatName name) const PURE;
+
+  /**
+   * Takes a metric name and quickly determines whether it can be rejected based
+   * purely on the StatName. If fastResults(stat_name).rejects() is 'false', we
+   * will need to check slowRejects as well. It should not be necessary to cache
+   * the result of fastRejects() -- it's cheap enough to recompute. However we
+   * should protect slowRejects() by a cache due to its speed and the potential
+   * need to take a symbol table lock.
+   *
+   * @param name the name of a Stats::Metric.
+   * @return A result indicating whether the stat can be quickly rejected, as
+   *         well as state that is then passed to slowRejects if rejection
+   *         cannot be quickly determined.
+   */
+  virtual FastResult fastRejects(StatName name) const PURE;
+
+  /**
+   * Takes a metric name and converts it to a string, if needed, to determine
+   * whether it needs to be rejected. This is intended to be used if
+   * fastRejects() cannot determine an early rejection. It is a good idea to
+   * cache the results of this, to avoid the stringification overhead, potential
+   * regex overhead, plus a global symbol table lock.
+   *
+   * @param fast_result the result of fastRejects(), which must be called first.
+   * @param name the name of a Stats::Metric.
    * @return bool true if that stat should not be instantiated.
    */
-  virtual bool rejects(const std::string& name) const PURE;
+  virtual bool slowRejects(FastResult fast_result, StatName name) const PURE;
 
   /**
    * Helps determine whether the matcher needs to be called. This can be used

source/common/common/matchers.cc

@@ -112,6 +112,16 @@ bool StringMatcherImpl::match(const absl::string_view value) const {
   }
 }
 
+bool StringMatcherImpl::getCaseSensitivePrefixMatch(std::string& prefix) const {
+  if (matcher_.match_pattern_case() ==
+          envoy::type::matcher::v3::StringMatcher::MatchPatternCase::kPrefix &&
+      !matcher_.ignore_case()) {
+    prefix = matcher_.prefix();
+    return true;
+  }
+  return false;
+}
+
 ListMatcher::ListMatcher(const envoy::type::matcher::v3::ListMatcher& matcher) : matcher_(matcher) {
   ASSERT(matcher_.match_pattern_case() ==
          envoy::type::matcher::v3::ListMatcher::MatchPatternCase::kOneOf);

source/common/common/matchers.h

@@ -85,11 +85,21 @@ class StringMatcherImpl : public ValueMatcher, public StringMatcher {
 public:
   explicit StringMatcherImpl(const envoy::type::matcher::v3::StringMatcher& matcher);
 
+  // StringMatcher
   bool match(const absl::string_view value) const override;
   bool match(const ProtobufWkt::Value& value) const override;
 
   const envoy::type::matcher::v3::StringMatcher& matcher() const { return matcher_; }
 
+  /**
+   * Helps applications optimize the case where a matcher is a case-sensitive
+   * prefix-match.
+   *
+   * @param prefix the returned prefix string
+   * @return true if the matcher is a case-sensitive prefix-match.
+   */
+  bool getCaseSensitivePrefixMatch(std::string& prefix) const;
+
 private:
   const envoy::type::matcher::v3::StringMatcher matcher_;
   Regex::CompiledMatcherPtr regex_;

source/common/config/utility.cc

@@ -223,8 +223,9 @@ Utility::createTagProducer(const envoy::config::bootstrap::v3::Bootstrap& bootst
 }
 
 Stats::StatsMatcherPtr
-Utility::createStatsMatcher(const envoy::config::bootstrap::v3::Bootstrap& bootstrap) {
-  return std::make_unique<Stats::StatsMatcherImpl>(bootstrap.stats_config());
+Utility::createStatsMatcher(const envoy::config::bootstrap::v3::Bootstrap& bootstrap,
+                            Stats::SymbolTable& symbol_table) {
+  return std::make_unique<Stats::StatsMatcherImpl>(bootstrap.stats_config(), symbol_table);
 }
 
 Stats::HistogramSettingsConstPtr

source/common/config/utility.h

@@ -431,7 +431,8 @@ class Utility {
    * Create StatsMatcher instance.
    */
   static Stats::StatsMatcherPtr
-  createStatsMatcher(const envoy::config::bootstrap::v3::Bootstrap& bootstrap);
+  createStatsMatcher(const envoy::config::bootstrap::v3::Bootstrap& bootstrap,
+                     Stats::SymbolTable& symbol_table);
 
   /**
    * Create HistogramSettings instance.

source/common/stats/BUILD

@@ -223,6 +223,7 @@ envoy_cc_library(
     srcs = ["stats_matcher_impl.cc"],
     hdrs = ["stats_matcher_impl.h"],
     deps = [
+        ":symbol_table_lib",
         "//envoy/stats:stats_interface",
         "//source/common/common:matchers_lib",
         "//source/common/protobuf",

source/common/stats/stats_matcher_impl.cc

@@ -7,12 +7,17 @@
 
 #include "source/common/common/utility.h"
 
+#include "absl/strings/match.h"
+
 namespace Envoy {
 namespace Stats {
 
 // TODO(ambuc): Refactor this into common/matchers.cc, since StatsMatcher is really just a thin
 // wrapper around what might be called a StringMatcherList.
-StatsMatcherImpl::StatsMatcherImpl(const envoy::config::metrics::v3::StatsConfig& config) {
+StatsMatcherImpl::StatsMatcherImpl(const envoy::config::metrics::v3::StatsConfig& config,
+                                   SymbolTable& symbol_table)
+    : symbol_table_(symbol_table), stat_name_pool_(std::make_unique<StatNamePool>(symbol_table)) {
+
   switch (config.stats_matcher().stats_matcher_case()) {
   case envoy::config::metrics::v3::StatsMatcher::StatsMatcherCase::kRejectAll:
     // In this scenario, there are no matchers to store.
@@ -22,13 +27,15 @@ StatsMatcherImpl::StatsMatcherImpl(const envoy::config::metrics::v3::StatsConfig
     // If we have an inclusion list, we are being default-exclusive.
     for (const auto& stats_matcher : config.stats_matcher().inclusion_list().patterns()) {
       matchers_.push_back(Matchers::StringMatcherImpl(stats_matcher));
+      optimizeLastMatcher();
     }
     is_inclusive_ = false;
     break;
   case envoy::config::metrics::v3::StatsMatcher::StatsMatcherCase::kExclusionList:
     // If we have an exclusion list, we are being default-inclusive.
     for (const auto& stats_matcher : config.stats_matcher().exclusion_list().patterns()) {
       matchers_.push_back(Matchers::StringMatcherImpl(stats_matcher));
+      optimizeLastMatcher();
     }
     FALLTHRU;
   default:
@@ -38,19 +45,74 @@ StatsMatcherImpl::StatsMatcherImpl(const envoy::config::metrics::v3::StatsConfig
   }
 }
 
-bool StatsMatcherImpl::rejects(const std::string& name) const {
-  //
-  //  is_inclusive_ | match | return
-  // ---------------+-------+--------
-  //        T       |   T   |   T     Default-inclusive and matching an (exclusion) matcher, deny.
-  //        T       |   F   |   F     Otherwise, allow.
-  //        F       |   T   |   F     Default-exclusive and matching an (inclusion) matcher, allow.
-  //        F       |   F   |   T     Otherwise, deny.
-  //
-  // This is an XNOR, which can be evaluated by checking for equality.
-
-  return (is_inclusive_ == std::any_of(matchers_.begin(), matchers_.end(),
-                                       [&name](auto& matcher) { return matcher.match(name); }));
+// If the last string-matcher added is a case-sensitive prefix match, and the
+// prefix ends in ".", then this drops that match and adds it to a list of
+// prefixes. This is beneficial because token prefixes can be handled more
+// efficiently as a StatName without requiring conversion to a string.
+//
+// In the future, other matcher patterns could be optimized in a similar way,
+// such as:
+//   * suffixes that begin with "."
+//   * exact-matches
+//   * substrings that begin and end with "."
+//
+// These are left unoptimized for the moment to keep the code-change simpler,
+// and because we haven't observed an acute performance need to optimize those
+// other patterns yet.
+void StatsMatcherImpl::optimizeLastMatcher() {
+  std::string prefix;
+  if (matchers_.back().getCaseSensitivePrefixMatch(prefix) && absl::EndsWith(prefix, ".") &&
+      prefix.size() > 1) {
+    prefixes_.push_back(stat_name_pool_->add(prefix.substr(0, prefix.size() - 1)));
+    matchers_.pop_back();
+  }
+}
+
+StatsMatcher::FastResult StatsMatcherImpl::fastRejects(StatName stat_name) const {
+  if (rejectsAll()) {
+    return FastResult::Rejects;
+  }
+  bool matches = fastRejectMatch(stat_name);
+  if ((is_inclusive_ || matchers_.empty()) && matches == is_inclusive_) {
+    // We can short-circuit the slow matchers only if they are empty, or if
+    // we are in inclusive-mode and we find a match.
+    return FastResult::Rejects;
+  } else if (matches) {
+    return FastResult::Matches;
+  }
+  return FastResult::NoMatch;
+}
+
+bool StatsMatcherImpl::fastRejectMatch(StatName stat_name) const {
+  return std::any_of(prefixes_.begin(), prefixes_.end(),
+                     [stat_name](StatName prefix) { return stat_name.startsWith(prefix); });
+}
+
+bool StatsMatcherImpl::slowRejects(FastResult fast_result, StatName stat_name) const {
+  bool match = slowRejectMatch(stat_name);
+
+  if (is_inclusive_ || match || fast_result != FastResult::Matches) {
+    //  is_inclusive_ | match | return
+    // ---------------+-------+--------
+    //        T       |   T   |   T   Default-inclusive and matching an (exclusion) matcher, deny.
+    //        T       |   F   |   F   Otherwise, allow.
+    //        F       |   T   |   F   Default-exclusive and matching an (inclusion) matcher, allow.
+    //        F       |   F   |   T   Otherwise, deny.
+    //
+    // This is an XNOR, which can be evaluated by checking for equality.
+    return match == is_inclusive_;
+  }
+
+  return false;
+}
+
+bool StatsMatcherImpl::slowRejectMatch(StatName stat_name) const {
+  if (matchers_.empty()) {
+    return false;
+  }
+  std::string name = symbol_table_->toString(stat_name);
+  return std::any_of(matchers_.begin(), matchers_.end(),
+                     [&name](auto& matcher) { return matcher.match(name); });
 }
 
 } // namespace Stats

source/common/stats/stats_matcher_impl.h

@@ -2,11 +2,13 @@
 
 #include <string>
 
+#include "envoy/common/optref.h"
 #include "envoy/config/metrics/v3/stats.pb.h"
 #include "envoy/stats/stats_matcher.h"
 
 #include "source/common/common/matchers.h"
 #include "source/common/protobuf/protobuf.h"
+#include "source/common/stats/symbol_table_impl.h"
 
 #include "absl/strings/string_view.h"
 
@@ -18,22 +20,40 @@ namespace Stats {
  */
 class StatsMatcherImpl : public StatsMatcher {
 public:
-  explicit StatsMatcherImpl(const envoy::config::metrics::v3::StatsConfig& config);
+  StatsMatcherImpl(const envoy::config::metrics::v3::StatsConfig& config,
+                   SymbolTable& symbol_table);
 
   // Default constructor simply allows everything.
   StatsMatcherImpl() = default;
 
   // StatsMatcher
-  bool rejects(const std::string& name) const override;
-  bool acceptsAll() const override { return is_inclusive_ && matchers_.empty(); }
-  bool rejectsAll() const override { return !is_inclusive_ && matchers_.empty(); }
+  bool rejects(StatName name) const override {
+    FastResult fast_result = fastRejects(name);
+    return fast_result == FastResult::Rejects || slowRejects(fast_result, name);
+  }
+  FastResult fastRejects(StatName name) const override;
+  bool slowRejects(FastResult, StatName name) const override;
+  bool acceptsAll() const override {
+    return is_inclusive_ && matchers_.empty() && prefixes_.empty();
+  }
+  bool rejectsAll() const override {
+    return !is_inclusive_ && matchers_.empty() && prefixes_.empty();
+  }
 
 private:
+  void optimizeLastMatcher();
+  bool fastRejectMatch(StatName name) const;
+  bool slowRejectMatch(StatName name) const;
+
   // Bool indicating whether or not the StatsMatcher is including or excluding stats by default. See
   // StatsMatcherImpl::rejects() for much more detail.
   bool is_inclusive_{true};
 
+  OptRef<SymbolTable> symbol_table_;
+  std::unique_ptr<StatNamePool> stat_name_pool_;
+
   std::vector<Matchers::StringMatcherImpl> matchers_;
+  std::vector<StatName> prefixes_;
 };
 
 } // namespace Stats

source/common/stats/symbol_table_impl.cc

@@ -154,6 +154,43 @@ void SymbolTableImpl::Encoding::decodeTokens(
   }
 }
 
+bool StatName::startsWith(StatName symbolic_prefix) const {
+  bool ret = true;
+  std::vector<Symbol> prefix_symbols;
+
+  // Decode the prefix as a StatNameVec.
+  SymbolTableImpl::Encoding::decodeTokens(
+      symbolic_prefix.data(), symbolic_prefix.dataSize(),
+      [&prefix_symbols](Symbol symbol) { prefix_symbols.push_back(symbol); },
+      [&ret](absl::string_view) { ret = false; });
+
+  // If there any dynamic components, our string_view lambda will be called, and
+  // then we'll return false for simplicity. We don't have a current need for
+  // prefixes to be expressed dynamically, and handling that case would add
+  // complexity.
+  if (!ret) {
+    return false;
+  }
+
+  // Now decode the StatName, matching against the symbols. It's OK for there to
+  // be dynamic string_view elements after the prefix match.
+  uint32_t index = 0;
+  SymbolTableImpl::Encoding::decodeTokens(
+      data(), dataSize(),
+      [&prefix_symbols, &index, &ret](Symbol symbol) {
+        if (index < prefix_symbols.size() && prefix_symbols[index] != symbol) {
+          ret = false;
+        }
+        ++index;
+      },
+      [&prefix_symbols, &index, &ret](absl::string_view) {
+        if (index < prefix_symbols.size()) {
+          ret = false;
+        }
+      });
+  return ret && index >= prefix_symbols.size();
+}
+
 std::vector<absl::string_view> SymbolTableImpl::decodeStrings(const SymbolTable::Storage array,
                                                               size_t size) const {
   std::vector<absl::string_view> strings;

source/common/stats/symbol_table_impl.h

@@ -456,6 +456,16 @@ class StatName {
    */
   bool empty() const { return size_and_data_ == nullptr || dataSize() == 0; }
 
+  /**
+   * Determines whether this starts with the prefix. Note: dynamic segments
+   * are not supported in the current implementation; this matching only works
+   * for symbolic segments. However it OK for this to include dynamic segments
+   * following the prefix.
+   *
+   * @param symbolic_prefix the prefix, which must not contain dynamic segments.
+   */
+  bool startsWith(StatName symbolic_prefix) const;
+
 private:
   /**
    * Casts the raw data as a string_view. Note that this string_view will not