No need to let ConsumerSpec be Printable.

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,