diff --git a/include/envoy/stats/scope.h b/include/envoy/stats/scope.h index d37ca645e771f..85b076d754c6f 100644 --- a/include/envoy/stats/scope.h +++ b/include/envoy/stats/scope.h @@ -113,6 +113,21 @@ class Scope { */ virtual const SymbolTable& constSymbolTable() const PURE; virtual SymbolTable& symbolTable() PURE; + + /** + * Performs fast conversion of string to StatName in the hot-path, at the + * expense of significant per-name-per-thread memory overhead. This incurs + * a lock only the first time this string is referenced from a thread. It + * should only be used names that cannot be determined during startup or xDS + * update. Forthose names that can be determined at that time, the StatNames + * should be collected via StatNamePool or StatNameManagedStorage once, and + * then used later on in the hot-path to compose fully elaborated StatName + * objects via SymbolTable::join(), which is lock-free. + * + * @param name The name of the stat or fragment of stat. + * @return the stat name allocated from symbolTable(). + */ + virtual StatName fastMemoryIntensiveStatNameLookup(absl::string_view name) PURE; }; } // namespace Stats diff --git a/include/envoy/stats/symbol_table.h b/include/envoy/stats/symbol_table.h index 4b4c8a4c4fe9e..10db6a388efc9 100644 --- a/include/envoy/stats/symbol_table.h +++ b/include/envoy/stats/symbol_table.h @@ -146,6 +146,7 @@ class SymbolTable { friend struct HeapStatData; friend class StatNameStorage; friend class StatNameList; + friend class StringStatNameMap; // The following methods are private, but are called by friend classes // StatNameStorage and StatNameList, which must be friendly with SymbolTable @@ -182,6 +183,24 @@ class SymbolTable { * */ virtual StoragePtr encode(absl::string_view name) PURE; + + /** + * Returns a vector of absl:string_view containing the symbols in the + * StatName. The string_view elements reference memory that is held in the + * SymbolTable as long as a reference to the symbols is retained. This takes + * a lock on the symbol table. + */ + virtual std::vector + statNameToStringVector(const StatName& stat_name) const PURE; + + /** + * Splits a string_view into multiple symbols. This does not require a lock on + * the symbol table. + * + * @param str the string to split, based on the semantics of the SymbolTable. + * @return the string, split into tokens. + */ + virtual std::vector splitString(absl::string_view str) const PURE; }; using SharedSymbolTable = std::shared_ptr; diff --git a/source/common/common/hash.h b/source/common/common/hash.h index b6c50e63ac74f..87a59b9a39a1b 100644 --- a/source/common/common/hash.h +++ b/source/common/common/hash.h @@ -82,7 +82,7 @@ struct ConstCharStarHash { }; struct ConstCharStarEqual { - size_t operator()(const char* a, const char* b) const { return strcmp(a, b) == 0; } + bool operator()(const char* a, const char* b) const { return strcmp(a, b) == 0; } }; template @@ -111,10 +111,10 @@ struct HeterogeneousStringEqual { // See description for HeterogeneousStringHash::is_transparent. using is_transparent = void; - size_t operator()(absl::string_view a, absl::string_view b) const { return a == b; } - size_t operator()(const SharedString& a, const SharedString& b) const { return *a == *b; } - size_t operator()(absl::string_view a, const SharedString& b) const { return a == *b; } - size_t operator()(const SharedString& a, absl::string_view b) const { return *a == b; } + bool operator()(absl::string_view a, absl::string_view b) const { return a == b; } + bool operator()(const SharedString& a, const SharedString& b) const { return *a == *b; } + bool operator()(absl::string_view a, const SharedString& b) const { return a == *b; } + bool operator()(const SharedString& a, absl::string_view b) const { return *a == b; } }; // We use heterogeneous hash/equal functors to do a find() without constructing diff --git a/source/common/stats/fake_symbol_table_impl.h b/source/common/stats/fake_symbol_table_impl.h index 096cb42d4d72b..e03fcbd8bfc73 100644 --- a/source/common/stats/fake_symbol_table_impl.h +++ b/source/common/stats/fake_symbol_table_impl.h @@ -125,6 +125,11 @@ class FakeSymbolTableImpl : public SymbolTable { fn(toStringView(stat_name)); } + std::vector statNameToStringVector(const StatName& stat_name) const override { + return {toStringView(stat_name)}; + } + std::vector splitString(absl::string_view s) const override { return {s}; } + private: absl::string_view toStringView(const StatName& stat_name) const { return {reinterpret_cast(stat_name.data()), diff --git a/source/common/stats/isolated_store_impl.cc b/source/common/stats/isolated_store_impl.cc index 905ff98db06d0..60f12e177f7b6 100644 --- a/source/common/stats/isolated_store_impl.cc +++ b/source/common/stats/isolated_store_impl.cc @@ -36,9 +36,22 @@ IsolatedStoreImpl::IsolatedStoreImpl(SymbolTable& symbol_table) }), null_gauge_(symbol_table) {} +IsolatedStoreImpl::~IsolatedStoreImpl() { stat_name_set_.free(symbolTable()); } + ScopePtr IsolatedStoreImpl::createScope(const std::string& name) { return std::make_unique(name, *this); } +StatName IsolatedStoreImpl::fastMemoryIntensiveStatNameLookup(absl::string_view name) { + absl::optional stat_name = string_stat_name_map_.find(name, symbolTable()); + if (!stat_name) { + StatNameStorage storage(name, symbolTable()); + auto insertion = stat_name_set_.insert(std::move(storage)); + ASSERT(insertion.second); // If the name is not in the map, it should not be in the set. + stat_name = insertion.first->statName(); + } + return *stat_name; +} + } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/isolated_store_impl.h b/source/common/stats/isolated_store_impl.h index d658565e63d9a..961322be06804 100644 --- a/source/common/stats/isolated_store_impl.h +++ b/source/common/stats/isolated_store_impl.h @@ -68,6 +68,7 @@ class IsolatedStoreImpl : public StoreImpl { public: IsolatedStoreImpl(); explicit IsolatedStoreImpl(SymbolTable& symbol_table); + ~IsolatedStoreImpl() override; // Stats::Scope Counter& counterFromStatName(StatName name) override { return counters_.get(name); } @@ -110,6 +111,8 @@ class IsolatedStoreImpl : public StoreImpl { return histogramFromStatName(storage.statName()); } + StatName fastMemoryIntensiveStatNameLookup(absl::string_view name) override; + private: IsolatedStoreImpl(std::unique_ptr&& symbol_table); @@ -119,6 +122,8 @@ class IsolatedStoreImpl : public StoreImpl { IsolatedStatsCache gauges_; IsolatedStatsCache histograms_; NullGaugeImpl null_gauge_; + StatNameStorageSet stat_name_set_; + StringStatNameMap string_stat_name_map_; }; } // namespace Stats diff --git a/source/common/stats/scope_prefixer.cc b/source/common/stats/scope_prefixer.cc index 44314d967de1c..0fbd607813239 100644 --- a/source/common/stats/scope_prefixer.cc +++ b/source/common/stats/scope_prefixer.cc @@ -5,14 +5,18 @@ #include "common/stats/symbol_table_impl.h" #include "common/stats/utility.h" +#include "absl/strings/str_cat.h" + namespace Envoy { namespace Stats { ScopePrefixer::ScopePrefixer(absl::string_view prefix, Scope& scope) - : scope_(scope), prefix_(Utility::sanitizeStatsName(prefix), symbolTable()) {} + : scope_(scope), prefix_(Utility::sanitizeStatsName(prefix), symbolTable()), + prefix_string_(std::string(prefix)) {} ScopePrefixer::ScopePrefixer(StatName prefix, Scope& scope) - : scope_(scope), prefix_(prefix, symbolTable()) {} + : scope_(scope), prefix_(prefix, symbolTable()), + prefix_string_(symbolTable().toString(prefix) + ".") {} ScopePrefixer::~ScopePrefixer() { prefix_.free(symbolTable()); } @@ -62,5 +66,9 @@ void ScopePrefixer::deliverHistogramToSinks(const Histogram& histograms, uint64_ scope_.deliverHistogramToSinks(histograms, val); } +StatName ScopePrefixer::fastMemoryIntensiveStatNameLookup(absl::string_view name) { + return scope_.fastMemoryIntensiveStatNameLookup(absl::StrCat(prefix_string_, name)); +} + } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/scope_prefixer.h b/source/common/stats/scope_prefixer.h index c289a59571d72..7734a2170b48b 100644 --- a/source/common/stats/scope_prefixer.h +++ b/source/common/stats/scope_prefixer.h @@ -45,9 +45,12 @@ class ScopePrefixer : public Scope { NullGaugeImpl& nullGauge(const std::string& str) override { return scope_.nullGauge(str); } + StatName fastMemoryIntensiveStatNameLookup(absl::string_view name) override; + private: Scope& scope_; StatNameStorage prefix_; + std::string prefix_string_; }; } // namespace Stats diff --git a/source/common/stats/symbol_table_impl.cc b/source/common/stats/symbol_table_impl.cc index 9a330cabf08b4..4f60c08302695 100644 --- a/source/common/stats/symbol_table_impl.cc +++ b/source/common/stats/symbol_table_impl.cc @@ -153,6 +153,16 @@ void SymbolTableImpl::callWithStringView(StatName stat_name, } std::string SymbolTableImpl::decodeSymbolVec(const SymbolVec& symbols) const { + return absl::StrJoin(decodeSymbolVecTokens(symbols), "."); +} + +std::vector +SymbolTableImpl::statNameToStringVector(const StatName& stat_name) const { + return decodeSymbolVecTokens(Encoding::decodeSymbols(stat_name.data(), stat_name.dataSize())); +} + +std::vector +SymbolTableImpl::decodeSymbolVecTokens(const SymbolVec& symbols) const { std::vector name_tokens; name_tokens.reserve(symbols.size()); { @@ -162,7 +172,7 @@ std::string SymbolTableImpl::decodeSymbolVec(const SymbolVec& symbols) const { name_tokens.push_back(fromSymbol(symbol)); } } - return absl::StrJoin(name_tokens, "."); + return name_tokens; } void SymbolTableImpl::incRefCount(const StatName& stat_name) { @@ -429,5 +439,37 @@ void StatNameList::clear(SymbolTable& symbol_table) { storage_.reset(); } +size_t StringViewVectorHash::operator()(const std::vector& a) const { + uint64_t hash = 0; + for (absl::string_view sv : a) { + hash = 63 * hash + HashUtil::xxHash64(sv); + } + return hash; +} + +absl::optional StringStatNameMap::find(absl::string_view name, + const SymbolTable& symbol_table) const { + absl::optional ret; + + // TODO(jmarantz)(: change the hash/compare mechanism to avoid needing to + // allocate memory during the find() operation. We should be able to + // iterate over the "."-separated tokens to compute the hash-value without + // having to have the allocated split vector first. Similar for compare. + // + // The only nuance -- and the reason to leave this as is for now, is that + // when using FakeSymbolTable, there is no actual splitting by token. + std::vector v = symbol_table.splitString(name); + auto iter = map_.find(v); + if (iter != map_.end()) { + ret = iter->second; + } + return ret; +} + +void StringStatNameMap::insert(StatName stat_name, const SymbolTable& symbol_table) { + std::vector v = symbol_table.statNameToStringVector(stat_name); + map_[v] = stat_name; +} + } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/symbol_table_impl.h b/source/common/stats/symbol_table_impl.h index 60c796d1def90..99f0246edb671 100644 --- a/source/common/stats/symbol_table_impl.h +++ b/source/common/stats/symbol_table_impl.h @@ -139,6 +139,11 @@ class SymbolTableImpl : public SymbolTable { void callWithStringView(StatName stat_name, const std::function& fn) const override; + std::vector statNameToStringVector(const StatName& stat_name) const override; + std::vector splitString(absl::string_view s) const override { + return absl::StrSplit(s, '.'); + } + #ifndef ENVOY_CONFIG_COVERAGE void debugPrint() const override; #endif @@ -183,6 +188,7 @@ class SymbolTableImpl : public SymbolTable { * @return std::string the retrieved stat name. */ std::string decodeSymbolVec(const SymbolVec& symbols) const; + std::vector decodeSymbolVecTokens(const SymbolVec& symbols) const; /** * Convenience function for encode(), symbolizing one string segment at a time. @@ -528,17 +534,11 @@ 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 -using StatNameHashMap = absl::flat_hash_map; +template using StatNameHashMap = absl::flat_hash_map; // Hash-set of StatNames -using StatNameHashSet = absl::flat_hash_set; +using StatNameHashSet = absl::flat_hash_set; // Helper class for sorting StatNames. struct StatNameLessThan { @@ -562,13 +562,13 @@ struct HeterogeneousStatNameHash { size_t operator()(StatName a) const { return a.hash(); } size_t operator()(const StatNameStorage& a) const { return a.statName().hash(); } + size_t operator()(absl::string_view str) const; }; struct HeterogeneousStatNameEqual { // See description for HeterogeneousStatNameHash::is_transparent. using is_transparent = void; - size_t operator()(StatName a, StatName b) const { return a == b; } size_t operator()(const StatNameStorage& a, const StatNameStorage& b) const { return a.statName() == b.statName(); } @@ -630,5 +630,40 @@ class StatNameStorageSet { HashSet hash_set_; }; +struct StringViewVectorHash { + size_t operator()(const std::vector& a) const; +}; + +// Captures a map from string_view to StatName. The backing store for both of +// these must be separately managed, and live beyond the map. +// +// This is intended for use as a TLS cache in thread_local_store.cc, with the +// backing store held in sets in the CentralCacheEntry. +class StringStatNameMap { +public: + /** + * This does not take a lock in the symbol table. + * @param name the string stat-name to find. + * @param symbol_table the symbol-table used to help split the string into tokens. + * @return the StatName, if found. + */ + absl::optional find(absl::string_view name, const SymbolTable& symbol_table) const; + + /** + * Inserts stat_name into the StatNameStorageMap. Caller must guarantee that + * the backing-store for StatName lasts longer than the StatNameStorageMap. + * + * Note: this takes a lock in the symbol table, in order to do + * symbol-string-lookups in its lock-protected symbol map. + * + * @param stat_name the name to insert. + * @param symbol_table the symbol-table used to find stable strings for the tokens in stat names. + */ + void insert(StatName stat_name, const SymbolTable& symbol_table); + +private: + absl::flat_hash_map, StatName, StringViewVectorHash> map_; +}; + } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index dcb65e34f2e3b..fa7570c77b216 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -31,9 +31,9 @@ ThreadLocalStoreImpl::~ThreadLocalStoreImpl() { ASSERT(shutting_down_ || !threading_ever_initialized_); default_scope_.reset(); ASSERT(scopes_.empty()); - for (StatNameStorageSet* rejected_stats : rejected_stats_purgatory_) { - rejected_stats->free(symbolTable()); - delete rejected_stats; + for (StatNameStorageSet* stats : stats_purgatory_) { + stats->free(symbolTable()); + delete stats; } } @@ -206,7 +206,15 @@ void ThreadLocalStoreImpl::releaseScopeCrossThread(ScopeImpl* scope) { // // We use a raw pointer here as it's easier to capture it in in the lambda. auto rejected_stats = new StatNameStorageSet; + auto hot_path_stats = new StatNameStorageSet; rejected_stats->swap(scope->central_cache_.rejected_stats_); + hot_path_stats->swap(scope->central_cache_.hot_path_stats_); + auto cleanup_stats = [rejected_stats, hot_path_stats, this]() { + rejected_stats->free(symbolTable()); + hot_path_stats->free(symbolTable()); + delete rejected_stats; + delete hot_path_stats; + }; // This can happen from any thread. We post() back to the main thread which will initiate the // cache flush operation. @@ -221,22 +229,22 @@ void ThreadLocalStoreImpl::releaseScopeCrossThread(ScopeImpl* scope) { // be cleared out in the ThreadLocalStoreImpl destructor. We'd prefer // to release the memory immediately, however, in which case we remove // the rejected stats set from purgatory. - rejected_stats_purgatory_.insert(rejected_stats); - auto clean_central_cache = [this, rejected_stats]() { + stats_purgatory_.insert(rejected_stats); + stats_purgatory_.insert(hot_path_stats); + auto clean_central_cache = [this, rejected_stats, hot_path_stats, cleanup_stats]() { { Thread::LockGuard lock(lock_); - rejected_stats_purgatory_.erase(rejected_stats); + stats_purgatory_.erase(rejected_stats); + stats_purgatory_.erase(hot_path_stats); } - rejected_stats->free(symbolTable()); - delete rejected_stats; + cleanup_stats(); }; lock.release(); main_thread_dispatcher_->post([this, clean_central_cache, scope_id]() { clearScopeFromCaches(scope_id, clean_central_cache); }); } else { - rejected_stats->free(symbolTable()); - delete rejected_stats; + cleanup_stats(); } } @@ -507,6 +515,39 @@ Histogram& ThreadLocalStoreImpl::ScopeImpl::histogramFromStatName(StatName name) return **central_ref; } +StatName +ThreadLocalStoreImpl::ScopeImpl::fastMemoryIntensiveStatNameLookup(absl::string_view name) { + absl::optional stat_name; + + // Try to populate stat_name from the TLS cache, without taking any locks. + StringStatNameMap* tls_cache = nullptr; + if (!parent_.shutting_down_ && parent_.tls_) { + TlsCacheEntry& entry = parent_.tls_->getTyped().scope_cache_[this->scope_id_]; + tls_cache = &entry.hot_path_stats_map_; + stat_name = tls_cache->find(name, symbolTable()); + if (stat_name) { + return *stat_name; + } + } + + // If that doesn't work, try the central cache. + { + Thread::LockGuard lock(parent_.lock_); + stat_name = central_cache_.hot_path_stats_map_.find(name, symbolTable()); + if (!stat_name) { + StatNameStorage storage(name, symbolTable()); + auto insertion = central_cache_.hot_path_stats_.insert(std::move(storage)); + stat_name = insertion.first->statName(); + ASSERT(insertion.second); // hot_path_stats_map_ and hot_path_stats_ are lock-protected. + central_cache_.hot_path_stats_map_.insert(*stat_name, symbolTable()); + } + } + if (tls_cache != nullptr) { + tls_cache->insert(*stat_name, symbolTable()); + } + return *stat_name; +} + absl::optional> ThreadLocalStoreImpl::ScopeImpl::findCounter(StatName name) const { return findStatLockHeld(name, central_cache_.counters_); diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h index c9a70d5ec7183..108652e030670 100644 --- a/source/common/stats/thread_local_store.h +++ b/source/common/stats/thread_local_store.h @@ -157,6 +157,10 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo } Histogram& histogram(const std::string& name) override { return default_scope_->histogram(name); } NullGaugeImpl& nullGauge(const std::string&) override { return null_gauge_; } + StatName fastMemoryIntensiveStatNameLookup(absl::string_view name) override { + return default_scope_->fastMemoryIntensiveStatNameLookup(name); + } + const SymbolTable& constSymbolTable() const override { return alloc_.constSymbolTable(); } SymbolTable& symbolTable() override { return alloc_.symbolTable(); } const TagProducer& tagProducer() const { return *tag_producer_; } @@ -221,11 +225,17 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo // We keep a TLS cache of rejected stat names. This costs memory, but // reduces runtime overhead running the matcher. Moreover, once symbol - // tables are integrated, rejection will need the fully elaborated string, - // and it we need to take a global symbol-table lock to run. We keep this - // StatName set here in the TLS cache to avoid taking a lock to compute - // rejection. + // tables are integrated, checking rejection without a cache would require + // the fully elaborated string, requiring acquisition of the global + // symbol-table lock. We keep this StatName set here in the TLS cache to + // avoid taking a lock to compute rejection. StatNameHashSet rejected_stats_; + + // We keep a TLS cache of names that can only be discovered on the hot-path, + // that we need to turn into stats. StringStatNameMap contains only + // references to separately maintained stats, and the backing store for this + // is in CentralCacheEntry.hot_path_stats_. + StringStatNameMap hot_path_stats_map_; }; struct CentralCacheEntry { @@ -233,6 +243,8 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo StatMap gauges_; StatMap histograms_; StatNameStorageSet rejected_stats_; + StatNameStorageSet hot_path_stats_; // Backing store for TlsCacheEntry.hot_path_stats_map_. + StringStatNameMap hot_path_stats_map_; }; struct ScopeImpl : public TlsScope { @@ -313,6 +325,8 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo std::unique_ptr& truncated_name_storage, std::vector& tags, std::string& tag_extracted_name); + StatName fastMemoryIntensiveStatNameLookup(absl::string_view name) override; + static std::atomic next_scope_id_; const uint64_t scope_id_; @@ -373,7 +387,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo std::vector deleted_gauges_; std::vector deleted_histograms_; - absl::flat_hash_set rejected_stats_purgatory_ GUARDED_BY(lock_); + absl::flat_hash_set stats_purgatory_ GUARDED_BY(lock_); }; } // namespace Stats diff --git a/test/common/stats/isolated_store_impl_test.cc b/test/common/stats/isolated_store_impl_test.cc index 186d73bf83e55..c010ca7e7dd88 100644 --- a/test/common/stats/isolated_store_impl_test.cc +++ b/test/common/stats/isolated_store_impl_test.cc @@ -138,6 +138,9 @@ TEST_F(StatsIsolatedStoreImplTest, AllWithSymbolTable) { EXPECT_EQ(4UL, store_.counters().size()); EXPECT_EQ(2UL, store_.gauges().size()); + + StatName fast_lookup = store_.fastMemoryIntensiveStatNameLookup("fast.lookup"); + EXPECT_EQ("fast.lookup", store_.symbolTable().toString(fast_lookup)); } TEST_F(StatsIsolatedStoreImplTest, ConstSymtabAccessor) { diff --git a/test/common/stats/symbol_table_impl_test.cc b/test/common/stats/symbol_table_impl_test.cc index a3ace4270941c..da8947ecd8171 100644 --- a/test/common/stats/symbol_table_impl_test.cc +++ b/test/common/stats/symbol_table_impl_test.cc @@ -537,6 +537,16 @@ TEST_P(StatNameTest, SharedStatNameStorageSetSwap) { set2.free(*table_); } +TEST_P(StatNameTest, StatNameStringMap) { + StatNameManagedStorage foo_bar("foo.bar", *table_); // Must outlive the map. + { + StringStatNameMap map; + EXPECT_FALSE(map.find("foo.bar", *table_)); + map.insert(foo_bar.statName(), *table_); + EXPECT_EQ(map.find("foo.bar", *table_), foo_bar.statName()); + } +} + // 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, diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index 52acdbb65ee8e..bc7d4cdef0eb3 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -221,6 +221,9 @@ TEST_F(StatsThreadLocalStoreTest, NoTls) { EXPECT_EQ(&g1, store_->gauges().front().get()); // front() ok when size()==1 EXPECT_EQ(2L, store_->gauges().front().use_count()); + StatName fast_lookup = store_->fastMemoryIntensiveStatNameLookup("fast.lookup"); + EXPECT_EQ("fast.lookup", symbol_table_.toString(fast_lookup)); + store_->shutdownThreading(); } @@ -264,6 +267,12 @@ TEST_F(StatsThreadLocalStoreTest, Tls) { EXPECT_EQ(&g1, store_->gauges().front().get()); // front() ok when size()==1 EXPECT_EQ(3L, store_->gauges().front().use_count()); + StatName fast_lookup = store_->fastMemoryIntensiveStatNameLookup("fast.lookup"); + // Request the same string so that we trigger the cache 'hit' case. + StatName fast_lookup2 = store_->fastMemoryIntensiveStatNameLookup("fast.lookup"); + EXPECT_EQ(fast_lookup, fast_lookup2); + EXPECT_EQ("fast.lookup", symbol_table_.toString(fast_lookup)); + store_->shutdownThreading(); tls_.shutdownThread(); diff --git a/test/integration/server.h b/test/integration/server.h index 6024db40e0d44..16148031c7297 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -126,6 +126,10 @@ class TestScopeWrapper : public Scope { } SymbolTable& symbolTable() override { return wrapped_scope_->symbolTable(); } + StatName fastMemoryIntensiveStatNameLookup(absl::string_view name) override { + return wrapped_scope_->fastMemoryIntensiveStatNameLookup(name); + } + private: Thread::MutexBasicLockable& lock_; ScopePtr wrapped_scope_; @@ -199,6 +203,11 @@ class TestIsolatedStoreImpl : public StoreRoot { return store_.histograms(); } + StatName fastMemoryIntensiveStatNameLookup(absl::string_view name) override { + Thread::LockGuard lock(lock_); + return store_.fastMemoryIntensiveStatNameLookup(name); + } + // Stats::StoreRoot void addSink(Sink&) override {} void setTagProducer(TagProducerPtr&&) override {} diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 3442a82648698..b1699ff9e414e 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -212,8 +212,9 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithStats) { // 2019/04/24 6161 49415 Pack tags and tag extracted names // 2019/05/07 6794 49957 Stats for excluded hosts in cluster // 2019/04/27 6733 50213 Use SymbolTable API for HTTP codes + // 2019/04/27 6733 50390 Add string -> stat-name cache in TLS. - EXPECT_EQ(m_per_cluster, 50213); + EXPECT_EQ(m_per_cluster, 50390); } } // namespace diff --git a/test/mocks/stats/mocks.cc b/test/mocks/stats/mocks.cc index 350ba468d5291..0e7abe4ec0e84 100644 --- a/test/mocks/stats/mocks.cc +++ b/test/mocks/stats/mocks.cc @@ -125,6 +125,17 @@ MockStore::MockStore() : StoreImpl(*fake_symbol_table_) { } MockStore::~MockStore() {} +StatName MockStore::fastMemoryIntensiveStatNameLookup(absl::string_view name) { + absl::optional stat_name = string_stat_name_map_.find(name, symbolTable()); + if (!stat_name) { + StatNameStorage storage(name, symbolTable()); + auto insertion = stat_name_set_.insert(std::move(storage)); + ASSERT(insertion.second); // If the name is not in the map, it should not be in the set. + stat_name = insertion.first->statName(); + } + return *stat_name; +} + MockIsolatedStatsStore::MockIsolatedStatsStore() : IsolatedStoreImpl(Test::Global::get()) {} MockIsolatedStatsStore::~MockIsolatedStatsStore() {} diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h index cc4ed56394983..25af3c053b729 100644 --- a/test/mocks/stats/mocks.h +++ b/test/mocks/stats/mocks.h @@ -205,9 +205,13 @@ class MockStore : public SymbolTableProvider, public StoreImpl { return histogram(symbol_table_->toString(name)); } + StatName fastMemoryIntensiveStatNameLookup(absl::string_view name) override; + Test::Global symbol_table_; testing::NiceMock counter_; std::vector> histograms_; + StatNameStorageSet stat_name_set_; + StringStatNameMap string_stat_name_map_; }; /**