absl: Absl hash hook in a couple of places rather than hash functors (#…

…8179)

Signed-off-by: Joshua Marantz <[email protected]>

source/common/common/utility.h

@@ -500,13 +500,6 @@ template <typename Value> class IntervalSetImpl : public IntervalSet<Value> {
   std::set<Interval, Compare> intervals_; // Intervals do not overlap or abut.
 };
 
-/**
- * Hashing functor for use with unordered_map and unordered_set with string_view as a key.
- */
-struct StringViewHash {
-  std::size_t operator()(const absl::string_view& k) const { return HashUtil::xxHash64(k); }
-};
-
 /**
  * Hashing functor for use with enum class types.
  * This is needed for GCC 5.X; newer versions of GCC, as well as clang7, provide native hashing

source/common/stats/symbol_table_impl.h

@@ -229,7 +229,7 @@ class SymbolTableImpl : public SymbolTable {
   // Bitmap implementation.
   // The encode map stores both the symbol and the ref count of that symbol.
   // Using absl::string_view lets us only store the complete string once, in the decode map.
-  using EncodeMap = absl::flat_hash_map<absl::string_view, SharedSymbol, StringViewHash>;
+  using EncodeMap = absl::flat_hash_map<absl::string_view, SharedSymbol>;
   using DecodeMap = absl::flat_hash_map<Symbol, InlineStringPtr>;
   EncodeMap encode_map_ GUARDED_BY(lock_);
   DecodeMap decode_map_ GUARDED_BY(lock_);
@@ -317,16 +317,31 @@ class StatName {
   // for lookup in a cache, and then on a miss need to store the data directly.
   StatName(const StatName& src, SymbolTable::Storage memory);
 
+  /**
+   * Defines default hash function so StatName can be used as a key in an absl
+   * hash-table without specifying a functor. See
+   * https://abseil.io/docs/cpp/guides/hash for details.
+   */
+  template <typename H> friend H AbslHashValue(H h, StatName stat_name) {
+    if (stat_name.empty()) {
+      return H::combine(std::move(h), absl::string_view());
+    }
+
+    // Casts the raw data as a string_view. Note that this string_view will not
+    // be in human-readable form, but it will be compatible with a string-view
+    // hasher.
+    const char* cdata = reinterpret_cast<const char*>(stat_name.data());
+    absl::string_view data_as_string_view = absl::string_view(cdata, stat_name.dataSize());
+    return H::combine(std::move(h), data_as_string_view);
+  }
+
   /**
    * Note that this hash function will return a different hash than that of
    * the elaborated string.
    *
    * @return uint64_t a hash of the underlying representation.
    */
-  uint64_t hash() const {
-    const char* cdata = reinterpret_cast<const char*>(data());
-    return HashUtil::xxHash64(absl::string_view(cdata, dataSize()));
-  }
+  uint64_t hash() const { return absl::Hash<StatName>()(*this); }
 
   bool operator==(const StatName& rhs) const {
     const uint64_t sz = dataSize();
@@ -339,6 +354,9 @@ class StatName {
    *                  overhead for the size itself.
    */
   uint64_t dataSize() const {
+    if (size_and_data_ == nullptr) {
+      return 0;
+    }
     return size_and_data_[0] | (static_cast<uint64_t>(size_and_data_[1]) << 8);
   }
 
@@ -523,22 +541,11 @@ class StatNameList {
   SymbolTable::StoragePtr storage_;
 };
 
-// Helper class for constructing hash-tables with StatName keys.
-struct StatNameHash {
-  size_t operator()(const StatName& a) const { return a.hash(); }
-};
-
-// Helper class for constructing hash-tables with StatName keys.
-struct StatNameCompare {
-  bool operator()(const StatName& a, const StatName& b) const { return a == b; }
-};
-
 // Value-templatized hash-map with StatName key.
-template <class T>
-using StatNameHashMap = absl::flat_hash_map<StatName, T, StatNameHash, StatNameCompare>;
+template <class T> using StatNameHashMap = absl::flat_hash_map<StatName, T>;
 
 // Hash-set of StatNames
-using StatNameHashSet = absl::flat_hash_set<StatName, StatNameHash, StatNameCompare>;
+using StatNameHashSet = absl::flat_hash_set<StatName>;
 
 // Helper class for sorting StatNames.
 struct StatNameLessThan {

source/common/stats/tag_producer_impl.h

@@ -17,6 +17,7 @@
 #include "common/config/well_known_names.h"
 #include "common/protobuf/protobuf.h"
 
+#include "absl/container/flat_hash_map.h"
 #include "absl/strings/string_view.h"
 
 namespace Envoy {
@@ -97,8 +98,7 @@ class TagProducerImpl : public TagProducer {
   // Maps a prefix word extracted out of a regex to a vector of TagExtractors. Note that
   // the storage for the prefix string is owned by the TagExtractor, which, depending on
   // implementation, may need make a copy of the prefix.
-  std::unordered_map<absl::string_view, std::vector<TagExtractorPtr>, StringViewHash>
-      tag_extractor_prefix_map_;
+  absl::flat_hash_map<absl::string_view, std::vector<TagExtractorPtr>> tag_extractor_prefix_map_;
   std::vector<Tag> default_tags_;
 };
 

test/common/stats/BUILD

@@ -84,6 +84,7 @@ envoy_cc_test(
 envoy_cc_test(
     name = "symbol_table_impl_test",
     srcs = ["symbol_table_impl_test.cc"],
+    external_deps = ["abseil_hash_testing"],
     deps = [
         ":stat_test_utility_lib",
         "//source/common/common:mutex_tracer_lib",

test/common/stats/symbol_table_impl_test.cc

@@ -10,6 +10,7 @@
 #include "test/test_common/logging.h"
 #include "test/test_common/utility.h"
 
+#include "absl/hash/hash_testing.h"
 #include "absl/synchronization/blocking_counter.h"
 #include "gtest/gtest.h"
 
@@ -565,6 +566,26 @@ TEST_P(StatNameTest, StatNameSet) {
   EXPECT_NE(dynamic2.data(), dynamic.data());
 }
 
+TEST_P(StatNameTest, StatNameEmptyEquivalent) {
+  StatName empty1;
+  StatName empty2 = makeStat("");
+  StatName non_empty = makeStat("a");
+  EXPECT_EQ(empty1, empty2);
+  EXPECT_EQ(empty1.hash(), empty2.hash());
+  EXPECT_NE(empty1, non_empty);
+  EXPECT_NE(empty2, non_empty);
+  EXPECT_NE(empty1.hash(), non_empty.hash());
+  EXPECT_NE(empty2.hash(), non_empty.hash());
+}
+
+TEST_P(StatNameTest, SupportsAbslHash) {
+  EXPECT_TRUE(absl::VerifyTypeImplementsAbslHashCorrectly({
+      StatName(),
+      makeStat(""),
+      makeStat("hello.world"),
+  }));
+}
+
 // Tests the memory savings realized from using symbol tables with 1k
 // clusters. This test shows the memory drops from almost 8M to less than
 // 2M. Note that only SymbolTableImpl is tested for memory consumption,