diff --git a/source/common/stats/allocator.h b/source/common/stats/allocator.h index 37aed8de6e1ae..9bc5f38eff22f 100644 --- a/source/common/stats/allocator.h +++ b/source/common/stats/allocator.h @@ -24,6 +24,20 @@ class Allocator { Allocator(SymbolTable& symbol_table) : symbol_table_(symbol_table) {} virtual ~Allocator(); +private: + // We use a private section for makeCounter, makeGauge, and makeTextReadout. + // We do not want general filter code to be creating stats in this way, they + // must go through the Scope interface. + // + // This allows the MetricSnapshotImpl to hold onto references to the stats + // holding onto references to the Scopes, rather than holding onto each + // individual stat. In other words, all the stats must be owned by one or + // more scopes. + // + // The alternative is to have that function hold onto reference to every stat, + // which requires incrementing and decrementing reference counts to each stat, + // which is very slow, particularly on ARM processors. + /** * @param name the full name of the stat. * @param tag_extracted_name the name of the stat with tag-values stripped out. @@ -50,6 +64,8 @@ class Allocator { */ TextReadoutSharedPtr makeTextReadout(StatName name, StatName tag_extracted_name, const StatNameTagVector& stat_name_tags); + +public: SymbolTable& symbolTable() { return symbol_table_; } const SymbolTable& constSymbolTable() const { return symbol_table_; } @@ -111,9 +127,13 @@ class Allocator { private: template friend class StatsSharedImpl; + friend class AllocatorTest; friend class CounterImpl; friend class GaugeImpl; + friend class IsolatedStoreImpl; + friend class MetricImplTest; friend class TextReadoutImpl; + friend class ThreadLocalStoreImpl; // A mutex is needed here to protect both the stats_ object from both // alloc() and free() operations. Although alloc() operations are called under existing locking, diff --git a/source/server/server.cc b/source/server/server.cc index 3722f644356e3..d2dd39a222324 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -171,45 +171,26 @@ void InstanceBase::failHealthcheck(bool fail) { MetricSnapshotImpl::MetricSnapshotImpl(Stats::Store& store, Upstream::ClusterManager& cluster_manager, TimeSource& time_source) { + // Capture references to all the scopes in this, which will in turn keep all + // of the contained counters, gauges, text readouts, and histograms live for + // the duration of the MetricsSnapshot. + store.forEachScope( + [this](std::size_t size) { scopes_.reserve(size); }, + [this](const Stats::Scope& scope) { scopes_.push_back(scope.getConstShared()); }); store.forEachSinkedCounter( - [this](std::size_t size) { - snapped_counters_.reserve(size); - counters_.reserve(size); - }, - [this](Stats::Counter& counter) { - snapped_counters_.push_back(Stats::CounterSharedPtr(&counter)); - counters_.push_back({counter.latch(), counter}); - }); + [this](std::size_t size) { counters_.reserve(size); }, + [this](Stats::Counter& counter) { counters_.push_back({counter.latch(), counter}); }); - store.forEachSinkedGauge( - [this](std::size_t size) { - snapped_gauges_.reserve(size); - gauges_.reserve(size); - }, - [this](Stats::Gauge& gauge) { - snapped_gauges_.push_back(Stats::GaugeSharedPtr(&gauge)); - gauges_.push_back(gauge); - }); + store.forEachSinkedGauge([this](std::size_t size) { gauges_.reserve(size); }, + [this](Stats::Gauge& gauge) { gauges_.push_back(gauge); }); store.forEachSinkedHistogram( - [this](std::size_t size) { - snapped_histograms_.reserve(size); - histograms_.reserve(size); - }, - [this](Stats::ParentHistogram& histogram) { - snapped_histograms_.push_back(Stats::ParentHistogramSharedPtr(&histogram)); - histograms_.push_back(histogram); - }); + [this](std::size_t size) { histograms_.reserve(size); }, + [this](Stats::ParentHistogram& histogram) { histograms_.push_back(histogram); }); store.forEachSinkedTextReadout( - [this](std::size_t size) { - snapped_text_readouts_.reserve(size); - text_readouts_.reserve(size); - }, - [this](Stats::TextReadout& text_readout) { - snapped_text_readouts_.push_back(Stats::TextReadoutSharedPtr(&text_readout)); - text_readouts_.push_back(text_readout); - }); + [this](std::size_t size) { text_readouts_.reserve(size); }, + [this](Stats::TextReadout& text_readout) { text_readouts_.push_back(text_readout); }); Upstream::HostUtility::forEachHostMetric( cluster_manager, diff --git a/source/server/server.h b/source/server/server.h index f18f3d0c028d7..a2d826d152b45 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -491,13 +491,10 @@ class MetricSnapshotImpl : public Stats::MetricSnapshot { SystemTime snapshotTime() const override { return snapshot_time_; } private: - std::vector snapped_counters_; + std::vector scopes_; std::vector counters_; - std::vector snapped_gauges_; std::vector> gauges_; - std::vector snapped_histograms_; std::vector> histograms_; - std::vector snapped_text_readouts_; std::vector> text_readouts_; std::vector host_counters_; std::vector host_gauges_; diff --git a/test/common/stats/allocator_test.cc b/test/common/stats/allocator_test.cc index 42ba717ab43ad..6723e165f324c 100644 --- a/test/common/stats/allocator_test.cc +++ b/test/common/stats/allocator_test.cc @@ -16,7 +16,6 @@ namespace Envoy { namespace Stats { -namespace { class AllocatorTest : public testing::Test { protected: @@ -37,6 +36,25 @@ class AllocatorTest : public testing::Test { } } + // Wrappers for Allocator stats construction methods, held in the test fixture + // so they can be friended by AllocatorImpl. Those methods are protected in + // order to prevent filters from creating stats directly on the allocator; + // they should exclusively be managed by Scopes. + CounterSharedPtr makeCounter(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags) { + return alloc_.makeCounter(name, tag_extracted_name, stat_name_tags); + } + + GaugeSharedPtr makeGauge(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags, Gauge::ImportMode import) { + return alloc_.makeGauge(name, tag_extracted_name, stat_name_tags, import); + } + + TextReadoutSharedPtr makeTextReadout(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags) { + return alloc_.makeTextReadout(name, tag_extracted_name, stat_name_tags); + } + SymbolTableImpl symbol_table_; // Declare the pool before the allocator because the allocator could contain // a TestSinkPredicates object whose lifetime should be bounded by that of the pool. @@ -48,9 +66,9 @@ class AllocatorTest : public testing::Test { // Allocate 2 counters of the same name, and you'll get the same object. TEST_F(AllocatorTest, CountersWithSameName) { StatName counter_name = makeStat("counter.name"); - CounterSharedPtr c1 = alloc_.makeCounter(counter_name, StatName(), {}); + CounterSharedPtr c1 = makeCounter(counter_name, StatName(), {}); EXPECT_EQ(1, c1->use_count()); - CounterSharedPtr c2 = alloc_.makeCounter(counter_name, StatName(), {}); + CounterSharedPtr c2 = makeCounter(counter_name, StatName(), {}); EXPECT_EQ(2, c1->use_count()); EXPECT_EQ(2, c2->use_count()); EXPECT_EQ(c1.get(), c2.get()); @@ -66,9 +84,9 @@ TEST_F(AllocatorTest, CountersWithSameName) { TEST_F(AllocatorTest, GaugesWithSameName) { StatName gauge_name = makeStat("gauges.name"); - GaugeSharedPtr g1 = alloc_.makeGauge(gauge_name, StatName(), {}, Gauge::ImportMode::Accumulate); + GaugeSharedPtr g1 = makeGauge(gauge_name, StatName(), {}, Gauge::ImportMode::Accumulate); EXPECT_EQ(1, g1->use_count()); - GaugeSharedPtr g2 = alloc_.makeGauge(gauge_name, StatName(), {}, Gauge::ImportMode::Accumulate); + GaugeSharedPtr g2 = makeGauge(gauge_name, StatName(), {}, Gauge::ImportMode::Accumulate); EXPECT_EQ(2, g1->use_count()); EXPECT_EQ(2, g2->use_count()); EXPECT_EQ(g1.get(), g2.get()); @@ -101,8 +119,8 @@ TEST_F(AllocatorTest, RefCountDecAllocRaceOrganic) { threads.push_back(thread_factory.createThread([&]() { go.WaitForNotification(); for (uint32_t i = 0; i < iters; ++i) { - alloc_.makeCounter(counter_name, StatName(), {}); - alloc_.makeGauge(gauge_name, StatName(), {}, Gauge::ImportMode::NeverImport); + makeCounter(counter_name, StatName(), {}); + makeGauge(gauge_name, StatName(), {}, Gauge::ImportMode::NeverImport); } })); } @@ -125,7 +143,7 @@ TEST_F(AllocatorTest, RefCountDecAllocRaceSynchronized) { alloc_.sync().enable(); alloc_.sync().waitOn(Allocator::DecrementToZeroSyncPoint); Thread::ThreadPtr thread = thread_factory.createThread([&]() { - CounterSharedPtr counter = alloc_.makeCounter(counter_name, StatName(), {}); + CounterSharedPtr counter = makeCounter(counter_name, StatName(), {}); counter->inc(); counter->reset(); // Blocks in thread synchronizer waiting on DecrementToZeroSyncPoint }); @@ -139,17 +157,17 @@ TEST_F(AllocatorTest, RefCountDecAllocRaceSynchronized) { TEST_F(AllocatorTest, HiddenGauge) { GaugeSharedPtr hidden_gauge = - alloc_.makeGauge(makeStat("hidden"), StatName(), {}, Gauge::ImportMode::HiddenAccumulate); + makeGauge(makeStat("hidden"), StatName(), {}, Gauge::ImportMode::HiddenAccumulate); EXPECT_EQ(hidden_gauge->importMode(), Gauge::ImportMode::HiddenAccumulate); EXPECT_TRUE(hidden_gauge->hidden()); GaugeSharedPtr non_hidden_gauge = - alloc_.makeGauge(makeStat("non_hidden"), StatName(), {}, Gauge::ImportMode::Accumulate); + makeGauge(makeStat("non_hidden"), StatName(), {}, Gauge::ImportMode::Accumulate); EXPECT_NE(non_hidden_gauge->importMode(), Gauge::ImportMode::HiddenAccumulate); EXPECT_FALSE(non_hidden_gauge->hidden()); - GaugeSharedPtr never_import_hidden_gauge = alloc_.makeGauge( - makeStat("never_import_hidden"), StatName(), {}, Gauge::ImportMode::NeverImport); + GaugeSharedPtr never_import_hidden_gauge = + makeGauge(makeStat("never_import_hidden"), StatName(), {}, Gauge::ImportMode::NeverImport); EXPECT_NE(never_import_hidden_gauge->importMode(), Gauge::ImportMode::HiddenAccumulate); EXPECT_FALSE(never_import_hidden_gauge->hidden()); } @@ -163,7 +181,7 @@ TEST_F(AllocatorTest, ForEachCounter) { for (size_t idx = 0; idx < num_stats; ++idx) { auto stat_name = makeStat(absl::StrCat("counter.", idx)); stat_names.insert(stat_name); - counters.emplace_back(alloc_.makeCounter(stat_name, StatName(), {})); + counters.emplace_back(makeCounter(stat_name, StatName(), {})); } size_t num_counters = 0; @@ -216,7 +234,7 @@ TEST_F(AllocatorTest, ForEachGauge) { for (size_t idx = 0; idx < num_stats; ++idx) { auto stat_name = makeStat(absl::StrCat("gauge.", idx)); stat_names.insert(stat_name); - gauges.emplace_back(alloc_.makeGauge(stat_name, StatName(), {}, Gauge::ImportMode::Accumulate)); + gauges.emplace_back(makeGauge(stat_name, StatName(), {}, Gauge::ImportMode::Accumulate)); } size_t num_gauges = 0; @@ -269,7 +287,7 @@ TEST_F(AllocatorTest, ForEachTextReadout) { for (size_t idx = 0; idx < num_stats; ++idx) { auto stat_name = makeStat(absl::StrCat("text_readout.", idx)); stat_names.insert(stat_name); - text_readouts.emplace_back(alloc_.makeTextReadout(stat_name, StatName(), {})); + text_readouts.emplace_back(makeTextReadout(stat_name, StatName(), {})); } size_t num_text_readouts = 0; @@ -326,7 +344,7 @@ TEST_F(AllocatorTest, ForEachWithNullSizeLambda) { // For each counter. for (size_t idx = 0; idx < num_stats; ++idx) { auto stat_name = makeStat(absl::StrCat("counter.", idx)); - counters.emplace_back(alloc_.makeCounter(stat_name, StatName(), {})); + counters.emplace_back(makeCounter(stat_name, StatName(), {})); } size_t num_iterations = 0; alloc_.forEachCounter(nullptr, [&num_iterations](Counter& counter) { @@ -338,7 +356,7 @@ TEST_F(AllocatorTest, ForEachWithNullSizeLambda) { // For each gauge. for (size_t idx = 0; idx < num_stats; ++idx) { auto stat_name = makeStat(absl::StrCat("gauge.", idx)); - gauges.emplace_back(alloc_.makeGauge(stat_name, StatName(), {}, Gauge::ImportMode::Accumulate)); + gauges.emplace_back(makeGauge(stat_name, StatName(), {}, Gauge::ImportMode::Accumulate)); } num_iterations = 0; alloc_.forEachGauge(nullptr, [&num_iterations](Gauge& gauge) { @@ -350,7 +368,7 @@ TEST_F(AllocatorTest, ForEachWithNullSizeLambda) { // For each text readout. for (size_t idx = 0; idx < num_stats; ++idx) { auto stat_name = makeStat(absl::StrCat("text_readout.", idx)); - text_readouts.emplace_back(alloc_.makeTextReadout(stat_name, StatName(), {})); + text_readouts.emplace_back(makeTextReadout(stat_name, StatName(), {})); } num_iterations = 0; alloc_.forEachTextReadout(nullptr, [&num_iterations](TextReadout& text_readout) { @@ -370,7 +388,7 @@ TEST_F(AllocatorTest, AskForDeletedStat) { std::vector counters; for (size_t idx = 0; idx < num_stats; ++idx) { auto stat_name = makeStat(absl::StrCat("counter.", idx)); - counters.emplace_back(alloc_.makeCounter(stat_name, StatName(), {})); + counters.emplace_back(makeCounter(stat_name, StatName(), {})); } // Reject a stat and remove it from "scope". StatName const rejected_counter_name = counters[4]->statName(); @@ -383,7 +401,7 @@ TEST_F(AllocatorTest, AskForDeletedStat) { rejected_counter.inc(); // Make the deleted stat again. - CounterSharedPtr deleted_counter = alloc_.makeCounter(rejected_counter_name, StatName(), {}); + CounterSharedPtr deleted_counter = makeCounter(rejected_counter_name, StatName(), {}); EXPECT_EQ(deleted_counter->value(), 0); EXPECT_EQ(rejected_counter.value(), 2); @@ -391,7 +409,7 @@ TEST_F(AllocatorTest, AskForDeletedStat) { std::vector gauges; for (size_t idx = 0; idx < num_stats; ++idx) { auto stat_name = makeStat(absl::StrCat("gauge.", idx)); - gauges.emplace_back(alloc_.makeGauge(stat_name, StatName(), {}, Gauge::ImportMode::Accumulate)); + gauges.emplace_back(makeGauge(stat_name, StatName(), {}, Gauge::ImportMode::Accumulate)); } // Reject a stat and remove it from "scope". StatName const rejected_gauge_name = gauges[4]->statName(); @@ -404,7 +422,7 @@ TEST_F(AllocatorTest, AskForDeletedStat) { // Make the deleted stat again. GaugeSharedPtr deleted_gauge = - alloc_.makeGauge(rejected_gauge_name, StatName(), {}, Gauge::ImportMode::Accumulate); + makeGauge(rejected_gauge_name, StatName(), {}, Gauge::ImportMode::Accumulate); EXPECT_EQ(deleted_gauge->value(), 0); EXPECT_EQ(rejected_gauge.value(), 10); @@ -412,7 +430,7 @@ TEST_F(AllocatorTest, AskForDeletedStat) { std::vector text_readouts; for (size_t idx = 0; idx < num_stats; ++idx) { auto stat_name = makeStat(absl::StrCat("text_readout.", idx)); - text_readouts.emplace_back(alloc_.makeTextReadout(stat_name, StatName(), {})); + text_readouts.emplace_back(makeTextReadout(stat_name, StatName(), {})); } // Reject a stat and remove it from "scope". StatName const rejected_text_readout_name = text_readouts[4]->statName(); @@ -425,7 +443,7 @@ TEST_F(AllocatorTest, AskForDeletedStat) { // Make the deleted stat again. TextReadoutSharedPtr deleted_text_readout = - alloc_.makeTextReadout(rejected_text_readout_name, StatName(), {}); + makeTextReadout(rejected_text_readout_name, StatName(), {}); EXPECT_EQ(deleted_text_readout->value(), ""); EXPECT_EQ(rejected_text_readout.value(), "deleted value"); @@ -447,9 +465,9 @@ TEST_F(AllocatorTest, ForEachSinkedCounter) { // sink every 3rd stat if ((idx + 1) % 3 == 0) { sink_predicates->add(stat_name); - sinked_counters.emplace_back(alloc_.makeCounter(stat_name, StatName(), {})); + sinked_counters.emplace_back(makeCounter(stat_name, StatName(), {})); } else { - unsinked_counters.emplace_back(alloc_.makeCounter(stat_name, StatName(), {})); + unsinked_counters.emplace_back(makeCounter(stat_name, StatName(), {})); } } @@ -493,10 +511,10 @@ TEST_F(AllocatorTest, ForEachSinkedGauge) { if ((idx + 1) % 5 == 0) { sink_predicates->add(stat_name); sinked_gauges.emplace_back( - alloc_.makeGauge(stat_name, StatName(), {}, Gauge::ImportMode::Accumulate)); + makeGauge(stat_name, StatName(), {}, Gauge::ImportMode::Accumulate)); } else { unsinked_gauges.emplace_back( - alloc_.makeGauge(stat_name, StatName(), {}, Gauge::ImportMode::Accumulate)); + makeGauge(stat_name, StatName(), {}, Gauge::ImportMode::Accumulate)); } } @@ -532,11 +550,9 @@ TEST_F(AllocatorTest, ForEachSinkedGaugeHidden) { size_t num_gauges = 0; size_t num_iterations = 0; - unhidden_gauge = - alloc_.makeGauge(unhidden_stat_name, StatName(), {}, Gauge::ImportMode::Accumulate); + unhidden_gauge = makeGauge(unhidden_stat_name, StatName(), {}, Gauge::ImportMode::Accumulate); - hidden_gauge = - alloc_.makeGauge(hidden_stat_name, StatName(), {}, Gauge::ImportMode::HiddenAccumulate); + hidden_gauge = makeGauge(hidden_stat_name, StatName(), {}, Gauge::ImportMode::HiddenAccumulate); alloc_.forEachSinkedGauge([&num_gauges](std::size_t size) { num_gauges = size; }, [&num_iterations, unhidden_stat_name](Gauge& gauge) { @@ -565,11 +581,9 @@ TEST_F(AllocatorTest, ForEachSinkedGaugeHiddenPredicate) { size_t num_gauges = 0; size_t num_iterations = 0; - unhidden_gauge = - alloc_.makeGauge(unhidden_stat_name, StatName(), {}, Gauge::ImportMode::Accumulate); + unhidden_gauge = makeGauge(unhidden_stat_name, StatName(), {}, Gauge::ImportMode::Accumulate); - hidden_gauge = - alloc_.makeGauge(hidden_stat_name, StatName(), {}, Gauge::ImportMode::HiddenAccumulate); + hidden_gauge = makeGauge(hidden_stat_name, StatName(), {}, Gauge::ImportMode::HiddenAccumulate); alloc_.forEachSinkedGauge([&num_gauges](std::size_t size) { num_gauges = size; }, [&num_iterations, &sink_predicates](Gauge& gauge) { @@ -596,9 +610,9 @@ TEST_F(AllocatorTest, ForEachSinkedTextReadout) { // sink every 2nd stat if ((idx + 1) % 2 == 0) { sink_predicates->add(stat_name); - sinked_text_readouts.emplace_back(alloc_.makeTextReadout(stat_name, StatName(), {})); + sinked_text_readouts.emplace_back(makeTextReadout(stat_name, StatName(), {})); } else { - unsinked_text_readouts.emplace_back(alloc_.makeTextReadout(stat_name, StatName(), {})); + unsinked_text_readouts.emplace_back(makeTextReadout(stat_name, StatName(), {})); } } @@ -626,6 +640,5 @@ TEST_F(AllocatorTest, ForEachSinkedTextReadout) { EXPECT_EQ(num_iterations, 0); } -} // namespace } // namespace Stats } // namespace Envoy diff --git a/test/common/stats/metric_impl_test.cc b/test/common/stats/metric_impl_test.cc index 2b7268d65b210..d7ac4c400ffb5 100644 --- a/test/common/stats/metric_impl_test.cc +++ b/test/common/stats/metric_impl_test.cc @@ -10,7 +10,6 @@ namespace Envoy { namespace Stats { -namespace { class MetricImplTest : public testing::Test { protected: @@ -24,6 +23,11 @@ class MetricImplTest : public testing::Test { EXPECT_EQ(0, symbol_table_.numSymbols()); } + CounterSharedPtr makeCounter(StatName name, StatName tag_extracted_name, + const StatNameTagVector& stat_name_tags) { + return alloc_.makeCounter(name, tag_extracted_name, stat_name_tags); + } + SymbolTableImpl symbol_table_; Allocator alloc_; StatNamePool pool_; @@ -31,13 +35,13 @@ class MetricImplTest : public testing::Test { // No truncation occurs in the implementation of HeapStatData. TEST_F(MetricImplTest, NoTags) { - CounterSharedPtr counter = alloc_.makeCounter(makeStat("counter"), StatName(), {}); + CounterSharedPtr counter = makeCounter(makeStat("counter"), StatName(), {}); EXPECT_EQ(0, counter->tags().size()); } TEST_F(MetricImplTest, OneTag) { - CounterSharedPtr counter = alloc_.makeCounter(makeStat("counter.name.value"), makeStat("counter"), - {{makeStat("name"), makeStat("value")}}); + CounterSharedPtr counter = makeCounter(makeStat("counter.name.value"), makeStat("counter"), + {{makeStat("name"), makeStat("value")}}); TagVector tags = counter->tags(); ASSERT_EQ(1, tags.size()); EXPECT_EQ("name", tags[0].name_); @@ -48,7 +52,7 @@ TEST_F(MetricImplTest, OneTag) { } TEST_F(MetricImplTest, TwoTagsIterOnce) { - CounterSharedPtr counter = alloc_.makeCounter( + CounterSharedPtr counter = makeCounter( makeStat("counter.name.value"), makeStat("counter"), {{makeStat("name1"), makeStat("value1")}, {makeStat("name2"), makeStat("value2")}}); StatName name1 = makeStat("name1"); @@ -64,7 +68,7 @@ TEST_F(MetricImplTest, TwoTagsIterOnce) { } TEST_F(MetricImplTest, FindTag) { - CounterSharedPtr counter = alloc_.makeCounter( + CounterSharedPtr counter = makeCounter( makeStat("counter.name.value"), makeStat("counter"), {{makeStat("name1"), makeStat("value1")}, {makeStat("name2"), makeStat("value2")}}); EXPECT_EQ(makeStat("value1"), Utility::findTag(*counter, makeStat("name1"))); @@ -84,6 +88,5 @@ TEST(GaugeHelperTest, AdjustSubtractsWhenSubIsBigger) { gauge.adjust(2, 5); } -} // namespace } // namespace Stats } // namespace Envoy diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index ca73948af9e0b..5d73454b52cb1 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -2407,6 +2407,11 @@ class ClusterShutdownCleanupStarvationTest : public ThreadLocalRealThreadsMixin, // empty callback to main to ensure that cross-scope thread cleanups complete. // In this test, we don't expect the use-count of the stat to get very high. TEST_F(ClusterShutdownCleanupStarvationTest, TwelveThreadsWithBlockade) { + Counter* counter = nullptr; + runOnMainBlocking([this, &counter]() { + counter = &store_->rootScope()->counterFromStatName(my_counter_scoped_name_); + }); + for (uint32_t i = 0; i < NumIters && elapsedTime() < std::chrono::seconds(5); ++i) { createScopesIncCountersAndCleanupAllThreads(); @@ -2420,8 +2425,6 @@ TEST_F(ClusterShutdownCleanupStarvationTest, TwelveThreadsWithBlockade) { mainDispatchBlock(); // Here we show that the counter cleanups have finished, because the use-count is 1. - CounterSharedPtr counter = - alloc_.makeCounter(my_counter_scoped_name_, StatName(), StatNameTagVector{}); EXPECT_EQ(1, counter->use_count()) << "index=" << i; } } @@ -2433,6 +2436,12 @@ TEST_F(ClusterShutdownCleanupStarvationTest, TwelveThreadsWithBlockade) { TEST_F(ClusterShutdownCleanupStarvationTest, TwelveThreadsWithoutBlockade) { store_->sync().enable(); store_->sync().waitOn(ThreadLocalStoreImpl::MainDispatcherCleanupSync); + + Counter* counter = nullptr; + runOnMainBlocking([this, &counter]() { + counter = &store_->rootScope()->counterFromStatName(my_counter_scoped_name_); + }); + for (uint32_t i = 0; i < NumIters && elapsedTime() < std::chrono::seconds(5); ++i) { createScopesIncCountersAndCleanupAllThreads(); // As we have blocked the main dispatcher cleanup function above, nothing @@ -2445,8 +2454,6 @@ TEST_F(ClusterShutdownCleanupStarvationTest, TwelveThreadsWithoutBlockade) { // running the test: NumScopes*NumThreads*NumIters == 70000, We use a timer // so we don't time out on asan/tsan tests, In opt builds this test takes // less than a second, and in fastbuild it takes less than 5. - CounterSharedPtr counter = - alloc_.makeCounter(my_counter_scoped_name_, StatName(), StatNameTagVector{}); uint32_t use_count = counter->use_count() - 1; // Subtract off this instance. EXPECT_EQ((i + 1) * NumScopes * NumThreads, use_count); } diff --git a/test/server/admin/prometheus_stats_test.cc b/test/server/admin/prometheus_stats_test.cc index a9814baf25397..2f43bb3fcc120 100644 --- a/test/server/admin/prometheus_stats_test.cc +++ b/test/server/admin/prometheus_stats_test.cc @@ -54,32 +54,26 @@ class HistogramWrapper { class PrometheusStatsFormatterTest : public testing::Test { protected: PrometheusStatsFormatterTest() - : alloc_(*symbol_table_), pool_(*symbol_table_), + : alloc_(*symbol_table_), store_(alloc_), scope_(store_.createScope("")), + pool_(*symbol_table_), endpoints_helper_(std::make_unique()) {} - ~PrometheusStatsFormatterTest() override { clearStorage(); } + ~PrometheusStatsFormatterTest() override { scope_.reset(); } void addCounter(const std::string& name, Stats::StatNameTagVector cluster_tags) { - Stats::StatNameManagedStorage name_storage(baseName(name, cluster_tags), *symbol_table_); - Stats::StatNameManagedStorage tag_extracted_name_storage(name, *symbol_table_); - counters_.push_back(alloc_.makeCounter(name_storage.statName(), - tag_extracted_name_storage.statName(), cluster_tags)); + counters_.push_back(&scope_->counterFromStatNameWithTags(pool_.add(name), cluster_tags)); } void addGauge(const std::string& name, Stats::StatNameTagVector cluster_tags, Stats::Gauge::ImportMode import_mode = Stats::Gauge::ImportMode::Accumulate) { - Stats::StatNameManagedStorage name_storage(baseName(name, cluster_tags), *symbol_table_); - Stats::StatNameManagedStorage tag_extracted_name_storage(name, *symbol_table_); - gauges_.push_back(alloc_.makeGauge( - name_storage.statName(), tag_extracted_name_storage.statName(), cluster_tags, import_mode)); + gauges_.push_back( + &scope_->gaugeFromStatNameWithTags(pool_.add(name), cluster_tags, import_mode)); } void addTextReadout(const std::string& name, const std::string& value, Stats::StatNameTagVector cluster_tags) { - Stats::StatNameManagedStorage name_storage(baseName(name, cluster_tags), *symbol_table_); - Stats::StatNameManagedStorage tag_extracted_name_storage(name, *symbol_table_); - Stats::TextReadoutSharedPtr textReadout = alloc_.makeTextReadout( - name_storage.statName(), tag_extracted_name_storage.statName(), cluster_tags); + Stats::TextReadoutSharedPtr textReadout = + &scope_->textReadoutFromStatNameWithTags(pool_.add(name), cluster_tags); textReadout->set(value); textReadouts_.push_back(textReadout); } @@ -124,6 +118,7 @@ class PrometheusStatsFormatterTest : public testing::Test { void clearStorage() { pool_.clear(); + scope_.reset(); counters_.clear(); gauges_.clear(); histograms_.clear(); @@ -134,6 +129,8 @@ class PrometheusStatsFormatterTest : public testing::Test { Stats::TestUtil::TestSymbolTable symbol_table_; Stats::Allocator alloc_; + Stats::ThreadLocalStoreImpl store_; + Stats::ScopeSharedPtr scope_; Stats::StatNamePool pool_; std::vector counters_; std::vector gauges_; diff --git a/test/server/server_stats_flush_benchmark_test.cc b/test/server/server_stats_flush_benchmark_test.cc index c95854d1f9118..6155627d12c7f 100644 --- a/test/server/server_stats_flush_benchmark_test.cc +++ b/test/server/server_stats_flush_benchmark_test.cc @@ -118,10 +118,13 @@ static void bmFlushToSinksWithPredicatesSet(::benchmark::State& state) { speed_test.test(state); } -BENCHMARK(bmFlushToSinks)->Unit(::benchmark::kMillisecond)->RangeMultiplier(10)->Range(10, 1000000); +BENCHMARK(bmFlushToSinks) + ->Unit(::benchmark::kMillisecond) + ->RangeMultiplier(10) + ->Range(10, 10000000); BENCHMARK(bmFlushToSinksWithPredicatesSet) ->Unit(::benchmark::kMillisecond) ->RangeMultiplier(10) - ->Range(10, 1000000); + ->Range(10, 10000000); } // namespace Envoy