A header only class for C++ time measurement

Question

I wrote a header only class (actually there is a second one for data bookkeeping and dumping) to measure the execution time of a C++ scope without worrying too much about boilerplates. The idea being to be able to simply instantiate a class at the begining of the scope you want to measure, and do a single call at the end to dump the measure.

I rely on the fact that the class instantiation is at the beginning of the scope, and its destruction at the very end. So my main worry is about compile time optimizations that could change the order of execution and bias the measure. Also I'm not satisfy on how I retrieve the type for ScopeTimer::Duration but I couldn't find the type properly :/

Here is the code:

scope_timer.hpp

#ifndef SCOPE_TIMER
#define SCOPE_TIMER
#include <chrono>
#include <string>
#include <vector>
#include <map>
#include <fstream>

class ScopeTimer {
public:
  using ScopeSignature = std::string;
  using DurationType = std::chrono::microseconds;
  using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());

  ScopeTimer(const ScopeSignature& scopeName);
  ~ScopeTimer();

  Duration  getDurationFromStart() const;
private:
  ScopeTimer();

  const ScopeSignature                                  scopeName;
  const std::chrono::high_resolution_clock::time_point  start;
};

class ScopeTimerStaticCore {
public:

  static void addTimingToNamedScope(const ScopeTimer::ScopeSignature& scopeName, const ScopeTimer::Duration& duration);
  static void dumpTimingToFile(const std::string& path);
  static void clearAllTiming();
  static void clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName);

private:
  using TimingVector = std::vector<ScopeTimer::Duration>;
  using ScopesTiming = std::map<ScopeTimer::ScopeSignature, TimingVector>;

  static ScopesTiming&  getScopesTimingStaticInstance() {
    static ScopesTiming scopesTimingContainer;

    return (scopesTimingContainer);
  };

};

/*******************************************************Implementations*******************************************************/

inline ScopeTimer::ScopeTimer(const ScopeSignature& scopeName) : scopeName(scopeName), start(std::chrono::high_resolution_clock::now()) {};

inline ScopeTimer::~ScopeTimer() {
  const Duration  scopeTimerLifetimeDuration = this->getDurationFromStart();

  ScopeTimerStaticCore::addTimingToNamedScope(this->scopeName, scopeTimerLifetimeDuration);
  return ;
};

inline ScopeTimer::Duration  ScopeTimer::getDurationFromStart() const {
  using std::chrono::duration_cast;

  const std::chrono::high_resolution_clock::time_point now = std::chrono::high_resolution_clock::now();

  return (duration_cast<DurationType>(now - this->start).count());
};


inline void  ScopeTimerStaticCore::addTimingToNamedScope(const ScopeTimer::ScopeSignature& scopeName, const ScopeTimer::Duration& duration) {
  ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();

  scopesTimingContainer[scopeName].push_back(duration);
  return ;
};

inline void  ScopeTimerStaticCore::dumpTimingToFile(const std::string& path) {
  const ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
  std::ofstream       dumpfile;

  dumpfile.open(path, std::ios::out | std::ios::trunc);
  for (ScopesTiming::const_iterator it_scopes = scopesTimingContainer.begin(); it_scopes != scopesTimingContainer.end(); ++it_scopes) {
    const ScopeTimer::ScopeSignature& currentScope = it_scopes->first;
    const TimingVector&               timings = it_scopes->second;

    for (TimingVector::const_iterator it_timings = timings.begin(); it_timings != timings.end(); ++it_timings)
      dumpfile << currentScope << "," << *it_timings << std::endl;
  }
  dumpfile.close();
  return ;
};

inline void  ScopeTimerStaticCore::clearAllTiming() {
  ScopesTiming& scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();

  scopesTimingContainer.clear();
  return ;
};

inline void  ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
  ScopesTiming&           scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
  ScopesTiming::iterator  it_scopes = scopesTimingContainer.find(scopeName);

  if (it_scopes != scopesTimingContainer.end())
    it_scopes->second.clear();
  return ;
};

#endif /* SCOPE_TIMER */

And an dummy program that use it

main.cpp

#include "../include/scope_timer.hpp"

void  functionA();
void  functionB();

int main() {
  for (size_t i = 0; i < 3; ++i) {
    functionA();
    functionB();
  }
  ScopeTimerStaticCore::dumpTimingToFile("/tmp/scope-timer_dump-dummy-test.csv");
  return (0);
};

dumb_functions.cpp

#include <thread>
#include <chrono>
#include "../include/scope_timer.hpp"

void  functionA() {
  ScopeTimer  scopeTimer("functionA");
  std::this_thread::sleep_for (std::chrono::milliseconds(500));
  return ;
};

void  functionB() {
  ScopeTimer  scopeTimer("functionB");
  std::this_thread::sleep_for (std::chrono::seconds(1));
  return ;
};

if you want to retrieve the code quickly here is the repo link

Answer 1

using Duration = decltype(std::chrono::duration_cast<DurationType>(std::chrono::high_resolution_clock::now() - std::chrono::high_resolution_clock::now()).count());

I'm confused by this line. decltype(std::chrono::duration_cast<DurationType>(expr)) is invariably DurationType, isn't it? That's why it's a "cast"? So this simplifies down to decltype(std::declval<DurationType&>().count()), which I'm pretty sure can be spelled as DurationType::rep unless you're really eager to support non-standard duration types that might not have a rep member. So:

using Duration = typename DurationType::rep;

And now it appears that maybe Duration is the wrong name for this typedef, eh?

(EDIT: Oops, the keyword typename is not needed here because DurationType is not dependent. Just using Duration = DurationType::rep; should be sufficient.)

---

static ScopesTiming&  getScopesTimingStaticInstance() {
  static ScopesTiming scopesTimingContainer;

  return (scopesTimingContainer);
};

Minor nits on whitespace and naming and parentheses and trailing semicolons:

static ScopesTiming& getScopesTimingStaticInstance() {
    static ScopesTiming instance;
    return instance;
}

The defining quality of instance is that it's a static instance of ScopesTiming. If you want to convey the additional information that ScopesTiming is actually a container type, then that information belongs in the name of the type. Personally I'd call it something like TimingVectorMap, because it's a map of TimingVectors.

---

Since the static map is not guarded by any mutex, your function addTimingToNamedScope (which mutates the map) is not safe to call from multiple threads concurrently. This could be a problem for real-world use.

---

ScopeTimer has two const-qualified fields. This doesn't do anything except pessimize its implicitly generated move-constructor into a copy-constructor. I recommend removing the const.

ScopeTimer also has an implicit conversion from ScopeSignature a.k.a. std::string, so that for example

void f(ScopeTimer timer);
std::string hello = "hello world";
f(hello);  // compiles without any diagnostic

I very strongly suggest that you never enable any implicit conversion unless you have a very good reason for it. This means putting explicit on every constructor and conversion operator.

explicit ScopeTimer(const ScopeSignature& scopeName);

---

dumpfile.open(path, std::ios::out | std::ios::trunc);

Should you check to see if the open succeeded?

---

inline void  ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
  ScopesTiming&           scopesTimingContainer = ScopeTimerStaticCore::getScopesTimingStaticInstance();
  ScopesTiming::iterator  it_scopes = scopesTimingContainer.find(scopeName);

  if (it_scopes != scopesTimingContainer.end())
    it_scopes->second.clear();
  return ;
};

This would be a good place to use C++11 auto:

inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
  ScopesTiming& instance = getScopesTimingStaticInstance();
  auto it = instance.find(scopeName);
  if (it != instance.end()) {
      it->second.clear();
  }
}

Or, if you don't mind removing the element from the map completely, you could just use erase:

inline void ScopeTimerStaticCore::clearTimingForNamedScope(const ScopeTimer::ScopeSignature& scopeName) {
  ScopesTiming& instance = getScopesTimingStaticInstance();
  instance.erase(scopeName);
}

I also notice that these functions would get a lot shorter and simpler to read if you put their definitions in-line into the class body of ScopeTimerStaticCore. In this case you could omit the keyword inline and the qualification of the parameter type:

void clearTimingForNamedScope(const ScopeSignature& scopeName) {
  ScopesTiming& instance = getScopesTimingStaticInstance();
  instance.erase(scopeName);
}

(Assuming that ScopeTimerStaticCore contains a member typedef using ScopeSignature = ScopeTimer::ScopeSignature;, I guess. It probably should — or vice versa.)