trjhtr

A very bare-bones timer class created using the std::chrono library. I would like to know if any optimisations are possible and just to clarity, I am using MSVC (this code will not compile with certain compilers).

Timer.h:

#pragma once

#include <chrono>

class Timer

private:
 std::chrono::time_point<std::chrono::steady_clock> m_StartTime;

public:
 void Start();
 float GetDuration();
;

Timer.cpp:

#include "Timer.h"

void Timer::Start()

 m_StartTime = std::chrono::high_resolution_clock::now();


float Timer::GetDuration()

 std::chrono::duration<float> duration = std::chrono::high_resolution_clock::now() - m_StartTime;
 return duration.count();

Keep solid picture of usage in your head

When creating a library, it is extremely important to always keep the intended usage in mind. If the usage syntax is disgusting, it is better to think of a new one before writing the library (of course within reasons). In this case, I believe it is not what you wanted.

Make interfaces easy to use correctly and hard to use incorrectly

Start() function could be built into constructor. If user didn't use Start(), they'll get wrong results. Constructor, on the other hand, will enforce initialization. Never write non-fully-initializable objects, unless it is not possible to do otheriwse (extremely rarely). Then, users will only need to create object at the right place.

Don't mix clocks

Doing this:

std::chrono::time_point<std::chrono::steady_clock> m_StartTime;

and then

m_StartTime = std::chrono::high_resolution_clock::now(); 

is extremely bad idea. Not all standard libraries alias std::chrono::high_resolution_clock to steady_clock. Though I guess this is a feature of the code rather than a bug. The above will trigger compilation error in libc++ at least.

Know the subject

I know this is extremely simple clock, but since std::chrono::high_resolution_clock is used, I'd expect at least some mechanism to prevent compiler/execution unit optimizations, like std::atomic_thread_fence around it to prevent compiler optimizations and in general, make the execution point in which the time is taken observable. If none of what I said makes any sense to you, then your usage cases don't care about this.

Make it more reusable

I'd expect something like this:

template <typename Clock = std::chrono::high_resolution_lock>
class stopwatch

 /*...*/
;

Both time_point and duration are inferrable from Clock. I'd also like to see this:

template <typename Units = std::chrono::nanoseconds, typename Rep = double>
Rep elapsed_time()

 //ns chrono omitted for brevity
 return static_cast<Rep>(duration_cast<Units>(/*...*/).count());

(The static_cast is just to remove warnings when narrowing is done, which is intentional in this case, as default doesn't perform narrowing).

The project might suggest to change ordering of the template parameters, as Rep might be used more compared to Units.

Minor

• Naming

CamelCase is usually used for type names in C++, the rest is camelCase. I personally prefer snake_case for everything, except template parameter and concept names, for which I use CamelCase. It is a matter of preference and consistency though.

• Make header only if feasible

In this case, adding .cpp file makes it harder to use. .cpp file doesn't seem to make any difference, as any linkage nuances are not mentioned in the post. Also, making it header-only will make it even more simpler, which is I guess, the intention.

Putting it all together

Final code roughly looks like this:

#include <chrono>
#include <atomic>

namespace shino

 template <typename Clock = std::chrono::high_resolution_clock>
 class stopwatch
 
 const typename Clock::time_point start_point;
 public:
 stopwatch() : 
 start_point(Clock::now())
 

 template <typename Rep = typename Clock::duration::rep, typename Units = typename Clock::duration>
 Rep elapsed_time() const
 
 std::atomic_thread_fence(std::memory_order_relaxed);
 auto counted_time = std::chrono::duration_cast<Units>(Clock::now() - start_point).count();
 std::atomic_thread_fence(std::memory_order_relaxed);
 return static_cast<Rep>(counted_time);
 
 ;

 using precise_stopwatch = stopwatch<>;
 using system_stopwatch = stopwatch<std::chrono::system_clock>;
 using monotonic_stopwatch = stopwatch<std::chrono::steady_clock>;

As you probably noticed, I changed default template arguments in elapsed_time(), and also incorporated const correctness from @JDługosz answer. The defaults change is to support all possible clocks and still avoid lossy conversionsby default.

Lets use this:

Measuring scheduler overhead using stopwatch

The measurements are rather naive:

#include <iostream>
#include <thread>

int main()

 std::cout << "testing scheduler overheadn";
 using namespace std::literals;
 for (auto wait_time = 100ms; wait_time <= 1s; wait_time += 100ms)
 
 shino::precise_stopwatch stopwatch;
 std::this_thread::sleep_for(wait_time);
 auto actual_wait_time = stopwatch.elapsed_time<unsigned int, std::chrono::microseconds>();
 std::cout << "Scheduler overhead is roughly " << actual_wait_time - (wait_time + 0us).count() << " microseconds"
 << " for " << wait_time.count() << " milliseconds of requested sleep timen";
 

(Weird +0us is to let compiler handle the conversion from ms to us)

On my system gives:

testing scheduler overhead
Scheduler overhead is roughly 114 microseconds for 100 milliseconds of requested sleep time
Scheduler overhead is roughly 110 microseconds for 200 milliseconds of requested sleep time
Scheduler overhead is roughly 75 microseconds for 300 milliseconds of requested sleep time
Scheduler overhead is roughly 111 microseconds for 400 milliseconds of requested sleep time
Scheduler overhead is roughly 111 microseconds for 500 milliseconds of requested sleep time
Scheduler overhead is roughly 107 microseconds for 600 milliseconds of requested sleep time
Scheduler overhead is roughly 110 microseconds for 700 milliseconds of requested sleep time
Scheduler overhead is roughly 110 microseconds for 800 milliseconds of requested sleep time
Scheduler overhead is roughly 112 microseconds for 900 milliseconds of requested sleep time
Scheduler overhead is roughly 93 microseconds for 1000 milliseconds of requested sleep time

Full code to copy and run

#include <chrono>
#include <atomic>

namespace shino

 template <typename Clock = std::chrono::high_resolution_clock>
 class stopwatch
 
 const typename Clock::time_point start_point;
 public:
 stopwatch() : 
 start_point(Clock::now())
 

 template <typename Rep = typename Clock::duration::rep, typename Units = typename Clock::duration>
 Rep elapsed_time() const
 
 std::atomic_thread_fence(std::memory_order_relaxed);
 auto counted_time = std::chrono::duration_cast<Units>(Clock::now() - start_point).count();
 std::atomic_thread_fence(std::memory_order_relaxed);
 return static_cast<Rep>(counted_time);
 
 ;

 using precise_stopwatch = stopwatch<>;
 using system_stopwatch = stopwatch<std::chrono::system_clock>;
 using monotonic_stopwatch = stopwatch<std::chrono::steady_clock>;


#include <iostream>
#include <thread>

int main()

 std::cout << "testing scheduler overheadn";
 using namespace std::literals;
 for (auto wait_time = 100ms; wait_time <= 1s; wait_time += 100ms)
 
 shino::precise_stopwatch stopwatch;
 std::this_thread::sleep_for(wait_time);
 auto actual_wait_time = stopwatch.elapsed_time<unsigned int, std::chrono::microseconds>();
 std::cout << "Scheduler overhead is roughly " << actual_wait_time - (wait_time + 0us).count() << " microseconds"
 << " for " << wait_time.count() << " milliseconds of requested sleep timen";
 

Wandbox demo.

float Timer::GetDuration()

 std::chrono::duration<float> duration = std::chrono::high_resolution_clock::now() - m_StartTime;
 return duration.count();

Why isn’t this a const member?

Is there some reason not to use auto when declaring the local variable duration?

I would leave the type and period explicitly to the user by templating them. The unit of the float is otherwise unclear.

#include <chrono> 

template<typename type = float, typename period = std::milli>
class stopwatch

public:
 using clock = std::chrono::high_resolution_clock;
 using duration = std::chrono::duration<type, period>;
 using time_point = std::chrono::time_point<clock, duration>;

 stopwatch () : time_(clock::now()) 
 stopwatch (const stopwatch& that) = default;
 stopwatch ( stopwatch&& temp) = default;
 ~stopwatch () = default;
 stopwatch& operator=(const stopwatch& that) = default;
 stopwatch& operator=( stopwatch&& temp) = default;

 duration tick ()
 
 time_point time = clock::now();
 duration delta = time - time_;
 time_ = time;
 return delta;
 

private:
 time_point time_;
;