Merge pull request #7227 from vespa-engine/arnej/use-static-metric-name-repo-in-metrics

Arnej/use static metric name repo in metrics

19 files changed, 190 insertions, 112 deletions

diff --git a/metrics/src/tests/metricmanagertest.cpp b/metrics/src/tests/metricmanagertest.cpp
index f5d3a77d671..8424d3be3fe 100644
--- a/metrics/src/tests/metricmanagertest.cpp
+++ b/metrics/src/tests/metricmanagertest.cpp
@@ -943,8 +943,8 @@ public:
         CPPUNIT_ASSERT_EQUAL_MSG(_jsonText, dimensions.size(),
                                  nthMetricDimensionCount(metricIndex));
         for (auto& dim : dimensions) {
-            CPPUNIT_ASSERT_EQUAL_MSG(_jsonText, std::string(dim.value),
-                                     nthMetricDimension(metricIndex, dim.key));
+            CPPUNIT_ASSERT_EQUAL_MSG(_jsonText, std::string(dim.value()),
+                                     nthMetricDimension(metricIndex, dim.key()));
         }
     }
 };
diff --git a/metrics/src/tests/metrictest.cpp b/metrics/src/tests/metrictest.cpp
index 04f6b6e85b4..d9c39ef5cd2 100644
--- a/metrics/src/tests/metrictest.cpp
+++ b/metrics/src/tests/metrictest.cpp
@@ -41,7 +41,7 @@ void
 MetricTest::testMetricsGetDimensionsAsPartOfMangledNameImpl()
 {
     MetricImpl m("test", {{"foo", "bar"}}, "description goes here");
-    CPPUNIT_ASSERT_EQUAL(std::string("test{foo:bar}"), m.getMangledName());
+    CPPUNIT_ASSERT_EQUAL(vespalib::string("test{foo:bar}"), m.getMangledName());
 }
 
 template <typename MetricImpl>
@@ -51,7 +51,7 @@ MetricTest::testMangledNameMayContainMultipleDimensionsImpl()
     MetricImpl m("test",
                 {{"flarn", "yarn"}, {"foo", "bar"}},
                 "description goes here");
-    CPPUNIT_ASSERT_EQUAL(std::string("test{flarn:yarn,foo:bar}"),
+    CPPUNIT_ASSERT_EQUAL(vespalib::string("test{flarn:yarn,foo:bar}"),
                          m.getMangledName());
 }
 
@@ -88,7 +88,7 @@ MetricTest::mangledNameListsDimensionsInLexicographicOrder()
     LongValueMetric m("test",
                       {{"xyz", "bar"}, {"abc", "foo"}, {"def", "baz"}},
                       "");
-    CPPUNIT_ASSERT_EQUAL(std::string("test{abc:foo,def:baz,xyz:bar}"),
+    CPPUNIT_ASSERT_EQUAL(vespalib::string("test{abc:foo,def:baz,xyz:bar}"),
                          m.getMangledName());
 }
 
@@ -96,15 +96,15 @@ void
 MetricTest::manglingDoesNotChangeOriginalMetricName()
 {
     LongValueMetric m("test", {{"foo", "bar"}}, "");
-    CPPUNIT_ASSERT_EQUAL(std::string("test"), m.getName());
+    CPPUNIT_ASSERT_EQUAL(vespalib::string("test"), m.getName());
 }
 
 void
 MetricTest::legacyTagsDoNotCreateMangledName()
 {
     LongValueMetric m("test", "foo bar", "");
-    CPPUNIT_ASSERT_EQUAL(std::string("test"), m.getName());
-    CPPUNIT_ASSERT_EQUAL(std::string("test"), m.getMangledName());
+    CPPUNIT_ASSERT_EQUAL(vespalib::string("test"), m.getName());
+    CPPUNIT_ASSERT_EQUAL(vespalib::string("test"), m.getMangledName());
 }
 
 } // metrics
diff --git a/metrics/src/vespa/metrics/CMakeLists.txt b/metrics/src/vespa/metrics/CMakeLists.txt
index 6eae8cd75e4..147e88cd61c 100644
--- a/metrics/src/vespa/metrics/CMakeLists.txt
+++ b/metrics/src/vespa/metrics/CMakeLists.txt
@@ -5,8 +5,8 @@ vespa_add_library(metrics
     countmetricvalues.cpp
     jsonwriter.cpp
     loadmetric.cpp
-    metric.cpp
     memoryconsumption.cpp
+    metric.cpp
     metricmanager.cpp
     metricset.cpp
     metricsnapshot.cpp
@@ -14,12 +14,14 @@ vespa_add_library(metrics
     metricvalueset.cpp
     namehash.cpp
     printutils.cpp
+    name_repo.cpp
     state_api_adapter.cpp
     summetric.cpp
     textwriter.cpp
     valuemetric.cpp
     valuemetricvalues.cpp
     xmlwriter.cpp
+
     INSTALL lib64
     DEPENDS
 )
diff --git a/metrics/src/vespa/metrics/countmetric.hpp b/metrics/src/vespa/metrics/countmetric.hpp
index e150ce0d632..b46d45d690f 100644
--- a/metrics/src/vespa/metrics/countmetric.hpp
+++ b/metrics/src/vespa/metrics/countmetric.hpp
@@ -160,7 +160,7 @@ CountMetric<T, SumOnAdd>::print(std::ostream& out, bool verbose,
     (void) indent;
     Values values(_values.getValues());
     if (values._value == 0 && !verbose) return;
-    out << this->_name << (SumOnAdd ? " count=" : " value=") << values._value;
+    out << this->getName() << (SumOnAdd ? " count=" : " value=") << values._value;
     if (SumOnAdd) {
         if (secondsPassed != 0) {
             double avgDiff = values._value / ((double) secondsPassed);
diff --git a/metrics/src/vespa/metrics/jsonwriter.cpp b/metrics/src/vespa/metrics/jsonwriter.cpp
index 6ea585f0635..d890d725c8a 100644
--- a/metrics/src/vespa/metrics/jsonwriter.cpp
+++ b/metrics/src/vespa/metrics/jsonwriter.cpp
@@ -70,8 +70,8 @@ void
 JsonWriter::writeDimensions(const DimensionSet& dimensions)
 {
     for (const auto& dimension : dimensions) {
-        if (!dimension.key.empty() && !dimension.value.empty()) {
-            _stream << dimension.key << dimension.value;
+        if (!dimension.key().empty() && !dimension.value().empty()) {
+            _stream << dimension.key() << dimension.value();
         }
     }
 }
diff --git a/metrics/src/vespa/metrics/memoryconsumption.cpp b/metrics/src/vespa/metrics/memoryconsumption.cpp
index 7cfda8f28af..0e69defa558 100644
--- a/metrics/src/vespa/metrics/memoryconsumption.cpp
+++ b/metrics/src/vespa/metrics/memoryconsumption.cpp
@@ -30,6 +30,24 @@ MemoryConsumption::getStringMemoryUsage(const std::string& s, uint32_t& uniqueCo
     return s.capacity();
 }
 
+
+uint32_t
+MemoryConsumption::getStringMemoryUsage(const vespalib::string& s, uint32_t& uniqueCount) {
+    ++_totalStringCount;
+    const char* internalString = s.c_str();
+    if (_seenStrings->find(internalString) != _seenStrings->end()) {
+        return 0;
+    }
+    ++uniqueCount;
+    _seenStrings->insert(internalString);
+    const void *p = &s;
+    if ((p <= internalString) && (internalString - sizeof(vespalib::string) < p)) {
+        // no extra space allocated outside object
+        return 0;
+    }
+    return s.capacity();
+}
+
 void
 MemoryConsumption::addSnapShotUsage(const std::string& name, uint32_t usage) {
     _snapShotUsage->push_back(std::pair<std::string, uint32_t>(name, usage));
diff --git a/metrics/src/vespa/metrics/memoryconsumption.h b/metrics/src/vespa/metrics/memoryconsumption.h
index 8ac724b7a43..1cbd8f1deab 100644
--- a/metrics/src/vespa/metrics/memoryconsumption.h
+++ b/metrics/src/vespa/metrics/memoryconsumption.h
@@ -87,6 +87,7 @@ public:
 
     /** Get memory usage of a string that is not included when doing sizeof */
     uint32_t getStringMemoryUsage(const std::string& s, uint32_t& uniqueCount);
+    uint32_t getStringMemoryUsage(const vespalib::string& s, uint32_t& uniqueCount);
     void addSnapShotUsage(const std::string& name, uint32_t usage);
 
     uint32_t getTotalMemoryUsage() const;
diff --git a/metrics/src/vespa/metrics/metric.cpp b/metrics/src/vespa/metrics/metric.cpp
index e67398e4626..05483a5be11 100644
--- a/metrics/src/vespa/metrics/metric.cpp
+++ b/metrics/src/vespa/metrics/metric.cpp
@@ -4,7 +4,7 @@
 #include "countmetric.h"
 #include "valuemetric.h"
 #include "metricset.h"
-#include "namehash.h"
+#include "memoryconsumption.h"
 #include <vespa/vespalib/text/stringtokenizer.h>
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/stllike/asciistream.h>
@@ -52,8 +52,8 @@ namespace {
 vespalib::Regexp Metric::_namePattern(namePattern);
 
 Tag::Tag(vespalib::stringref k, vespalib::stringref v)
-    : key(k),
-      value(v)
+    : _key(NameRepo::tagKeyId(k)),
+      _value(NameRepo::tagValueId(v))
 { }
 
 Tag::Tag(const Tag &) = default;
@@ -64,8 +64,9 @@ Metric::Metric(const String& name,
                const String& tags,
                const String& description,
                MetricSet* owner)
-    : _name(name),
-      _description(description),
+    : _name(NameRepo::metricId(name)),
+      _mangledName(_name),
+      _description(NameRepo::descriptionId(description)),
       _tags(legacyTagStringToKeyedTags(tags)),
       _owner(nullptr) // Set later by registry
 {
@@ -78,8 +79,9 @@ Metric::Metric(const String& name,
                Tags dimensions,
                const String& description,
                MetricSet* owner)
-    : _name(name),
-      _description(description),
+    : _name(NameRepo::metricId(name)),
+      _mangledName(_name),
+      _description(NameRepo::descriptionId(description)),
       _tags(std::move(dimensions)),
       _owner(nullptr)
 {
@@ -92,6 +94,7 @@ Metric::Metric(const String& name,
 Metric::Metric(const Metric& other, MetricSet* owner)
     : Printable(other),
       _name(other._name),
+      _mangledName(other._mangledName),
       _description(other._description),
       _tags(other._tags),
       _owner(nullptr)
@@ -108,7 +111,7 @@ Metric::~Metric() { }
 bool
 Metric::tagsSpecifyAtLeastOneDimension(const Tags& tags) const
 {
-    auto hasNonEmptyTagValue = [](const Tag& t) { return !t.value.empty(); };
+    auto hasNonEmptyTagValue = [](const Tag& t) { return !t.value().empty(); };
     return std::any_of(tags.begin(), tags.end(), hasNonEmptyTagValue);
 }
 
@@ -120,32 +123,33 @@ Metric::assignMangledNameWithDimensions()
         return;
     }
     sortTagsInDeterministicOrder();
-    _mangledName = createMangledNameWithDimensions();
+    vespalib::string mangled = createMangledNameWithDimensions();
+    _mangledName = NameRepo::metricId(mangled);
 }
 
 void
 Metric::sortTagsInDeterministicOrder()
 {
     std::sort(_tags.begin(), _tags.end(), [](const Tag& a, const Tag& b) {
-        return a.key < b.key;
+        return a.key() < b.key();
     });
 }
 
-std::string
+vespalib::string
 Metric::createMangledNameWithDimensions() const
 {
     vespalib::asciistream s;
-    s << _name << '{';
+    s << getName() << '{';
     const size_t sz = _tags.size();
     for (size_t i = 0; i < sz; ++i) {
         const Tag& dimension(_tags[i]);
-        if (dimension.value.empty()) {
+        if (dimension.value().empty()) {
             continue;
         }
         if (i != 0) {
             s << ',';
         }
-        s << dimension.key << ':' << dimension.value;
+        s << dimension.key() << ':' << dimension.value();
     }
     s << '}';
     return s.str();
@@ -154,13 +158,13 @@ Metric::createMangledNameWithDimensions() const
 void
 Metric::verifyConstructionParameters()
 {
-    if (_name.size() == 0) {
+    if (getName().size() == 0) {
         throw vespalib::IllegalArgumentException(
                 "Metric cannot have empty name", VESPA_STRLOC);
     }
-    if (!_namePattern.match(_name)) {
+    if (!_namePattern.match(getName())) {
         throw vespalib::IllegalArgumentException(
-                "Illegal metric name '" + _name + "'. Names must match pattern "
+                "Illegal metric name '" + getName() + "'. Names must match pattern "
                 + namePattern, VESPA_STRLOC);
     }
 }
@@ -185,9 +189,9 @@ vespalib::string
 Metric::getPath() const
 {
     if (_owner == 0 || _owner->_owner == 0) {
-        return _name;
+        return getName();
     } else {
-        return _owner->getPath() + "." + _name;
+        return _owner->getPath() + "." + getName();
     }
 }
 
@@ -195,10 +199,10 @@ std::vector<Metric::String>
 Metric::getPathVector() const
 {
     std::vector<String> result;
-    result.push_back(_name);
+    result.push_back(getName());
     const MetricSet* owner(_owner);
     while (owner != 0) {
-        result.push_back(owner->_name);
+        result.push_back(owner->getName());
         owner = owner->_owner;
     }
     std::reverse(result.begin(), result.end());
@@ -209,7 +213,7 @@ bool
 Metric::hasTag(const String& tag) const
 {
     return std::find_if(_tags.begin(), _tags.end(), [&](const Tag& t) {
-        return t.key == tag;
+        return t.key() == tag;
     }) != _tags.end();
 }
 
@@ -217,9 +221,8 @@ void
 Metric::addMemoryUsage(MemoryConsumption& mc) const
 {
     ++mc._metricCount;
-    mc._metricName += mc.getStringMemoryUsage(_name, mc._metricNameUnique);
-    mc._metricDescription += mc.getStringMemoryUsage(
-                                    _description, mc._metricDescriptionUnique);
+    mc._metricName += mc.getStringMemoryUsage(getName(), mc._metricNameUnique);
+    mc._metricDescription += mc.getStringMemoryUsage(getDescription(), mc._metricDescriptionUnique);
     mc._metricTagCount += _tags.size();
     // XXX figure out what we actually want to report from tags here...
     // XXX we don't care about unique strings since they don't matter anymore.
@@ -228,21 +231,10 @@ Metric::addMemoryUsage(MemoryConsumption& mc) const
 }
 
 void
-Metric::updateNames(NameHash& hash) const
-{
-    Metric& m(const_cast<Metric&>(*this));
-    hash.updateName(m._name);
-    hash.updateName(m._description);
-    // Tags use vespalib::string which isn't refcounted under the hood and
-    // use small string optimizations, meaning the implicit ref sharing hack
-    // won't work for them anyway.
-}
-
-void
 Metric::printDebug(std::ostream& out, const std::string& indent) const
 {
     (void) indent;
-    out << "name=" << _name << ", instance=" << ((const void*) this)
+    out << "name=" << getName() << ", instance=" << ((const void*) this)
         << ", owner=" << ((const void*) _owner);
 }
 
diff --git a/metrics/src/vespa/metrics/metric.h b/metrics/src/vespa/metrics/metric.h
index 91ad88fecf7..dc9e45e89af 100644
--- a/metrics/src/vespa/metrics/metric.h
+++ b/metrics/src/vespa/metrics/metric.h
@@ -4,6 +4,7 @@
 #include <vespa/vespalib/util/printable.h>
 #include <vespa/vespalib/stllike/string.h>
 #include <vespa/vespalib/util/regexp.h>
+#include "name_repo.h"
 
 namespace metrics {
 
@@ -14,7 +15,6 @@ class MetricSet;
 class MetricSnapshot;
 class XmlWriterMetricVisitor;
 class MemoryConsumption;
-class NameHash;
 
 /** Implement class to visit metrics. */
 struct MetricVisitor {
@@ -83,8 +83,8 @@ struct MetricVisitor {
  */
 struct Tag
 {
-    vespalib::string key;
-    vespalib::string value;
+    const vespalib::string& key() const { return NameRepo::tagKey(_key); }
+    const vespalib::string& value() const { return NameRepo::tagValue(_value); }
 
     Tag(vespalib::stringref k, vespalib::stringref v);
     Tag(const Tag &);
@@ -92,6 +92,10 @@ struct Tag
     Tag(Tag &&) = default;
     Tag & operator = (Tag &&) = default;
     ~Tag();
+
+private:
+    TagKeyId _key;
+    TagValueId _value;
 };
 
 class Metric : public vespalib::Printable
@@ -120,17 +124,17 @@ public:
     Metric & operator = (Metric && rhs) = default;
     ~Metric();
 
-    const String& getName() const { return _name; }
+    const vespalib::string& getName() const { return NameRepo::metricName(_name); }
     /**
      * Get mangled name iff the metric contains any dimensions, otherwise
      * the original metric name is returned.
      */
-    const String& getMangledName() const {
-        return (_mangledName.empty() ? _name : _mangledName);
+    const vespalib::string& getMangledName() const {
+        return NameRepo::metricName(_mangledName);
     }
     vespalib::string getPath() const;
     std::vector<String> getPathVector() const;
-    const String& getDescription() const { return _description; }
+    const vespalib::string& getDescription() const { return NameRepo::description(_description); }
     const Tags& getTags() const { return _tags; }
     /** Return whether there exists a tag with a key equal to 'tag' */
     bool hasTag(const String& tag) const;
@@ -216,12 +220,15 @@ public:
 
     /** Used by sum metric to alter name of cloned metric for sum. */
     void setName(const String& name) {
-        _name = name;
+        MetricNameId newName = NameRepo::metricId(name);
+        _name = newName;
         assignMangledNameWithDimensions();
     }
 
     /** Used by sum metric to alter description of cloned metric for sum. */
-    void setDescription(const String& d) { _description = d; }
+    void setDescription(const vespalib::string& d) {
+        _description = NameRepo::descriptionId(d);
+    }
     /** Used by sum metric to alter tag of cloned metric for sum. */
     void setTags(Tags tags) {
         _tags = std::move(tags);
@@ -242,18 +249,6 @@ public:
 
     virtual void addMemoryUsage(MemoryConsumption&) const;
 
-    /**
-     * Update names using the given name hash, to utilize ref counting.
-     *
-     * NOTE:
-     * This is a hack that only works on GCC until they decide to finally break
-     * ABI compatibility and remove that particular multicore-hostile feature
-     * of their std::string implementation. If we want proper string ref
-     * counting, all strings should be replaced with explicit string handles
-     * and should only be created via a shared factory.
-     */
-    virtual void updateNames(NameHash&) const;
-
     /** Print debug information of the metric tree. */
     virtual void printDebug(std::ostream&, const std::string& indent="") const;
 
@@ -282,7 +277,7 @@ private:
      */
     void sortTagsInDeterministicOrder();
 
-    std::string createMangledNameWithDimensions() const;
+    vespalib::string createMangledNameWithDimensions() const;
 
     void verifyConstructionParameters();
     /**
@@ -292,9 +287,9 @@ private:
     void registerWithOwnerIfRequired(MetricSet* owner);
 
 protected:
-    String _name;
-    String _mangledName;
-    String _description;
+    MetricNameId _name;
+    MetricNameId _mangledName;
+    DescriptionId _description;
     std::vector<Tag> _tags;
     MetricSet* _owner;
 
diff --git a/metrics/src/vespa/metrics/metricmanager.cpp b/metrics/src/vespa/metrics/metricmanager.cpp
index b8255a6532f..544998e25cd 100644
--- a/metrics/src/vespa/metrics/metricmanager.cpp
+++ b/metrics/src/vespa/metrics/metricmanager.cpp
@@ -366,23 +366,15 @@ MetricManager::handleMetricsAltered(const MetricLockGuard & guard)
         configMap[consumer.name] = ConsumerSpec::SP(new ConsumerSpec(std::move(consumerMetricBuilder._matchedMetrics)));
     }
     LOG(debug, "Recreating snapshots to include altered metrics");
-    _activeMetrics.updateNames(_nameHash);
     _totalMetrics->recreateSnapshot(_activeMetrics.getMetrics(),
                                     _snapshotUnsetMetrics);
-    _totalMetrics->updateNames(_nameHash);
     for (uint32_t i=0; i<_snapshots.size(); ++i) {
         _snapshots[i]->recreateSnapshot(_activeMetrics.getMetrics(),
                                         _snapshotUnsetMetrics);
-        _snapshots[i]->updateNames(_nameHash);
     }
     LOG(debug, "Setting new consumer config. Clearing dirty flag");
     _consumerConfig.swap(configMap);
     _consumerConfigChanged = false;
-    LOG(debug, "Unified %u of %u strings configuring %" PRIu64 " consumers.",
-        _nameHash.getUnifiedStringCount(),
-        _nameHash.getCheckedStringCount(),
-        _config->consumer.size());
-    _nameHash.resetCounts();
 }
 
 namespace {
@@ -966,7 +958,6 @@ MetricManager::getMemoryConsumption(const MetricLockGuard & guard) const
     _totalMetrics->addMemoryUsage(*mc);
     postTotal = mc->getTotalMemoryUsage();
     mc->addSnapShotUsage("total", postTotal - preTotal);
-    _nameHash.addMemoryUsage(*mc);
     return mc;
 }
 
diff --git a/metrics/src/vespa/metrics/metricmanager.h b/metrics/src/vespa/metrics/metricmanager.h
index 423eb41a787..ec166529b94 100644
--- a/metrics/src/vespa/metrics/metricmanager.h
+++ b/metrics/src/vespa/metrics/metricmanager.h
@@ -48,7 +48,7 @@
 #include <vespa/metrics/config-metricsmanager.h>
 #include <vespa/metrics/metricset.h>
 #include <vespa/metrics/metricsnapshot.h>
-#include <vespa/metrics/namehash.h>
+#include <vespa/metrics/memoryconsumption.h>
 #include <vespa/metrics/valuemetric.h>
 #include <vespa/metrics/updatehook.h>
 #include <vespa/vespalib/stllike/hash_set.h>
@@ -125,10 +125,6 @@ private:
     LongAverageMetric _snapshotLatency;
     LongAverageMetric _sleepTimes;
 
-    // Name hash trying to ensure string ref counting works as well as can be
-    // expected
-    NameHash _nameHash;
-
 public:
     MetricManager(std::unique_ptr<Timer> timer = std::unique_ptr<Timer>(new Timer));
     ~MetricManager();
diff --git a/metrics/src/vespa/metrics/metricset.cpp b/metrics/src/vespa/metrics/metricset.cpp
index cfaeb8b6f02..6ffae806322 100644
--- a/metrics/src/vespa/metrics/metricset.cpp
+++ b/metrics/src/vespa/metrics/metricset.cpp
@@ -321,7 +321,7 @@ void
 MetricSet::print(std::ostream& out, bool verbose,
                  const std::string& indent, uint64_t secondsPassed) const
 {
-    out << _name << ":";
+    out << getName() << ":";
     for (const Metric* metric : _metricOrder) {
         out << "\n" << indent << "  ";
         metric->print(out, verbose, indent + "  ", secondsPassed);
@@ -350,15 +350,6 @@ MetricSet::addMemoryUsage(MemoryConsumption& mc) const
 }
 
 void
-MetricSet::updateNames(NameHash& hash) const
-{
-    Metric::updateNames(hash);
-    for (const Metric* metric : _metricOrder) {
-        metric->updateNames(hash);
-    }
-}
-
-void
 MetricSet::printDebug(std::ostream& out, const std::string& indent) const
 {
     out << "set ";
diff --git a/metrics/src/vespa/metrics/metricset.h b/metrics/src/vespa/metrics/metricset.h
index 48027db374a..a8fea2fc290 100644
--- a/metrics/src/vespa/metrics/metricset.h
+++ b/metrics/src/vespa/metrics/metricset.h
@@ -75,8 +75,6 @@ public:
     bool used() const override;
     void addMemoryUsage(MemoryConsumption&) const override;
 
-    /** Update names using the given name hash, to utilize ref counting. */
-    void updateNames(NameHash&) const override;
     void printDebug(std::ostream&, const std::string& indent="") const override;
     bool isMetricSet() const override { return true; }
     void addToPart(Metric& m) const override { addTo(m, 0); }
diff --git a/metrics/src/vespa/metrics/metricsnapshot.cpp b/metrics/src/vespa/metrics/metricsnapshot.cpp
index 0b6ad8f9acd..ac3c41215aa 100644
--- a/metrics/src/vespa/metrics/metricsnapshot.cpp
+++ b/metrics/src/vespa/metrics/metricsnapshot.cpp
@@ -148,13 +148,6 @@ MetricSnapshotSet::addMemoryUsage(MemoryConsumption& mc) const
 }
 
 void
-MetricSnapshotSet::updateNames(NameHash& hash) const
-{
-    if (_count != 1) _building->updateNames(hash);
-    _current->updateNames(hash);
-}
-
-void
 MetricSnapshotSet::setFromTime(time_t fromTime)
 {
     if (_count != 1) _building->setFromTime(fromTime);
diff --git a/metrics/src/vespa/metrics/metricsnapshot.h b/metrics/src/vespa/metrics/metricsnapshot.h
index 4cf8d113743..b00c001505c 100644
--- a/metrics/src/vespa/metrics/metricsnapshot.h
+++ b/metrics/src/vespa/metrics/metricsnapshot.h
@@ -69,7 +69,6 @@ public:
     void recreateSnapshot(const MetricSet& metrics, bool copyUnset);
 
     void addMemoryUsage(MemoryConsumption&) const;
-    void updateNames(NameHash& hash) const { _snapshot->updateNames(hash); }
 };
 
 class MetricSnapshotSet {
@@ -107,7 +106,6 @@ public:
      */
     void recreateSnapshot(const MetricSet& metrics, bool copyUnset);
     void addMemoryUsage(MemoryConsumption&) const;
-    void updateNames(NameHash& hash) const;
     void setFromTime(time_t fromTime);
 };
 
diff --git a/metrics/src/vespa/metrics/name_repo.cpp b/metrics/src/vespa/metrics/name_repo.cpp
new file mode 100644
index 00000000000..380120194d7
--- /dev/null
+++ b/metrics/src/vespa/metrics/name_repo.cpp
@@ -0,0 +1,71 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "name_repo.h"
+#include <vespa/vespalib/metrics/name_collection.h>
+
+using vespalib::metrics::NameCollection;
+
+namespace metrics {
+
+namespace {
+NameCollection metricNames;
+NameCollection descriptions;
+NameCollection tagKeys;
+NameCollection tagValues;
+}
+
+MetricNameId
+NameRepo::metricId(const vespalib::string &name)
+{
+    size_t id = metricNames.resolve(name);
+    return MetricNameId(id);
+}
+
+DescriptionId
+NameRepo::descriptionId(const vespalib::string &name)
+{
+    size_t id = descriptions.resolve(name);
+    return DescriptionId(id);
+}
+
+TagKeyId
+NameRepo::tagKeyId(const vespalib::string &name)
+{
+    size_t id = tagKeys.resolve(name);
+    return TagKeyId(id);
+}
+
+TagValueId
+NameRepo::tagValueId(const vespalib::string &value)
+{
+    size_t id = tagValues.resolve(value);
+    return TagValueId(id);
+}
+
+const vespalib::string&
+NameRepo::metricName(MetricNameId id)
+{
+    return metricNames.lookup(id.id());
+}
+
+const vespalib::string&
+NameRepo::description(DescriptionId id)
+{
+    return descriptions.lookup(id.id());
+}
+
+const vespalib::string&
+NameRepo::tagKey(TagKeyId id)
+{
+    return tagKeys.lookup(id.id());
+}
+
+const vespalib::string&
+NameRepo::tagValue(TagValueId id)
+{
+    return tagValues.lookup(id.id());
+}
+
+
+} // namespace metrics
+
diff --git a/metrics/src/vespa/metrics/name_repo.h b/metrics/src/vespa/metrics/name_repo.h
new file mode 100644
index 00000000000..44f8f622bff
--- /dev/null
+++ b/metrics/src/vespa/metrics/name_repo.h
@@ -0,0 +1,32 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+#include <vespa/vespalib/stllike/string.h>
+#include <vespa/vespalib/metrics/handle.h>
+
+namespace metrics {
+
+struct MetricNameIdTag {};
+struct DescriptionIdTag {};
+struct TagKeyIdTag {};
+struct TagValueIdTag {};
+
+using MetricNameId = vespalib::metrics::Handle<MetricNameIdTag>;
+using DescriptionId = vespalib::metrics::Handle<DescriptionIdTag>;
+using TagKeyId = vespalib::metrics::Handle<TagKeyIdTag>;
+using TagValueId = vespalib::metrics::Handle<TagValueIdTag>;
+
+struct NameRepo {
+    static MetricNameId metricId(const vespalib::string &name);
+    static DescriptionId descriptionId(const vespalib::string &name);
+    static TagKeyId tagKeyId(const vespalib::string &name);
+    static TagValueId tagValueId(const vespalib::string &value);
+
+    static const vespalib::string& metricName(MetricNameId id);
+    static const vespalib::string& description(DescriptionId id);
+    static const vespalib::string& tagKey(TagKeyId id);
+    static const vespalib::string& tagValue(TagValueId id);
+};
+
+} // metrics
+
diff --git a/metrics/src/vespa/metrics/valuemetric.hpp b/metrics/src/vespa/metrics/valuemetric.hpp
index 60ed219edec..8fe8769e9ad 100644
--- a/metrics/src/vespa/metrics/valuemetric.hpp
+++ b/metrics/src/vespa/metrics/valuemetric.hpp
@@ -213,7 +213,7 @@ ValueMetric<AvgVal, TotVal, SumOnAdd>::print(
     (void) secondsPassed;
     Values values(_values.getValues());
     if (!inUse(values) && !verbose) return;
-    out << this->_name << " average=" << (values._count == 0
+    out << this->getName() << " average=" << (values._count == 0
             ? 0 : static_cast<double>(values._total) / values._count)
         << " last=" << values._last;
     if (!summedAverage()) {
diff --git a/metrics/src/vespa/metrics/xmlwriter.cpp b/metrics/src/vespa/metrics/xmlwriter.cpp
index 197ffdd1e3d..90b25621cf8 100644
--- a/metrics/src/vespa/metrics/xmlwriter.cpp
+++ b/metrics/src/vespa/metrics/xmlwriter.cpp
@@ -98,9 +98,9 @@ XmlWriter::printCommonXmlParts(const Metric& metric) const
     if (_verbosity >= 3 && tags.size() > 0) {
         std::ostringstream ost;
         // XXX print tag values as well
-        ost << tags[0].key;
+        ost << tags[0].key();
         for (uint32_t i=1; i<tags.size(); ++i) {
-            ost << "," << tags[i].key;
+            ost << "," << tags[i].key();
         }
         _xos << XmlAttribute("tags", ost.str());
     }