Time spend in scoped timer

Question

I want to check some timing of my (pre-C11) C++ and wrote a class:

class ScopedTimer {
    timespec start_,end_;
    std::string name_;      /// name of this timer (used in output)
    bool active_;           /// if not active, no output is printed
    clockid_t clock_id_;    /// clock type used for this timer

public:
    ScopedTimer(std::string name, clockid_t clock_id = CLOCK_REALTIME, bool active = true):
        name_(name),
        active_(active),
        clock_id_(clock_id)
    {
        clock_gettime(clock_id_, &start_);
    }

    ~ScopedTimer(){
        if (active_){
            clock_gettime(clock_id_, &end_);

            double dt_ms = (end_.tv_sec - start_.tv_sec) * 1000.0 + (end_.tv_nsec - start_.tv_nsec) / 1000000.0;

             cout << "runtime " << name_ << "  " << dt_ms  << "  ms" << endl;
        }
    }

};

It is rather easy to check the running time of a full function:

void foo()
{
 ScopedTimer t("foo");
  do_stuff();
}

or also to check a part of a function:

void findCorners(cv::Mat& img, bool with_timing_output)
{
  cv::Mat gx_dil;
  cv::Mat dil_element_x = createDilElement(1);
    {
     ScopedTimer t("dilation during findCorners",with_timing_output);
     cv::dilate(gx_img, gx_dil, dil_element_x, Point(-1, -1), 5, BORDER_DEFAULT);
    }
}

What do you think about it?

Answer 1

It looks fine to me and it does the job. I would only consider changing a couple things:

1. Decouple it from the platform dependent clock_gettime. This piece of code could be useful on any platform that supports time queries with millisecond precision (basically anything nowadays). If you encapsulate that aspect a little more, it can be easily ported to say, Windows using the QueryPerformance*() WinAPI functions. That clockid_t parameter in the constructor is the only bit leaking a platform specific object to the user, which you can easily remove or replace by a portable typedef/constant.

2. Support a generic output stream instead of hardcoding std::cout. Also very simple to do, just add an extra std::ostream& member an take an extra parameter in the constructor, which can be defaulted to std::cout.

3. Unless you want the stream to be flushed after printing a timing, replace endl with a \n. Flushing the stream might have performance implication if you are doing a lot of instrumentation in the code.

4. Not of consequence in this case, since this class is meant to be a stack-lived variable, however, I'll note it anyways as a general guideline: Avoid declaring booleans and variables smaller than the native word size in the middle of a class. e.g.:

   timespec start_,end_;
   std::string name_;      
   bool active_;        // <--- Not the best place for it
   clockid_t clock_id_;   
   

   On most compilers/platforms, a boolean is the size of a byte, which in that case will introduce padding if declared in the middle of other member variables of different sizes. A good rule of thumb is to always place the small fields, like bools, chars and the like at the very end of the member data declarations, so any padding added by the compiler stays minimal and at the end. Again, not of importance in you code, just thought it would be interesting to mention for future reference.