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
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 overhead\n";
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 time\n";
}
}
(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 overhead\n";
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 time\n";
}
}
Wandbox demo.
Stopwatch
. Feel free to browse and borrow any ideas you like. \$\endgroup\$