Improve locks for multithreading (#10)

* Use std::unique_lock instead of std::mutex lock/unlock methods
* Improve std::vector initialization

lib/monitors/nvidiamonitor.cpp

@@ -133,16 +133,17 @@ void uprofile::NvidiaMonitor::watchGPU(int period)
  int err = read_nvidia_smi_stdout(stdout_fd, gpuUsage, usedMem, totalMem);
  if (err != 0) {
  cerr << errorMsg << ": read_error = " << strerror(err) << endl;
- m_mutex.lock();
+ unique_lock<mutex> lk(m_mutex);
  m_watching = false;
- m_mutex.unlock();
+ lk.unlock();
  break;
  }
- m_mutex.lock();
+
+ unique_lock<mutex> lk(m_mutex);
  m_gpuUsage = !gpuUsage.empty() ? stof(gpuUsage) : 0.f;
  m_usedMem = !usedMem.empty() ? stoi(usedMem) * 1024 : 0; // MiB to KiB
  m_totalMem = !totalMem.empty() ? stoi(totalMem) * 1024 : 0; // MiB to KiB
- m_mutex.unlock();
+ lk.unlock();
  }
  }));
  }
@@ -155,9 +156,9 @@ void uprofile::NvidiaMonitor::abortWatchGPU()
 {
 #if defined(__linux__)
  if (m_watcherThread) {
- m_mutex.lock();
+ unique_lock<mutex> lk(m_mutex);
  m_watching = false;
- m_mutex.unlock();
+ lk.unlock();
  m_watcherThread->join();
  m_watcherThread.reset();
  }

lib/uprofileimpl.cpp

@@ -67,17 +67,17 @@ void UProfileImpl::addGPUMonitor(IGPUMonitor* monitor)
  return;
  }
 
- if (!m_gpuMonitor) {
- removeGPUMonitor();
- }
+ removeGPUMonitor();
 
  m_gpuMonitor = monitor;
 }
 
 void UProfileImpl::removeGPUMonitor()
 {
- delete m_gpuMonitor;
- m_gpuMonitor = NULL;
+ if (m_gpuMonitor) {
+ delete m_gpuMonitor;
+ m_gpuMonitor = NULL;
+ }
 }
 
 void UProfileImpl::timeBegin(const std::string& title)
@@ -91,14 +91,14 @@ void UProfileImpl::timeEnd(const std::string& title)
  unsigned long long beginTimestamp = 0;
 
  // Find step in the map
- m_stepsMutex.lock();
+ unique_lock<mutex> lk(m_stepsMutex);
  auto it = m_steps.find(title);
  bool found = it != m_steps.end();
  if (found) {
  beginTimestamp = (*it).second;
  m_steps.erase(it);
  }
- m_stepsMutex.unlock();
+ lk.unlock();
 
  if (found) {
  write(ProfilingType::TIME_EXEC, {std::to_string(beginTimestamp), title});
@@ -236,7 +236,6 @@ void UProfileImpl::setTimestampUnit(TimestampUnit tsUnit)
 
 void UProfileImpl::write(ProfilingType type, const std::list<std::string>& data)
 {
- std::lock_guard<std::mutex> guard(m_fileMutex);
  if (m_file.is_open()) {
  const char csvSeparator = ';';
  std::string strType;
@@ -266,6 +265,7 @@ void UProfileImpl::write(ProfilingType type, const std::list<std::string>& data)
  strType = "undefined";
  break;
  }
+ std::lock_guard<std::mutex> guard(m_fileMutex);
  m_file << strType.c_str() << csvSeparator << getTimestamp();
  for (auto it = data.cbegin(); it != data.cend(); ++it) {
  m_file << csvSeparator << *it;

lib/util/cpumonitor.cpp

@@ -14,10 +14,10 @@
 using namespace std;
 
 uprofile::CpuMonitor::CpuMonitor() :
- m_nbCpus(getNumberOfCPUCores())
+ m_nbCpus(getNumberOfCPUCores()),
+ m_lastIdleTimes(m_nbCpus, 0),
+ m_lastTotalTimes(m_nbCpus, 0)
 {
- m_lastIdleTimes.resize(m_nbCpus, 0);
- m_lastTotalTimes.resize(m_nbCpus, 0);
 }
 
 uprofile::CpuMonitor::~CpuMonitor()
@@ -60,7 +60,7 @@ void uprofile::CpuMonitor::extractCpuTimes(const string& cpuInfo, size_t& idleTi
 
 vector<float> uprofile::CpuMonitor::getUsage()
 {
- vector<float> usages;
+ vector<float> usages(m_nbCpus, 0);
 #if defined(__linux__)
  ifstream procStat("/proc/stat");
  // /proc/stat dumps the following info:
@@ -85,7 +85,7 @@ vector<float> uprofile::CpuMonitor::getUsage()
 
  // To compute CPU load, we compute the time the CPU has been idle since the last read.
  float cpuLoad = 100.0 * (1.0 - (float)(idleTime - m_lastIdleTimes[cpuIndex]) / (totalTime - m_lastTotalTimes[cpuIndex]));
- usages.push_back(cpuLoad);
+ usages[cpuIndex] = cpuLoad;
 
  // Save the times value for the next read
  m_lastIdleTimes[cpuIndex] = idleTime;

lib/util/cpumonitor.h

@@ -30,11 +30,10 @@ class CpuMonitor
  static size_t getNumberOfCPUCores();
  static void extractCpuTimes(const string& cpuInfo, size_t& idleTime, size_t& totalTime);
 
+ size_t m_nbCpus;
  // Store the last idle and total time for each CPU
  vector<size_t> m_lastIdleTimes;
  vector<size_t> m_lastTotalTimes;
-
- size_t m_nbCpus;
 };
 }
 

lib/util/timer.cpp

@@ -10,7 +10,6 @@
 #include "timer.h"
 
 Timer::Timer(int interval) :
- m_th(NULL),
  m_interval(interval)
 {
 }
@@ -30,35 +29,34 @@ void Timer::setTimeout(const std::function<void(void)>& timeout)
  m_timeout = timeout;
 }
 
-bool Timer::isRunning()
+bool Timer::isRunning() const
 {
  std::lock_guard<std::mutex> lock(m_mutex);
  return m_running;
 }
 
 void Timer::start()
 {
- if (m_interval > 0 && m_th == NULL) {
+ if (!m_th && m_interval > 0) {
  m_running = true;
- m_th = new thread([=]() {
+ m_th = unique_ptr<thread>(new thread([=]() {
  while (isRunning()) {
  this_thread::sleep_for(chrono::milliseconds(m_interval));
  if (m_timeout) {
  m_timeout();
  }
  }
- });
+ }));
  }
 }
 
 void Timer::stop()
 {
- m_mutex.lock();
+ unique_lock<mutex> lk(m_mutex);
  m_running = false;
- m_mutex.unlock();
+ lk.unlock();
  if (m_th) {
  m_th->join();
- delete m_th;
- m_th = NULL;
+ m_th.reset();
  }
 }

lib/util/timer.h

@@ -13,6 +13,7 @@
 #include <chrono>
 #include <functional>
 #include <iostream>
+#include <memory>
 #include <mutex>
 #include <thread>
 
@@ -28,13 +29,13 @@ class Timer
  void setInterval(int interval);
  void start();
  void stop();
- bool isRunning();
+ bool isRunning() const;
 
 private:
- thread* m_th;
+ unique_ptr<thread> m_th;
  bool m_running;
  int m_interval;
- std::mutex m_mutex;
+ mutable std::mutex m_mutex;
  std::function<void(void)> m_timeout;
 };