Skip to content

Commit

Permalink
feat(core): Don't use atomics if access is always protected by a mutex
Browse files Browse the repository at this point in the history
```
before:
--------------------------------------------------------------------
Benchmark                          Time             CPU   Iterations
--------------------------------------------------------------------
BM_Histogram_Observe/0          79.1 ns         79.1 ns      8292745
BM_Histogram_Observe/0          76.9 ns         76.9 ns      8572721
BM_Histogram_Observe/1          77.0 ns         77.0 ns      8621758
BM_Histogram_Observe/8          79.5 ns         79.5 ns      8611917
BM_Histogram_Observe/64         88.4 ns         88.4 ns      7598500
BM_Histogram_Observe/512         155 ns          155 ns      4451089
BM_Histogram_Observe/4096        685 ns          685 ns      1022647
BM_Histogram_Collect/0          30.2 ns         30.2 ns     22709077
BM_Histogram_Collect/0          30.8 ns         30.8 ns     23167482
BM_Histogram_Collect/1          29.9 ns         29.9 ns     22629327
BM_Histogram_Collect/8          52.1 ns         52.1 ns     13481123
BM_Histogram_Collect/64          275 ns          275 ns      2516561
BM_Histogram_Collect/512        2092 ns         2092 ns       334960
BM_Histogram_Collect/4096      16551 ns        16550 ns        41957

after:
--------------------------------------------------------------------
Benchmark                          Time             CPU   Iterations
--------------------------------------------------------------------
BM_Histogram_Observe/0          62.4 ns         62.4 ns     10253322
BM_Histogram_Observe/0          62.5 ns         62.5 ns     11240054
BM_Histogram_Observe/1          63.0 ns         63.0 ns     11206021
BM_Histogram_Observe/8          68.3 ns         68.3 ns     10251804
BM_Histogram_Observe/64         77.9 ns         77.9 ns      8973251
BM_Histogram_Observe/512         146 ns          146 ns      4800350
BM_Histogram_Observe/4096        702 ns          702 ns       998891
BM_Histogram_Collect/0          28.7 ns         28.7 ns     23895049
BM_Histogram_Collect/0          29.2 ns         29.2 ns     23922703
BM_Histogram_Collect/1          28.6 ns         28.6 ns     23867051
BM_Histogram_Collect/8          47.5 ns         47.5 ns     14753513
BM_Histogram_Collect/64          270 ns          270 ns      2597279
BM_Histogram_Collect/512        2073 ns         2073 ns       339326
BM_Histogram_Collect/4096      16438 ns        16437 ns        42435

```
  • Loading branch information
gjasny committed Nov 12, 2021
1 parent b6615c7 commit 12313b7
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 11 deletions.
6 changes: 2 additions & 4 deletions core/include/prometheus/histogram.h
Expand Up @@ -4,10 +4,8 @@
#include <vector>

#include "prometheus/client_metric.h"
#include "prometheus/counter.h"
#include "prometheus/detail/builder.h" // IWYU pragma: export
#include "prometheus/detail/core_export.h"
#include "prometheus/gauge.h"
#include "prometheus/metric_type.h"

namespace prometheus {
Expand Down Expand Up @@ -70,8 +68,8 @@ class PROMETHEUS_CPP_CORE_EXPORT Histogram {
private:
const BucketBoundaries bucket_boundaries_;
mutable std::mutex mutex_;
std::vector<Counter> bucket_counts_;
Gauge sum_;
std::vector<double> bucket_counts_;
double sum_;
};

/// \brief Return a builder to configure and register a Histogram metric.
Expand Down
20 changes: 13 additions & 7 deletions core/src/histogram.cc
Expand Up @@ -12,7 +12,9 @@
namespace prometheus {

Histogram::Histogram(const BucketBoundaries& buckets)
: bucket_boundaries_{buckets}, bucket_counts_{buckets.size() + 1}, sum_{} {
: bucket_boundaries_{buckets},
bucket_counts_(buckets.size() + 1u, 0.0),
sum_{0.0} {
assert(std::is_sorted(std::begin(bucket_boundaries_),
std::end(bucket_boundaries_)));
}
Expand All @@ -26,8 +28,10 @@ void Histogram::Observe(const double value) {
[value](const double boundary) { return boundary >= value; })));

std::lock_guard<std::mutex> lock(mutex_);
sum_.Increment(value);
bucket_counts_[bucket_index].Increment();
sum_ += value;
if (!(value < 0.0)) {
bucket_counts_[bucket_index]++;
}
}

void Histogram::ObserveMultiple(const std::vector<double>& bucket_increments,
Expand All @@ -39,10 +43,12 @@ void Histogram::ObserveMultiple(const std::vector<double>& bucket_increments,
}

std::lock_guard<std::mutex> lock(mutex_);
sum_.Increment(sum_of_values);
sum_ += sum_of_values;

for (std::size_t i{0}; i < bucket_counts_.size(); ++i) {
bucket_counts_[i].Increment(bucket_increments[i]);
if (!(bucket_increments[i] < 0.0)) {
bucket_counts_[i] += bucket_increments[i];
}
}
}

Expand All @@ -54,7 +60,7 @@ ClientMetric Histogram::Collect() const {
auto cumulative_count = 0ULL;
metric.histogram.bucket.reserve(bucket_counts_.size());
for (std::size_t i{0}; i < bucket_counts_.size(); ++i) {
cumulative_count += bucket_counts_[i].Value();
cumulative_count += bucket_counts_[i];
auto bucket = ClientMetric::Bucket{};
bucket.cumulative_count = cumulative_count;
bucket.upper_bound = (i == bucket_boundaries_.size()
Expand All @@ -63,7 +69,7 @@ ClientMetric Histogram::Collect() const {
metric.histogram.bucket.push_back(std::move(bucket));
}
metric.histogram.sample_count = cumulative_count;
metric.histogram.sample_sum = sum_.Value();
metric.histogram.sample_sum = sum_;

return metric;
}
Expand Down

0 comments on commit 12313b7

Please sign in to comment.