Simplify gauge and histogram implementation. (#922)

Use mutex locker instead of atomic operations in Gauge.
Remove unnecessary constructor overload from Histogram.

cartographer_ros/cartographer_ros/metrics/internal/counter.h

@@ -29,11 +29,11 @@ class Counter : public ::cartographer::metrics::Counter {
   explicit Counter(const std::map<std::string, std::string>& labels)
       : gauge_(labels) {}
 
-  void Increment(const double by_value) override { gauge_.Increment(by_value); }
+  void Increment(const double value) override { gauge_.Increment(value); }
 
   void Increment() override { gauge_.Increment(); }
 
-  double Value() const { return gauge_.Value(); }
+  double Value() { return gauge_.Value(); }
 
   cartographer_ros_msgs::Metric ToRosMessage() {
     cartographer_ros_msgs::Metric msg = gauge_.ToRosMessage();

cartographer_ros/cartographer_ros/metrics/internal/family.h

@@ -62,7 +62,7 @@ class HistogramFamily : public ::cartographer::metrics::Family<
  public:
   HistogramFamily(const std::string& name, const std::string& description,
                   const BucketBoundaries& boundaries)
-      : boundaries_(boundaries) {}
+      : name_(name), description_(description), boundaries_(boundaries) {}
 
   Histogram* Add(const std::map<std::string, std::string>& labels) override;
 

cartographer_ros/cartographer_ros/metrics/internal/gauge.h

@@ -17,10 +17,10 @@
 #ifndef CARTOGRAPHER_ROS_METRICS_INTERNAL_GAUGE_H
 #define CARTOGRAPHER_ROS_METRICS_INTERNAL_GAUGE_H
 
-#include <atomic>
 #include <map>
 #include <string>
 
+#include "cartographer/common/mutex.h"
 #include "cartographer/metrics/gauge.h"
 #include "cartographer_ros_msgs/Metric.h"
 
@@ -30,23 +30,25 @@ namespace metrics {
 class Gauge : public ::cartographer::metrics::Gauge {
  public:
   explicit Gauge(const std::map<std::string, std::string>& labels)
-      : labels_(labels) {}
+      : labels_(labels), value_(0.) {}
 
-  void Decrement(const double by_value) override { Add(-1. * by_value); }
+  void Decrement(const double value) override { Add(-1. * value); }
 
   void Decrement() override { Decrement(1.); }
 
-  void Increment(const double by_value) override { Add(by_value); }
+  void Increment(const double value) override { Add(value); }
 
   void Increment() override { Increment(1.); }
 
   void Set(double value) override {
-    double expected = value_.load();
+    ::cartographer::common::MutexLocker lock(&mutex_);
-    while (!value_.compare_exchange_weak(expected, value)) {
+    value_ = value;
-      ;
+  }
-    }
+
+  double Value() {
+    ::cartographer::common::MutexLocker lock(&mutex_);
+    return value_;
   }
-  double Value() const { return value_.load(); }
 
   cartographer_ros_msgs::Metric ToRosMessage() {
     cartographer_ros_msgs::Metric msg;
@@ -63,12 +65,13 @@ class Gauge : public ::cartographer::metrics::Gauge {
 
  private:
   void Add(const double value) {
-    double current = Value();
+    ::cartographer::common::MutexLocker lock(&mutex_);
-    Set(current + value);
+    value_ += value;
   }
 
+  cartographer::common::Mutex mutex_;
   const std::map<std::string, std::string> labels_;
-  std::atomic<double> value_{0.};
+  double value_ GUARDED_BY(mutex_);
 };
 
 }  // namespace metrics

cartographer_ros/cartographer_ros/metrics/internal/histogram.cc

@@ -36,15 +36,11 @@ Histogram::Histogram(const std::map<std::string, std::string>& labels,
                        std::end(bucket_boundaries_)));
 }
 
-Histogram::Histogram(const BucketBoundaries& bucket_boundaries)
-    : Histogram({}, bucket_boundaries) {}
-
 void Histogram::Observe(double value) {
   auto bucket_index =
       std::distance(bucket_boundaries_.begin(),
                     std::upper_bound(bucket_boundaries_.begin(),
                                      bucket_boundaries_.end(), value));
-  // TODO: check lock contention and potential for atomic operations.
   ::cartographer::common::MutexLocker lock(&mutex_);
   sum_ += value;
   bucket_counts_[bucket_index] += 1;

cartographer_ros/cartographer_ros/metrics/internal/histogram.h

@@ -36,8 +36,6 @@ class Histogram : public ::cartographer::metrics::Histogram {
   explicit Histogram(const std::map<std::string, std::string>& labels,
                      const BucketBoundaries& bucket_boundaries);
 
-  explicit Histogram(const BucketBoundaries& bucket_boundaries);
-
   void Observe(double value) override;
 
   std::map<double, double> CountsByBucket();

cartographer_ros/cartographer_ros/metrics/internal/metrics_test.cc

@@ -53,7 +53,7 @@ TEST(Metrics, CounterTest) {
 
 TEST(Metrics, HistogramFixedWidthTest) {
   auto boundaries = ::cartographer::metrics::Histogram::FixedWidth(1, 3);
-  Histogram histogram(boundaries);
+  Histogram histogram({}, boundaries);
 
   // Observe some values that fit in finite buckets.
   std::array<double, 3> values = {{0., 2, 2.5}};
@@ -78,7 +78,7 @@ TEST(Metrics, HistogramFixedWidthTest) {
 TEST(Metrics, HistogramScaledPowersOfTest) {
   auto boundaries =
       ::cartographer::metrics::Histogram::ScaledPowersOf(2, 1, 2048);
-  Histogram histogram(boundaries);
+  Histogram histogram({}, boundaries);
 
   // Observe some values that fit in finite buckets.
   std::array<double, 3> values = {{256, 512, 666}};