From 77ade5ea2edc381e33901455ac998e816d4ef91e Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Mon, 29 Oct 2018 23:45:40 +0000 Subject: [PATCH] Use std::hash for check cache. Signed-off-by: Wayne Zhang --- include/istio/utils/concat_hash.h | 24 ++++++------------------ src/istio/mixerclient/check_cache.cc | 6 +++--- src/istio/mixerclient/check_cache.h | 4 ++-- src/istio/mixerclient/quota_cache.cc | 6 +++--- src/istio/mixerclient/quota_cache.h | 4 ++-- src/istio/mixerclient/referenced.cc | 6 +++--- src/istio/mixerclient/referenced.h | 10 +++++----- src/istio/mixerclient/referenced_test.cc | 23 ++++++----------------- 8 files changed, 30 insertions(+), 53 deletions(-) diff --git a/include/istio/utils/concat_hash.h b/include/istio/utils/concat_hash.h index bc48e4d6338..8906f07089d 100644 --- a/include/istio/utils/concat_hash.h +++ b/include/istio/utils/concat_hash.h @@ -17,11 +17,15 @@ #define ISTIO_UTILS_CONCAT_HASH_H_ #include +#include #include namespace istio { namespace utils { +// The hash type for Check cache. +typedef std::size_t HashType; + // This class concatenates multiple values into a string as hash class ConcatHash { public: @@ -48,24 +52,8 @@ class ConcatHash { return *this; } - // Returns the concated string as hash. - std::string getHash() const { return hash_; } - - // Converts a binary string to a printable string for unit-test only - static std::string DebugString(const std::string& hash) { - std::string out; - out.reserve(hash.size() * 2); - for (auto c : hash) { - if (std::isprint(c)) { - out.append(1, c); - } else { - char buf[10]; - sprintf(buf, "%02x", (unsigned char)c); - out.append(buf); - } - } - return out; - } + // Returns the hash of the concated string. + HashType getHash() const { return std::hash{}(hash_); } private: std::string hash_; diff --git a/src/istio/mixerclient/check_cache.cc b/src/istio/mixerclient/check_cache.cc index 0e3407fc58e..2f10557ce2a 100644 --- a/src/istio/mixerclient/check_cache.cc +++ b/src/istio/mixerclient/check_cache.cc @@ -106,7 +106,7 @@ Status CheckCache::Check(const Attributes &attributes, Tick time_now, std::lock_guard lock(cache_mutex_); for (const auto &it : referenced_map_) { const Referenced &reference = it.second; - std::string signature; + utils::HashType signature; if (!reference.Signature(attributes, "", &signature)) { continue; } @@ -145,7 +145,7 @@ Status CheckCache::CacheResponse(const Attributes &attributes, // Failed to decode referenced_attributes, not to cache this result. return ConvertRpcStatus(response.precondition().status()); } - std::string signature; + utils::HashType signature; if (!referenced.Signature(attributes, "", &signature)) { GOOGLE_LOG(ERROR) << "Response referenced mismatchs with request"; GOOGLE_LOG(ERROR) << "Request attributes: " << attributes.DebugString(); @@ -154,7 +154,7 @@ Status CheckCache::CacheResponse(const Attributes &attributes, } std::lock_guard lock(cache_mutex_); - std::string hash = referenced.Hash(); + utils::HashType hash = referenced.Hash(); if (referenced_map_.find(hash) == referenced_map_.end()) { referenced_map_[hash] = referenced; GOOGLE_LOG(INFO) << "Add a new Referenced for check cache: " diff --git a/src/istio/mixerclient/check_cache.h b/src/istio/mixerclient/check_cache.h index 36e9080f5cd..e5aa7ea934d 100644 --- a/src/istio/mixerclient/check_cache.h +++ b/src/istio/mixerclient/check_cache.h @@ -155,13 +155,13 @@ class CheckCache { // Key is the signature of the Attributes. Value is the CacheElem. // It is a LRU cache with maximum size. // When the maximum size is reached, oldest idle items will be removed. - using CheckLRUCache = utils::SimpleLRUCache; + using CheckLRUCache = utils::SimpleLRUCache; // The check options. CheckOptions options_; // Referenced map keyed with their hashes - std::unordered_map referenced_map_; + std::unordered_map referenced_map_; // Mutex guarding the access of cache_; std::mutex cache_mutex_; diff --git a/src/istio/mixerclient/quota_cache.cc b/src/istio/mixerclient/quota_cache.cc index cfd97385d6c..4d77a038fff 100644 --- a/src/istio/mixerclient/quota_cache.cc +++ b/src/istio/mixerclient/quota_cache.cc @@ -174,7 +174,7 @@ void QuotaCache::CheckCache(const Attributes& request, bool check_use_cache, PerQuotaReferenced& quota_ref = quota_referenced_map_[quota->name]; for (const auto& it : quota_ref.referenced_map) { const Referenced& referenced = it.second; - std::string signature; + utils::HashType signature; if (!referenced.Signature(request, quota->name, &signature)) { continue; } @@ -216,7 +216,7 @@ void QuotaCache::SetResponse(const Attributes& attributes, return; } - std::string signature; + utils::HashType signature; if (!referenced.Signature(attributes, quota_name, &signature)) { GOOGLE_LOG(ERROR) << "Quota response referenced mismatchs with request"; GOOGLE_LOG(ERROR) << "Request attributes: " << attributes.DebugString(); @@ -232,7 +232,7 @@ void QuotaCache::SetResponse(const Attributes& attributes, } PerQuotaReferenced& quota_ref = quota_referenced_map_[quota_name]; - std::string hash = referenced.Hash(); + utils::HashType hash = referenced.Hash(); if (quota_ref.referenced_map.find(hash) == quota_ref.referenced_map.end()) { quota_ref.referenced_map[hash] = referenced; GOOGLE_LOG(INFO) << "Add a new Referenced for quota cache: " << quota_name diff --git a/src/istio/mixerclient/quota_cache.h b/src/istio/mixerclient/quota_cache.h index ce7d1025f9e..231e4324795 100644 --- a/src/istio/mixerclient/quota_cache.h +++ b/src/istio/mixerclient/quota_cache.h @@ -140,7 +140,7 @@ class QuotaCache { std::unique_ptr pending_item; // Referenced map keyed with their hashes - std::unordered_map referenced_map; + std::unordered_map referenced_map; }; // Set a quota response. @@ -154,7 +154,7 @@ class QuotaCache { // Key is the signature of the Attributes. Value is the CacheElem. // It is a LRU cache with MaxIdelTime as response_expiration_time. - using QuotaLRUCache = utils::SimpleLRUCache; + using QuotaLRUCache = utils::SimpleLRUCache; // The quota options. QuotaOptions options_; diff --git a/src/istio/mixerclient/referenced.cc b/src/istio/mixerclient/referenced.cc index 187ef22b45f..464e44a8611 100644 --- a/src/istio/mixerclient/referenced.cc +++ b/src/istio/mixerclient/referenced.cc @@ -117,7 +117,7 @@ bool Referenced::Fill(const Attributes &attributes, bool Referenced::Signature(const Attributes &attributes, const std::string &extra_key, - std::string *signature) const { + utils::HashType *signature) const { if (!CheckAbsentKeys(attributes) || !CheckExactKeys(attributes)) { return false; } @@ -201,7 +201,7 @@ bool Referenced::CheckExactKeys(const Attributes &attributes) const { void Referenced::CalculateSignature(const Attributes &attributes, const std::string &extra_key, - std::string *signature) const { + utils::HashType *signature) const { const auto &attributes_map = attributes.attributes(); utils::ConcatHash hasher(kMaxConcatHashSize); @@ -278,7 +278,7 @@ void Referenced::CalculateSignature(const Attributes &attributes, *signature = hasher.getHash(); } -std::string Referenced::Hash() const { +utils::HashType Referenced::Hash() const { utils::ConcatHash hasher(kMaxConcatHashSize); // keys are sorted during Fill diff --git a/src/istio/mixerclient/referenced.h b/src/istio/mixerclient/referenced.h index 14e247d6f87..698c5bcbf32 100644 --- a/src/istio/mixerclient/referenced.h +++ b/src/istio/mixerclient/referenced.h @@ -37,13 +37,13 @@ class Referenced { // Calculate a cache signature for the attributes. // Return false if attributes are mismatched, such as "absence" attributes - // present - // or "exact" match attributes don't present. + // present or "exact" match attributes don't present. bool Signature(const ::istio::mixer::v1::Attributes &attributes, - const std::string &extra_key, std::string *signature) const; + const std::string &extra_key, + utils::HashType *signature) const; // A hash value to identify an instance. - std::string Hash() const; + utils::HashType Hash() const; // For debug logging only. std::string DebugString() const; @@ -58,7 +58,7 @@ class Referenced { // Do the actual signature calculation. void CalculateSignature(const ::istio::mixer::v1::Attributes &attributes, const std::string &extra_key, - std::string *signature) const; + utils::HashType *signature) const; // Holds reference to an attribute and potentially a map key struct AttributeRef { diff --git a/src/istio/mixerclient/referenced_test.cc b/src/istio/mixerclient/referenced_test.cc index 5ddc7ccb503..0ab99179c59 100644 --- a/src/istio/mixerclient/referenced_test.cc +++ b/src/istio/mixerclient/referenced_test.cc @@ -177,10 +177,7 @@ TEST(ReferencedTest, FillSuccessTest) { "duration-key, int-key, string-key, string-map-key[If-Match], " "time-key, "); - EXPECT_EQ(utils::ConcatHash::DebugString(referenced.Hash()), - "string-map-key00User-Agent00target.name00target.service00:bool-" - "key00bytes-key00double-key00duration-key00int-key00string-" - "key00string-map-key00If-Match00time-key00"); + EXPECT_EQ(referenced.Hash(), 15726019709841724427U); } TEST(ReferencedTest, FillFail1Test) { @@ -209,7 +206,7 @@ TEST(ReferencedTest, NegativeSignature1Test) { Referenced referenced; EXPECT_TRUE(referenced.Fill(attrs, pb)); - std::string signature; + utils::HashType signature; Attributes attributes1; // "target.service" should be absence. @@ -248,16 +245,10 @@ TEST(ReferencedTest, OKSignature1Test) { Referenced referenced; EXPECT_TRUE(referenced.Fill(attributes, pb)); - std::string signature; + utils::HashType signature; EXPECT_TRUE(referenced.Signature(attributes, "extra", &signature)); - EXPECT_EQ(utils::ConcatHash::DebugString(signature), - "bool-key000100bytes-key00this is a bytes " - "value00double-key009a99999999f9X@00duration-" - "key000500000000000000000000000000int-key00#0000000000000000string-" - "key00this is a string " - "value00string-map-key00If-Match00value10000time-" - "key000000000000000000000000000000extra"); + EXPECT_EQ(signature, 7485122822970549717U); } TEST(ReferencedTest, StringMapReferencedTest) { @@ -276,11 +267,9 @@ TEST(ReferencedTest, StringMapReferencedTest) { Referenced referenced; EXPECT_TRUE(referenced.Fill(attrs, pb)); - std::string signature; + utils::HashType signature; EXPECT_TRUE(referenced.Signature(attrs, "extra", &signature)); - EXPECT_EQ(utils::ConcatHash::DebugString(signature), - "map-key100value100map-key200exact-subkey400subvalue400exact-" - "subkey500subvalue50000extra"); + EXPECT_EQ(signature, 5578853713114714386U); // negative test: map-key3 must absence ::istio::mixer::v1::Attributes attr1(attrs);