diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2023-03-01 07:19:42 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2023-03-01 17:02:23 +0000 |
commit | 4ddd73423b5791ec69da4a65eb99fbeec5e62287 (patch) | |
tree | ad1c445725da9048be44f2c7fe1b40a0c310f84d /metrics/src | |
parent | 92d9f209733942dfb20db236712b88ecd3762091 (diff) |
No need to let ConsumerSpec be Printable.
Diffstat (limited to 'metrics/src')
-rw-r--r-- | metrics/src/tests/metricmanagertest.cpp | 8 | ||||
-rw-r--r-- | metrics/src/vespa/metrics/metricmanager.cpp | 25 | ||||
-rw-r--r-- | metrics/src/vespa/metrics/metricmanager.h | 14 |
3 files changed, 23 insertions, 24 deletions
diff --git a/metrics/src/tests/metricmanagertest.cpp b/metrics/src/tests/metricmanagertest.cpp index bee4ad350c9..8417a68d780 100644 --- a/metrics/src/tests/metricmanagertest.cpp +++ b/metrics/src/tests/metricmanagertest.cpp @@ -165,7 +165,7 @@ getMatchedMetrics(const vespalib::string& config) MetricLockGuard g(mm.getMetricLock()); mm.visit(g, mm.getActiveMetrics(g), visitor, "consumer"); - MetricManager::ConsumerSpec::SP consumerSpec(mm.getConsumerSpec(g, "consumer")); + const MetricManager::ConsumerSpec * consumerSpec = mm.getConsumerSpec(g, "consumer"); return { visitor.toString(), consumerSpec ? consumerSpec->toString() : "Non-existing consumer" }; } @@ -478,8 +478,7 @@ TEST_F(MetricManagerTest, test_snapshots) { MetricLockGuard lockGuard(mm.getMetricLock()); mm.visit(lockGuard, mm.getActiveMetrics(lockGuard), visitor, "snapper"); - MetricManager::ConsumerSpec::SP consumerSpec( - mm.getConsumerSpec(lockGuard, "snapper")); + const MetricManager::ConsumerSpec * consumerSpec = mm.getConsumerSpec(lockGuard, "snapper"); EXPECT_EQ(std::string("\n" "temp.val6\n" "temp.sub.val1\n" @@ -492,8 +491,7 @@ TEST_F(MetricManagerTest, test_snapshots) "*temp.multisub.sum.val1\n" "*temp.multisub.sum.val2\n" "*temp.multisub.sum.valsum\n"), - "\n" + visitor.toString()) << (consumerSpec.get() ? consumerSpec->toString() - : "Non-existing consumer"); + "\n" + visitor.toString()) << (consumerSpec ? consumerSpec->toString() : "Non-existing consumer"); } // Initially, there should be no metrics logged ASSERT_PROCESS_TIME(mm, 1000); diff --git a/metrics/src/vespa/metrics/metricmanager.cpp b/metrics/src/vespa/metrics/metricmanager.cpp index ff0b051990f..52226e32e19 100644 --- a/metrics/src/vespa/metrics/metricmanager.cpp +++ b/metrics/src/vespa/metrics/metricmanager.cpp @@ -44,19 +44,20 @@ MetricManager::assertMetricLockLocked(const MetricLockGuard& g) const { } } -void -MetricManager::ConsumerSpec::print(std::ostream& out, bool verbose, const std::string& indent) const +vespalib::string +MetricManager::ConsumerSpec::toString() const { - (void) verbose; + vespalib::asciistream out; out << "ConsumerSpec("; std::set<Metric::String> sortedMetrics; for (const Metric::String & name : includedMetrics) { sortedMetrics.insert(name); } for (const auto & s : sortedMetrics) { - out << "\n" << indent << " " << s; + out << "\n" << " " << s; } out << ")"; + return out.str(); } void @@ -379,12 +380,12 @@ MetricManager::handleMetricsAltered(const MetricLockGuard & guard) LOG(info, "Metrics registration changes detected. Handling changes."); } _activeMetrics.getMetrics().clearRegistrationAltered(); - std::map<Metric::String, ConsumerSpec::SP> configMap; + std::map<Metric::String, ConsumerSpec> configMap; LOG(debug, "Calculating new consumer config"); for (const auto & consumer : _config->consumer) { ConsumerMetricBuilder consumerMetricBuilder(consumer); _activeMetrics.getMetrics().visit(consumerMetricBuilder); - configMap[consumer.name] = std::make_shared<ConsumerSpec>(std::move(consumerMetricBuilder._matchedMetrics)); + configMap[consumer.name] = ConsumerSpec(std::move(consumerMetricBuilder._matchedMetrics)); } LOG(debug, "Recreating snapshots to include altered metrics"); _totalMetrics->recreateSnapshot(_activeMetrics.getMetrics(), _snapshotUnsetMetrics); @@ -500,11 +501,11 @@ MetricManager::configure(const MetricLockGuard & , std::unique_ptr<Config> confi } -MetricManager::ConsumerSpec::SP +const MetricManager::ConsumerSpec * MetricManager::getConsumerSpec(const MetricLockGuard &, const Metric::String& consumer) const { auto it(_consumerConfig.find(consumer)); - return (it != _consumerConfig.end() ? it->second : ConsumerSpec::SP()); + return (it != _consumerConfig.end() ? &it->second : nullptr); } //#define VERIFY_ALL_METRICS_VISITED 1 @@ -565,8 +566,8 @@ MetricManager::visit(const MetricLockGuard & guard, const MetricSnapshot& snapsh if (consumer == "") { snapshot.getMetrics().visit(visitor); } else { - ConsumerSpec::SP consumerSpec(getConsumerSpec(guard, consumer)); - if (consumerSpec.get()) { + const ConsumerSpec * consumerSpec = getConsumerSpec(guard, consumer); + if (consumerSpec) { ConsumerMetricVisitor consumerVis(*consumerSpec, visitor); snapshot.getMetrics().visit(consumerVis); #ifdef VERIFY_ALL_METRICS_VISITED @@ -844,10 +845,10 @@ MetricManager::getMemoryConsumption(const MetricLockGuard & guard) const assertMetricLockLocked(guard); auto mc = std::make_unique<MemoryConsumption>(); mc->_consumerCount += _consumerConfig.size(); - mc->_consumerMeta += (sizeof(ConsumerSpec::SP) + sizeof(ConsumerSpec)) * _consumerConfig.size(); + mc->_consumerMeta += sizeof(ConsumerSpec) * _consumerConfig.size(); for (const auto & consumer : _consumerConfig) { mc->_consumerId += mc->getStringMemoryUsage(consumer.first, mc->_consumerIdUnique) + sizeof(Metric::String); - consumer.second->addMemoryUsage(*mc); + consumer.second.addMemoryUsage(*mc); } uint32_t preTotal = mc->getTotalMemoryUsage(); _activeMetrics.addMemoryUsage(*mc); diff --git a/metrics/src/vespa/metrics/metricmanager.h b/metrics/src/vespa/metrics/metricmanager.h index 70c93f6be2b..82a6523b18b 100644 --- a/metrics/src/vespa/metrics/metricmanager.h +++ b/metrics/src/vespa/metrics/metricmanager.h @@ -73,20 +73,19 @@ public: * Spec saved from config. If metricSetChildren has content, metric pointed * to is a metric set. */ - struct ConsumerSpec : public vespalib::Printable { - using SP = std::shared_ptr<ConsumerSpec>; - + struct ConsumerSpec { vespalib::hash_set<Metric::String> includedMetrics; + ConsumerSpec(ConsumerSpec &&) noexcept = default; ConsumerSpec & operator= (ConsumerSpec &&) noexcept = default; ConsumerSpec(); - ~ConsumerSpec() override; + ~ConsumerSpec(); bool contains(const Metric& m) const { return (includedMetrics.find(m.getPath()) != includedMetrics.end()); } - void print(std::ostream& out, bool verbose, const std::string& indent) const override; + vespalib::string toString() const; void addMemoryUsage(MemoryConsumption&) const; }; @@ -96,7 +95,7 @@ private: std::unique_ptr<config::ConfigSubscriber> _configSubscriber; std::unique_ptr<config::ConfigHandle<MetricsmanagerConfig>> _configHandle; std::unique_ptr<MetricsmanagerConfig> _config; - std::map<Metric::String, ConsumerSpec::SP> _consumerConfig; + std::map<Metric::String, ConsumerSpec> _consumerConfig; std::list<UpdateHook*> _periodicUpdateHooks; std::list<UpdateHook*> _snapshotUpdateHooks; mutable std::mutex _waiter; @@ -246,7 +245,8 @@ public: std::vector<uint32_t> getSnapshotPeriods(const MetricLockGuard& l) const; - ConsumerSpec::SP getConsumerSpec(const MetricLockGuard & guard, const Metric::String& consumer) const; + // Public only for testing. The returned pointer is only valid while holding the lock. + const ConsumerSpec * getConsumerSpec(const MetricLockGuard & guard, const Metric::String& consumer) const; /** * If you add or remove metrics from the active metric sets, normally, |