A header only class for C++ time measurement












4












$begingroup$


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










share|improve this question











$endgroup$

















    4












    $begingroup$


    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










    share|improve this question











    $endgroup$















      4












      4








      4





      $begingroup$


      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










      share|improve this question











      $endgroup$




      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







      c++ performance c++11 benchmarking






      share|improve this question















      share|improve this question













      share|improve this question




      share|improve this question








      edited Jan 20 at 15:46







      shorty_ponton

















      asked Jan 20 at 14:32









      shorty_pontonshorty_ponton

      234




      234






















          1 Answer
          1






          active

          oldest

          votes


















          4












          $begingroup$

          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.)






          share|improve this answer











          $endgroup$













          • $begingroup$
            I cannot compile the example code when I try to use DurationType::rep :/ About the codestyle I'll add some of your suggestions thanks Totally agree for the "open check" "full erase" and "explicit declaration" (I didn't know about that last one, so big win for me thanks)
            $endgroup$
            – shorty_ponton
            Jan 22 at 9:37










          • $begingroup$
            Works fine for me, but I admit that the typename is not needed, because DurationType is not template-dependent. If your compiler is complaining about the typename keyword, you can safely remove it. Otherwise, it should work fine.
            $endgroup$
            – Quuxplusone
            Jan 22 at 17:49











          Your Answer





          StackExchange.ifUsing("editor", function () {
          return StackExchange.using("mathjaxEditing", function () {
          StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
          StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
          });
          });
          }, "mathjax-editing");

          StackExchange.ifUsing("editor", function () {
          StackExchange.using("externalEditor", function () {
          StackExchange.using("snippets", function () {
          StackExchange.snippets.init();
          });
          });
          }, "code-snippets");

          StackExchange.ready(function() {
          var channelOptions = {
          tags: "".split(" "),
          id: "196"
          };
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function() {
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled) {
          StackExchange.using("snippets", function() {
          createEditor();
          });
          }
          else {
          createEditor();
          }
          });

          function createEditor() {
          StackExchange.prepareEditor({
          heartbeatType: 'answer',
          autoActivateHeartbeat: false,
          convertImagesToLinks: false,
          noModals: true,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          imageUploader: {
          brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
          contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
          allowUrls: true
          },
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          });


          }
          });














          draft saved

          draft discarded


















          StackExchange.ready(
          function () {
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f211857%2fa-header-only-class-for-c-time-measurement%23new-answer', 'question_page');
          }
          );

          Post as a guest















          Required, but never shown

























          1 Answer
          1






          active

          oldest

          votes








          1 Answer
          1






          active

          oldest

          votes









          active

          oldest

          votes






          active

          oldest

          votes









          4












          $begingroup$

          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.)






          share|improve this answer











          $endgroup$













          • $begingroup$
            I cannot compile the example code when I try to use DurationType::rep :/ About the codestyle I'll add some of your suggestions thanks Totally agree for the "open check" "full erase" and "explicit declaration" (I didn't know about that last one, so big win for me thanks)
            $endgroup$
            – shorty_ponton
            Jan 22 at 9:37










          • $begingroup$
            Works fine for me, but I admit that the typename is not needed, because DurationType is not template-dependent. If your compiler is complaining about the typename keyword, you can safely remove it. Otherwise, it should work fine.
            $endgroup$
            – Quuxplusone
            Jan 22 at 17:49
















          4












          $begingroup$

          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.)






          share|improve this answer











          $endgroup$













          • $begingroup$
            I cannot compile the example code when I try to use DurationType::rep :/ About the codestyle I'll add some of your suggestions thanks Totally agree for the "open check" "full erase" and "explicit declaration" (I didn't know about that last one, so big win for me thanks)
            $endgroup$
            – shorty_ponton
            Jan 22 at 9:37










          • $begingroup$
            Works fine for me, but I admit that the typename is not needed, because DurationType is not template-dependent. If your compiler is complaining about the typename keyword, you can safely remove it. Otherwise, it should work fine.
            $endgroup$
            – Quuxplusone
            Jan 22 at 17:49














          4












          4








          4





          $begingroup$

          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.)






          share|improve this answer











          $endgroup$



          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.)







          share|improve this answer














          share|improve this answer



          share|improve this answer








          edited Jan 22 at 17:50

























          answered Jan 20 at 16:09









          QuuxplusoneQuuxplusone

          12.3k12061




          12.3k12061












          • $begingroup$
            I cannot compile the example code when I try to use DurationType::rep :/ About the codestyle I'll add some of your suggestions thanks Totally agree for the "open check" "full erase" and "explicit declaration" (I didn't know about that last one, so big win for me thanks)
            $endgroup$
            – shorty_ponton
            Jan 22 at 9:37










          • $begingroup$
            Works fine for me, but I admit that the typename is not needed, because DurationType is not template-dependent. If your compiler is complaining about the typename keyword, you can safely remove it. Otherwise, it should work fine.
            $endgroup$
            – Quuxplusone
            Jan 22 at 17:49


















          • $begingroup$
            I cannot compile the example code when I try to use DurationType::rep :/ About the codestyle I'll add some of your suggestions thanks Totally agree for the "open check" "full erase" and "explicit declaration" (I didn't know about that last one, so big win for me thanks)
            $endgroup$
            – shorty_ponton
            Jan 22 at 9:37










          • $begingroup$
            Works fine for me, but I admit that the typename is not needed, because DurationType is not template-dependent. If your compiler is complaining about the typename keyword, you can safely remove it. Otherwise, it should work fine.
            $endgroup$
            – Quuxplusone
            Jan 22 at 17:49
















          $begingroup$
          I cannot compile the example code when I try to use DurationType::rep :/ About the codestyle I'll add some of your suggestions thanks Totally agree for the "open check" "full erase" and "explicit declaration" (I didn't know about that last one, so big win for me thanks)
          $endgroup$
          – shorty_ponton
          Jan 22 at 9:37




          $begingroup$
          I cannot compile the example code when I try to use DurationType::rep :/ About the codestyle I'll add some of your suggestions thanks Totally agree for the "open check" "full erase" and "explicit declaration" (I didn't know about that last one, so big win for me thanks)
          $endgroup$
          – shorty_ponton
          Jan 22 at 9:37












          $begingroup$
          Works fine for me, but I admit that the typename is not needed, because DurationType is not template-dependent. If your compiler is complaining about the typename keyword, you can safely remove it. Otherwise, it should work fine.
          $endgroup$
          – Quuxplusone
          Jan 22 at 17:49




          $begingroup$
          Works fine for me, but I admit that the typename is not needed, because DurationType is not template-dependent. If your compiler is complaining about the typename keyword, you can safely remove it. Otherwise, it should work fine.
          $endgroup$
          – Quuxplusone
          Jan 22 at 17:49


















          draft saved

          draft discarded




















































          Thanks for contributing an answer to Code Review Stack Exchange!


          • Please be sure to answer the question. Provide details and share your research!

          But avoid



          • Asking for help, clarification, or responding to other answers.

          • Making statements based on opinion; back them up with references or personal experience.


          Use MathJax to format equations. MathJax reference.


          To learn more, see our tips on writing great answers.




          draft saved


          draft discarded














          StackExchange.ready(
          function () {
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f211857%2fa-header-only-class-for-c-time-measurement%23new-answer', 'question_page');
          }
          );

          Post as a guest















          Required, but never shown





















































          Required, but never shown














          Required, but never shown












          Required, but never shown







          Required, but never shown

































          Required, but never shown














          Required, but never shown












          Required, but never shown







          Required, but never shown







          Popular posts from this blog

          Mario Kart Wii

          The Binding of Isaac: Rebirth/Afterbirth

          What does “Dominus providebit” mean?