Remove mutex from Metric class (#95)

Author: awegrzyn

CMakeLists.txt

@@ -67,6 +67,7 @@ set(INCLUDE_OUTPUT_DIRECTORY "${PROJECT_BINARY_DIR}/include")
 set(SRCS
   src/Monitoring.cxx
   src/Metric.cxx
+  src/ComplexMetric.cxx
   src/Backends/InfluxDB.cxx
   src/Backends/Flume.cxx
   src/Backends/StdOut.cxx
@@ -145,9 +146,8 @@ set(EXAMPLES
   examples/6-Increment.cxx
   examples/7-Latency.cxx
   examples/8-Multiple.cxx
-  examples/9-Timer.cxx
+  examples/9-AutoUpdate.cxx
   examples/10-Buffering.cxx
-  examples/11-AutoUpdate.cxx
 )
 
 foreach (example ${EXAMPLES})

README.md

@@ -113,7 +113,6 @@ Each backend has its default verbosity (see backend in [Monitoring instance](#mo
 ### Customized metrics
 Two additional methods can be chained the to `send(Metric&& metric)` in order to __insert custom tags__ or __set custom timestamp__:
    + `addTags(std::vector<Tag>&& tags)`
-   + `setTimestamp(std::chrono::time_point<std::chrono::system_clock>& timestamp)`
 
 See how it works in the example: [examples/2-TaggedMetrics.cxx](examples/2-TaggedMetrics.cxx), [examples/3-UserDefinedTimestamp.cxx](examples/3-UserDefinedTimestamp.cxx).
 

examples/11-AutoUpdate.cxx examples/9-AutoUpdate.cxxexamples/11-AutoUpdate.cxx renamed to examples/9-AutoUpdate.cxx

@@ -10,9 +10,6 @@ using namespace o2::monitoring;
 int main() {
   auto monitoring = MonitoringFactory::Get("stdout://");
 
-  // Enable periodical value pushing (default every 1s)
-  monitoring->enableAutoPush();
-
   // Get reference to metrics
   auto& qcMetric = monitoring->getAutoPushMetric("qcMetric"); 
   auto& qcMetric2 = monitoring->getAutoPushMetric("qcMetric2");

examples/9-Timer.cxx

@@ -0,0 +1,60 @@
+///
+/// \file ComplexMetric.h
+/// \author Adam Wegrzynek <[email protected]>
+///
+
+#ifndef ALICEO2_MONITORING_CORE_COMPLEXMETRIC_H
+#define ALICEO2_MONITORING_CORE_COMPLEXMETRIC_H
+
+#include "Metric.h"
+#include <mutex>
+
+namespace o2
+{
+/// ALICE O2 Monitoring system
+namespace monitoring
+{
+
+// \brief Extends metric to value setter
+class ComplexMetric : public o2::monitoring::Metric
+{
+  public:
+    /// Integer metric construtor
+    /// \param value 	 	metric value (int)
+    /// \param name 	 	metric name
+    ComplexMetric(int value, const std::string& name);
+
+    /// String metric construtor
+    /// \param value            metric value (string)
+    /// \param name             the metric name
+    ComplexMetric(std::string value, const std::string& name);
+
+    /// Double metric constructor
+    /// \param value            metric value (double)
+    /// \param name             metric name
+    ComplexMetric(double value, const std::string& name);
+
+    /// uint64_t metric constructor
+    /// \param value            metric value (uint64_t)
+    /// \param name             metric name
+    ComplexMetric(uint64_t value, const std::string& name);
+
+    /// boost variant metric constructor, required by derived metrics logic
+    /// \param value            metric value (boost variant)
+    /// \param name             metric name
+    ComplexMetric(boost::variant< int, std::string, double, uint64_t >, const std::string& name);
+
+    /// Default destructor
+    ~ComplexMetric() = default;
+
+    // Resets metric's timestamp
+    void resetTimestamp();
+
+    /// Assign operator overload, assignes new values to the metric object
+    ComplexMetric& operator=(const boost::variant< int, std::string, double, uint64_t >& value);
+};
+
+} // namespace monitoring
+} // namespace o2
+
+#endif // ALICEO2_MONITORING_CORE_COMPLEXMETRIC_H

include/Monitoring/ComplexMetric.h

@@ -8,7 +8,6 @@
 
 #include <string>
 #include <chrono>
-#include <mutex>
 #include <vector>
 #include <boost/variant.hpp>
 #include "Tag.h"
@@ -58,15 +57,6 @@ class Metric
     /// Default destructor
     ~Metric() = default;
 
-    /// Copy initialization
-    Metric(const Metric& other);
-
-    /// Copy assignment
-    Metric& operator=(Metric const& other);
-
-    /// Assign operator overload, assignes new values to the metric object
-    Metric& operator=(const boost::variant< int, std::string, double, uint64_t >& value);
-        
     /// Name getter
     /// \return	metric name
     std::string getName() const;
@@ -96,7 +86,7 @@ class Metric
     /// return          timestamp as std::chrono::system_clock
     static auto getCurrentTimestamp() -> decltype(std::chrono::system_clock::now());
 
-  private:
+  protected:
     /// Metric value
     boost::variant< int, std::string, double, uint64_t > mValue;
 
@@ -108,9 +98,6 @@ class Metric
 
     /// Metric tags
     std::vector<Tag> tagSet;
-
-    /// Mutex for accesing metric value
-    mutable std::mutex mValueMutex;
 };
 
 } // namespace monitoring

include/Monitoring/Metric.h

@@ -17,6 +17,7 @@
 #include <vector>
 #include <deque>
 
+#include "Monitoring/ComplexMetric.h"
 #include "Monitoring/Backend.h"
 #include "Monitoring/DerivedMetrics.h"
 #include "Monitoring/ProcessMonitor.h"
@@ -76,16 +77,6 @@ class Monitoring
     /// \param interval		refresh interval
     void enableProcessMonitoring(const unsigned int interval = 5);
 
-    /// Starts timing
-    /// Sets a start timestamp and timeout
-    /// \param name 		metric name
-    void startTimer(std::string name);
-
-    /// Stops timing
-    /// Sets stop timestamp, calculates delta and sends value
-    /// \param name 		metric name
-    void stopAndSendTimer(std::string name);
-
     /// Flushes metric buffer (this can also happen when buffer is full)
     void flushBuffer();
 
@@ -101,11 +92,7 @@ class Monitoring
     /// Returns a metric which will be periodically sent to backends
     /// \param name 		metric name
     /// \return 		periodically send metric
-    Metric& getAutoPushMetric(std::string name);
-
-    /// Enables periodical push interval
-    /// \param interval 	interval in seconds
-    void enableAutoPush(const unsigned int interval = 1);
+    ComplexMetric& getAutoPushMetric(std::string name, unsigned int interval = 1);
 
   private:
     /// Derived metrics handler
@@ -115,9 +102,6 @@ class Monitoring
     /// Vector of backends (where metrics are passed to)
     std::vector <std::unique_ptr<Backend>> mBackends;
 
-    /// List of timers
-    std::unordered_map <std::string, std::chrono::time_point<std::chrono::steady_clock>> mTimers;
-
     /// Pushes metric to all backends or to the buffer
     void pushToBackends(Metric&& metric);
 
@@ -143,7 +127,7 @@ class Monitoring
     unsigned int mBufferSize;
 
     /// Store for automatically pushed metrics
-    std::deque<Metric> mPushStore;
+    std::deque<ComplexMetric> mPushStore;
 
     /// Process monitor interval
     std::atomic<unsigned int> mProcessMonitoringInterval;

include/Monitoring/Monitoring.h

@@ -0,0 +1,49 @@
+///
+/// \file ComplexMetric.cxx
+/// \author Adam Wegrzynek <[email protected]>
+///
+
+#include "Monitoring/ComplexMetric.h"
+
+#include <iostream>
+#include <chrono>
+#include <memory>
+
+namespace o2
+{
+/// ALICE O2 Monitoring system
+namespace monitoring
+{
+
+ComplexMetric::ComplexMetric(int value, const std::string& name) :
+  Metric(value, name)
+{}
+
+ComplexMetric::ComplexMetric(std::string value, const std::string& name) :
+  Metric(value, name)
+{}
+
+ComplexMetric::ComplexMetric(double value, const std::string& name) :
+  Metric(value, name)
+{}
+
+ComplexMetric::ComplexMetric(uint64_t value, const std::string& name) :
+  Metric(value, name)
+{}
+
+ComplexMetric::ComplexMetric(boost::variant< int, std::string, double, uint64_t > value, const std::string& name) :
+  Metric(value, name)
+{}
+
+void ComplexMetric::resetTimestamp()
+{
+   mTimestamp = Metric::getCurrentTimestamp();
+}
+
+ComplexMetric& ComplexMetric::operator=(const boost::variant< int, std::string, double, uint64_t >& value) {
+  mValue = value;
+  return *this;
+}
+
+} // namespace monitoring
+} // namespace o2

src/ComplexMetric.cxx

@@ -50,40 +50,11 @@ Metric::Metric(boost::variant< int, std::string, double, uint64_t > value, const
   mValue(value), mName(name), mTimestamp(timestamp)
 {}
 
-Metric::Metric(const Metric& other)
-{
-  std::lock_guard<std::mutex> lock(other.mValueMutex);
-  mName = other.mName;
-  mValue = other.mValue;
-  mTimestamp = other.mTimestamp;
-  tagSet = other.tagSet;
-}
-
-Metric& Metric::operator=(Metric const& other)
-{
-  if (&other != this) {
-    std::unique_lock<std::mutex> lockThis(mValueMutex, std::defer_lock);
-    std::unique_lock<std::mutex> lockOther(other.mValueMutex, std::defer_lock);
-    std::lock(lockThis, lockOther);
-
-    mName = other.mName;
-    mValue = other.mValue;
-    mTimestamp = other.mTimestamp;
-    tagSet = other.tagSet;
-  }
-  return *this;
-}
-
 boost::variant< int, std::string, double, uint64_t > Metric::getValue() const
 {
   return mValue;
 }
 
-Metric& Metric::operator=(const boost::variant< int, std::string, double, uint64_t >& value) {
-  mValue = value;
-  return *this;
-}
-
 Metric&& Metric::addTags(std::vector<Tag>&& tags)
 {
   tagSet = std::move(tags);

src/Metric.cxx

@@ -63,28 +63,6 @@ void Monitoring::enableProcessMonitoring(const unsigned int interval) {
   #endif
 }
 
-void Monitoring::startTimer(std::string name) {
-  auto search = mTimers.find(name);
-  if (search == mTimers.end()) {
-    auto now = std::chrono::steady_clock::now();
-    mTimers.insert(std::make_pair(name, now));
-  } else {
-    MonLogger::Get() << "Monitoring timer : Timer for " << name << " already started" << MonLogger::End();
-  }
-}
-
-void Monitoring::stopAndSendTimer(std::string name) {
-  auto search = mTimers.find(name);
-  if (search != mTimers.end()) {
-    auto now = std::chrono::duration_cast <std::chrono::milliseconds>(std::chrono::steady_clock::now().time_since_epoch()).count();
-    auto start = std::chrono::duration_cast <std::chrono::milliseconds>(search->second.time_since_epoch()).count();
-    uint64_t duration = now - start;
-    send({duration, name});
-  } else {
-    MonLogger::Get() << "Monitoring timer : Cannot stop " << name << " timer as it hasn't started" << MonLogger::End();
-  }
-}
-
 void Monitoring::addGlobalTag(std::string name, std::string value)
 {
   for (auto& backend: mBackends) {
@@ -124,7 +102,8 @@ void Monitoring::pushLoop()
 
     if (mAutoPushInterval != 0 && (loopCount % (mAutoPushInterval*10)) == 0) {
       std::vector<Metric> metrics;
-      for (auto& metric : mPushStore) {
+      for (auto metric : mPushStore) {
+        metric.resetTimestamp();
         metrics.push_back(metric);
       }
       send(std::move(metrics));
@@ -134,10 +113,12 @@ void Monitoring::pushLoop()
   }
 }
 
-Metric& Monitoring::getAutoPushMetric(std::string name)
+ComplexMetric& Monitoring::getAutoPushMetric(std::string name, unsigned int interval)
 {
-  if (mAutoPushInterval == 0) {
-    MonLogger::Get() << "[WARN] AutoPush is not enabled" << MonLogger::End();
+  if (!mMonitorRunning) {
+    mMonitorRunning = true;
+    mMonitorThread = std::thread(&Monitoring::pushLoop, this);
+    mAutoPushInterval = interval;
   }
   mPushStore.emplace_back(boost::variant< int, std::string, double, uint64_t > {}, name);
   return mPushStore.back();
@@ -166,15 +147,6 @@ void Monitoring::debug(Metric&& metric)
   }
 }
 
-void Monitoring::enableAutoPush(unsigned int interval)
-{
-  if (!mMonitorRunning) {
-    mMonitorRunning = true;
-    mMonitorThread = std::thread(&Monitoring::pushLoop, this);
-  }
-  mAutoPushInterval = interval;
-}
-
 void Monitoring::pushToBackends(Metric&& metric)
 {
   if (mBuffering) {