diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-03-18 12:35:50 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2021-03-18 12:35:50 +0000 |
commit | bdf23bc4260b92687ffb41ccbd8fe4ec0474e5b1 (patch) | |
tree | d1802439a4fa57a745ee3b120472d6000fc64bf4 /metrics/src | |
parent | 2e0ad58bb9bbb5cc24289acff25046a5b9442e2b (diff) |
- Compute the Path objects once, instead of parsing them for every metric.
- Use vespalib::string all over to avoid back and forth construction.
Diffstat (limited to 'metrics/src')
-rw-r--r-- | metrics/src/vespa/metrics/metric.h | 3 | ||||
-rw-r--r-- | metrics/src/vespa/metrics/metricmanager.cpp | 255 | ||||
-rw-r--r-- | metrics/src/vespa/metrics/metricset.cpp | 12 | ||||
-rw-r--r-- | metrics/src/vespa/metrics/metricset.h | 6 |
4 files changed, 144 insertions, 132 deletions
diff --git a/metrics/src/vespa/metrics/metric.h b/metrics/src/vespa/metrics/metric.h index f254ec34074..10b74a2da22 100644 --- a/metrics/src/vespa/metrics/metric.h +++ b/metrics/src/vespa/metrics/metric.h @@ -13,7 +13,6 @@ struct AbstractValueMetric; class Metric; class MetricSet; class MetricSnapshot; -class XmlWriterMetricVisitor; class MemoryConsumption; /** Implement class to visit metrics. */ @@ -104,7 +103,7 @@ private: class Metric : public vespalib::Printable { public: - using String = std::string; + using String = vespalib::string; using stringref = vespalib::stringref; using UP = std::unique_ptr<Metric>; using SP = std::shared_ptr<Metric>; diff --git a/metrics/src/vespa/metrics/metricmanager.cpp b/metrics/src/vespa/metrics/metricmanager.cpp index 8ca74384af0..7fa13a06e7b 100644 --- a/metrics/src/vespa/metrics/metricmanager.cpp +++ b/metrics/src/vespa/metrics/metricmanager.cpp @@ -190,147 +190,160 @@ MetricManager::init(const config::ConfigUri & uri, FastOS_ThreadPool& pool, namespace { - struct Path { - vespalib::StringTokenizer _path; +struct Path { + vespalib::StringTokenizer _path; - Path(vespalib::stringref fullpath) : _path(fullpath, ".") { } + Path(vespalib::stringref fullpath) : _path(fullpath, ".") { } - vespalib::string toString() const { - vespalib::asciistream ost; - ost << _path[0]; - for (uint32_t i=1; i<path().size(); ++i) ost << "." << _path[i]; - return ost.str(); - } + vespalib::string toString() const { + vespalib::asciistream ost; + ost << _path[0]; + for (uint32_t i=1; i<path().size(); ++i) ost << "." << _path[i]; + return ost.str(); + } - bool matchesPattern(const Path& p) const { - if (path().size() != p.path().size()) { + bool matchesPattern(const Path& p) const { + if (path().size() != p.path().size()) { + return false; + } + for (uint32_t i=0; i<p.path().size(); ++i) { + if (p._path[i] == "*") continue; + if (_path[i] != p._path[i]) { return false; } - for (uint32_t i=0; i<p.path().size(); ++i) { - if (p._path[i] == "*") continue; - if (_path[i] != p._path[i]) { - return false; - } - } - return true; - } - const vespalib::StringTokenizer::TokenList& path() const { - return _path.getTokens(); } + return true; + } + const vespalib::StringTokenizer::TokenList& path() const { + return _path.getTokens(); + } +}; + +struct ConsumerMetricBuilder : public MetricVisitor { + const Config::Consumer& _consumer; + std::vector<Path> _added; + std::vector<Path> _removed; + MetricManager::ConsumerSpec _matchedMetrics; + // Keep a stack of matches to facilitate tree traversal. + struct Result { + bool tagAdded; + bool tagRemoved; + bool nameAdded; + bool nameRemoved; + uint32_t metricCount; + + Result() : tagAdded(false), tagRemoved(false), + nameAdded(false), nameRemoved(false), metricCount(0) {} }; + std::list<Result> result; - struct ConsumerMetricBuilder : public MetricVisitor { - const Config::Consumer& _consumer; - MetricManager::ConsumerSpec _matchedMetrics; - // Keep a stack of matches to facilitate tree traversal. - struct Result { - bool tagAdded; - bool tagRemoved; - bool nameAdded; - bool nameRemoved; - uint32_t metricCount; - - Result() : tagAdded(false), tagRemoved(false), - nameAdded(false), nameRemoved(false), metricCount(0) {} - }; - std::list<Result> result; - - ConsumerMetricBuilder(const Config::Consumer& c) __attribute__((noinline)); - ~ConsumerMetricBuilder() __attribute__((noinline)); - - bool tagAdded(const Metric& metric) { - for (const auto& s : _consumer.tags) { - if ((s == "*" && !metric.getTags().empty()) - || metric.hasTag(s)) - { - return true; - } + ConsumerMetricBuilder(const Config::Consumer& c) __attribute__((noinline)); + ~ConsumerMetricBuilder() __attribute__((noinline)); + + bool tagAdded(const Metric& metric) { + for (const auto& s : _consumer.tags) { + if ((s == "*" && !metric.getTags().empty()) + || metric.hasTag(s)) + { + return true; } - return false; } - bool tagRemoved(const Metric& metric) { - for (const auto& s : _consumer.removedtags) { - if ((s == "*" && !metric.getTags().empty()) - || metric.hasTag(s)) - { - return true; - } + return false; + } + bool tagRemoved(const Metric& metric) { + for (const auto& s : _consumer.removedtags) { + if ((s == "*" && !metric.getTags().empty()) + || metric.hasTag(s)) + { + return true; } - return false; } - bool nameAdded(const Metric& metric) { - Metric::String path(metric.getPath()); - Path mpath(path); - for (const auto& s : _consumer.addedmetrics) { - if (mpath.matchesPattern(Path(s))) { - return true; - } + return false; + } + bool nameAdded(const Path& mpath) { + for (const auto& p : _added) { + if (mpath.matchesPattern(p)) { + return true; } - return false; } - bool nameRemoved(const Metric& metric) { - Metric::String path(metric.getPath()); - Path mpath(path); - for (const auto& s : _consumer.removedmetrics) { - if (mpath.matchesPattern(Path(s))) { - return true; - } + return false; + } + bool nameRemoved(const Path & mpath) { + for (const auto& p : _removed) { + if (mpath.matchesPattern(p)) { + return true; } - return false; } + return false; + } - bool visitMetricSet(const MetricSet& metricSet, - bool autoGenerated) override - { - (void) autoGenerated; - result.push_back(Result()); - // If current metric match anything, use that. - // Otherwise, copy from parent - if (nameRemoved(metricSet)) { - result.back().nameRemoved = true; - } else if (nameAdded(metricSet)) { - result.back().nameAdded = true; - } else if (tagRemoved(metricSet)) { - result.back().tagRemoved = true; - } else if (tagAdded(metricSet)) { - result.back().tagAdded = true; - } else if (result.size() > 1) { - result.back() = *++result.rbegin(); - result.back().metricCount = 0; - } - return true; + bool visitMetricSet(const MetricSet& metricSet, bool autoGenerated) override + { + (void) autoGenerated; + result.push_back(Result()); + // If current metric match anything, use that. + // Otherwise, copy from parent + vespalib::string fullName = metricSet.getPath(); + Path path(fullName); + if (nameRemoved(path)) { + result.back().nameRemoved = true; + } else if (nameAdded(path)) { + result.back().nameAdded = true; + } else if (tagRemoved(metricSet)) { + result.back().tagRemoved = true; + } else if (tagAdded(metricSet)) { + result.back().tagAdded = true; + } else if (result.size() > 1) { + result.back() = *++result.rbegin(); + result.back().metricCount = 0; } - void doneVisitingMetricSet(const MetricSet& metricSet) override { - if (result.back().metricCount > 0 && result.size() != 1) { - LOG(spam, "Adding metricset %s", metricSet.getPath().c_str()); - _matchedMetrics.includedMetrics.insert(metricSet.getPath()); - } - result.pop_back(); + return true; + } + void doneVisitingMetricSet(const MetricSet& metricSet) override { + if (result.back().metricCount > 0 && result.size() != 1) { + LOG(spam, "Adding metricset %s", metricSet.getPath().c_str()); + _matchedMetrics.includedMetrics.insert(metricSet.getPath()); } - bool visitMetric(const Metric& metric, bool autoGenerated) override { - (void) autoGenerated; - if (result.back().nameRemoved || nameRemoved(metric)) return true; - bool nAdded = (result.back().nameAdded || nameAdded(metric)); - if (!nAdded && (result.back().tagRemoved || tagRemoved(metric))) { - return true; - } - if (nAdded || result.back().tagAdded || tagAdded(metric)) { - _matchedMetrics.includedMetrics.insert(metric.getPath()); - LOG(spam, "Adding metric %s", metric.getPath().c_str()); - for (Result & r : result) { - ++r.metricCount; - } - } + result.pop_back(); + } + bool visitMetric(const Metric& metric, bool autoGenerated) override { + (void) autoGenerated; + vespalib::string fullName = metric.getPath(); + Path path(fullName); + if (result.back().nameRemoved || nameRemoved(path)) return true; + bool nAdded = (result.back().nameAdded || nameAdded(path)); + if (!nAdded && (result.back().tagRemoved || tagRemoved(metric))) { return true; } + if (nAdded || result.back().tagAdded || tagAdded(metric)) { + _matchedMetrics.includedMetrics.insert(fullName); + LOG(spam, "Adding metric %s", fullName.c_str()); + for (Result & r : result) { + ++r.metricCount; + } + } + return true; + } - }; - ConsumerMetricBuilder::ConsumerMetricBuilder(const Config::Consumer& c) - : _consumer(c), _matchedMetrics() - { - LOG(spam, "Adding metrics for consumer %s", c.name.c_str()); +}; + +ConsumerMetricBuilder::ConsumerMetricBuilder(const Config::Consumer& c) + : _consumer(c), + _added(), + _removed(), + _matchedMetrics() +{ + _added.reserve(_consumer.addedmetrics.size()); + for (const auto& s : _consumer.addedmetrics) { + _added.emplace_back(s); + } + _removed.reserve(_consumer.removedmetrics.size()); + for (const auto& s : _consumer.removedmetrics) { + _removed.emplace_back(s); } - ConsumerMetricBuilder::~ConsumerMetricBuilder() = default; + LOG(spam, "Adding metrics for consumer %s", c.name.c_str()); +} +ConsumerMetricBuilder::~ConsumerMetricBuilder() = default; } @@ -352,8 +365,8 @@ void MetricManager::handleMetricsAltered(const MetricLockGuard & guard) { (void) guard; - if (_config.get() == NULL) { - LOG(info, "_config is NULL -> very odd indeed."); + if ( ! _config ) { + LOG(info, "_config is not set -> very odd indeed."); return; } if (_consumerConfig.empty()) { @@ -367,7 +380,7 @@ MetricManager::handleMetricsAltered(const MetricLockGuard & guard) for (const auto & consumer : _config->consumer) { ConsumerMetricBuilder consumerMetricBuilder(consumer); _activeMetrics.getMetrics().visit(consumerMetricBuilder); - configMap[consumer.name] = ConsumerSpec::SP(new ConsumerSpec(std::move(consumerMetricBuilder._matchedMetrics))); + configMap[consumer.name] = std::make_shared<ConsumerSpec>(std::move(consumerMetricBuilder._matchedMetrics)); } LOG(debug, "Recreating snapshots to include altered metrics"); _totalMetrics->recreateSnapshot(_activeMetrics.getMetrics(), diff --git a/metrics/src/vespa/metrics/metricset.cpp b/metrics/src/vespa/metrics/metricset.cpp index 2916b32eb56..e994015d487 100644 --- a/metrics/src/vespa/metrics/metricset.cpp +++ b/metrics/src/vespa/metrics/metricset.cpp @@ -51,7 +51,7 @@ MetricSet::clone(std::vector<Metric::UP> &ownerList, CopyType type, const Metric* -MetricSet::getMetricInternal(const String& name) const +MetricSet::getMetricInternal(stringref name) const { for (const Metric* metric : _metricOrder) { if (metric->getMangledName() == name) { @@ -71,14 +71,14 @@ double MetricSet::getDoubleValue(stringref) const { } const Metric* -MetricSet::getMetric(const String& name) const +MetricSet::getMetric(stringref name) const { - std::string::size_type pos = name.find('.'); - if (pos == std::string::npos) { + vespalib::string::size_type pos = name.find('.'); + if (pos == vespalib::string::npos) { return getMetricInternal(name); } else { - std::string child(name.substr(0, pos)); - std::string rest(name.substr(pos+1)); + stringref child(name.substr(0, pos)); + stringref rest(name.substr(pos+1)); const Metric* m(getMetricInternal(child)); if (m == 0) return 0; if (!m->isMetricSet()) { diff --git a/metrics/src/vespa/metrics/metricset.h b/metrics/src/vespa/metrics/metricset.h index 66225b8f868..84b5eb8c848 100644 --- a/metrics/src/vespa/metrics/metricset.h +++ b/metrics/src/vespa/metrics/metricset.h @@ -57,8 +57,8 @@ public: int64_t getLongValue(stringref id) const override; double getDoubleValue(stringref id) const override; - const Metric* getMetric(const String& name) const; - Metric* getMetric(const String& name) { + const Metric* getMetric(stringref name) const; + Metric* getMetric(stringref name) { return const_cast<Metric*>( const_cast<const MetricSet*>(this)->getMetric(name)); } @@ -81,7 +81,7 @@ private: MetricSet& operator=(const MetricSet&); void tagRegistrationAltered(); - const Metric* getMetricInternal(const String& name) const; + const Metric* getMetricInternal(stringref name) const; virtual void addTo(Metric&, std::vector<Metric::UP> *ownerList) const; }; |