From 23d07842d1db87316dde73df050401307f90dcbf Mon Sep 17 00:00:00 2001 From: Nick Brachet Date: Mon, 30 Nov 2020 16:59:01 -0500 Subject: [PATCH 1/3] Optimistically return metrics from the cache (if present) *before* building a metric to add to the cache --- core/include/prometheus/family.h | 3 +++ core/src/family.cc | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/core/include/prometheus/family.h b/core/include/prometheus/family.h index aa946832..d70923d2 100644 --- a/core/include/prometheus/family.h +++ b/core/include/prometheus/family.h @@ -109,6 +109,8 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable { /// labels already exists - the already existing dimensional data. template T& Add(const std::map& labels, Args&&... args) { + T* metric = GetMetric(labels); + if (metric) return *metric; return Add(labels, detail::make_unique(args...)); } @@ -148,6 +150,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable { ClientMetric CollectMetric(std::size_t hash, T* metric) const; T& Add(const std::map& labels, std::unique_ptr object); + T* GetMetric(const std::map& labels) const; }; } // namespace prometheus diff --git a/core/src/family.cc b/core/src/family.cc index 83631abc..abb7813f 100644 --- a/core/src/family.cc +++ b/core/src/family.cc @@ -14,6 +14,25 @@ Family::Family(const std::string& name, const std::string& help, assert(CheckMetricName(name_)); } +template +T* Family::GetMetric(const std::map& labels) const { + auto hash = detail::hash_labels(labels); + std::lock_guard lock{mutex_}; + auto metrics_iter = metrics_.find(hash); + + if (metrics_iter == metrics_.end()) + return nullptr; + +#ifndef NDEBUG + auto labels_iter = labels_.find(hash); + assert(labels_iter != labels_.end()); + const auto& old_labels = labels_iter->second; + assert(labels == old_labels); +#endif + + return metrics_iter->second.get(); +} + template T& Family::Add(const std::map& labels, std::unique_ptr object) { From 4c0c398ea899bc5d2f6586102f47f3506acf607c Mon Sep 17 00:00:00 2001 From: Nick Brachet Date: Fri, 4 Dec 2020 17:31:59 -0500 Subject: [PATCH 2/3] fix indentation --- core/src/family.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/family.cc b/core/src/family.cc index abb7813f..bfc2a095 100644 --- a/core/src/family.cc +++ b/core/src/family.cc @@ -24,10 +24,10 @@ T* Family::GetMetric(const std::map& labels) const return nullptr; #ifndef NDEBUG - auto labels_iter = labels_.find(hash); - assert(labels_iter != labels_.end()); - const auto& old_labels = labels_iter->second; - assert(labels == old_labels); + auto labels_iter = labels_.find(hash); + assert(labels_iter != labels_.end()); + const auto& old_labels = labels_iter->second; + assert(labels == old_labels); #endif return metrics_iter->second.get(); From 04e3899aff3ecc92ca1a8622229f399846aa5e40 Mon Sep 17 00:00:00 2001 From: Nick Brachet Date: Wed, 9 Dec 2020 16:07:39 -0500 Subject: [PATCH 3/3] re-work to save computation of hash --- core/include/prometheus/family.h | 13 +++++----- core/src/family.cc | 42 +++++++++++++------------------- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/core/include/prometheus/family.h b/core/include/prometheus/family.h index d70923d2..f36e3ec6 100644 --- a/core/include/prometheus/family.h +++ b/core/include/prometheus/family.h @@ -109,9 +109,9 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable { /// labels already exists - the already existing dimensional data. template T& Add(const std::map& labels, Args&&... args) { - T* metric = GetMetric(labels); - if (metric) return *metric; - return Add(labels, detail::make_unique(args...)); + metrics_iterator iter = FindMetric(labels); + if (iter->second) return *(iter->second); + return Add(iter, detail::make_unique(args...)); } /// \brief Remove the given dimensional data. @@ -147,10 +147,11 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable { const std::map constant_labels_; mutable std::mutex mutex_; + using metrics_iterator = typename std::unordered_map>::iterator; + ClientMetric CollectMetric(std::size_t hash, T* metric) const; - T& Add(const std::map& labels, - std::unique_ptr object); - T* GetMetric(const std::map& labels) const; + T& Add(metrics_iterator hint, std::unique_ptr object); + metrics_iterator FindMetric(const std::map& labels); }; } // namespace prometheus diff --git a/core/src/family.cc b/core/src/family.cc index bfc2a095..969dc737 100644 --- a/core/src/family.cc +++ b/core/src/family.cc @@ -15,27 +15,8 @@ Family::Family(const std::string& name, const std::string& help, } template -T* Family::GetMetric(const std::map& labels) const { - auto hash = detail::hash_labels(labels); - std::lock_guard lock{mutex_}; - auto metrics_iter = metrics_.find(hash); - - if (metrics_iter == metrics_.end()) - return nullptr; - -#ifndef NDEBUG - auto labels_iter = labels_.find(hash); - assert(labels_iter != labels_.end()); - const auto& old_labels = labels_iter->second; - assert(labels == old_labels); -#endif - - return metrics_iter->second.get(); -} - -template -T& Family::Add(const std::map& labels, - std::unique_ptr object) { +typename Family::metrics_iterator +Family::FindMetric(const std::map& labels) { auto hash = detail::hash_labels(labels); std::lock_guard lock{mutex_}; auto metrics_iter = metrics_.find(hash); @@ -47,7 +28,7 @@ T& Family::Add(const std::map& labels, const auto& old_labels = labels_iter->second; assert(labels == old_labels); #endif - return *metrics_iter->second; + return metrics_iter; } else { #ifndef NDEBUG for (auto& label_pair : labels) { @@ -56,12 +37,23 @@ T& Family::Add(const std::map& labels, } #endif - auto metric = metrics_.insert(std::make_pair(hash, std::move(object))); + auto metric = metrics_.insert(std::make_pair(hash, nullptr)); assert(metric.second); labels_.insert({hash, labels}); - labels_reverse_lookup_.insert({metric.first->second.get(), hash}); - return *(metric.first->second); + return metric.first; + } +} + +template +T& Family::Add(metrics_iterator hint, std::unique_ptr object) { + std::lock_guard lock{mutex_}; + auto hash = hint->first; + assert(metrics_.find(hash) == hint); + if (! hint->second) { + labels_reverse_lookup_.insert({object.get(), hash}); + hint->second.swap(object); } + return *(hint->second); } template