I have written a little class that is intended to be used to spend unused time in a loop body, sleeping.
That is, I have a loop. I want every iteration to take at least X seconds. If the operations actually complete quicker, the excess time shall be spent sleeping.
My implementation is based on a class that is created in a scope, and whose destructor sleeps for the required amount of time when the object's lifetime ends.
Is this a sane approach to the problem? Are there any flaws in the implementation? Is there potential for improvements regarding coding style?
#include <chrono>
#include <thread>
class TimeSink {
public:
typedef std::chrono::milliseconds duration_ms;
TimeSink(duration_ms const & min_duration)
: min_duration(min_duration), start(std::chrono::steady_clock::now()) {}
~TimeSink() {
time_point end = std::chrono::steady_clock::now();
auto elapsed = end-start;
auto remaining_time_to_sleep = min_duration - elapsed;
if (remaining_time_to_sleep > epsilon) {
std::this_thread::sleep_for(remaining_time_to_sleep);
}
}
private:
typedef std::chrono::time_point<std::chrono::steady_clock> time_point;
const duration_ms epsilon = duration_ms(0);
duration_ms min_duration;
time_point start;
};
int main() {
TimeSink t(TimeSink::duration_ms(1000));
}
1 Answer 1
Allow users to customize your TimeSink class
We achieve this by making it a template class:
template <typename TimeUnit, typename Clock>
class time_sink;
This will allow your class to be used in different scenarios; it removes restrictions.
Since the TimeUnit
template parameter is the one most likely to be changed, we can provide a default argument for Clock
:
template <typename TimeUnit, typename Clock = std::chrono::steady_clock>
class time_sink;
Your destructor can be simplified
Instead of doing...
time_point end = std::chrono::steady_clock::now();
auto elapsed = end-start;
auto remaining_time_to_sleep = min_duration - elapsed;
if (remaining_time_to_sleep > epsilon)
{
std::this_thread::sleep_for(remaining_time_to_sleep);
}
...we can do:
auto time_spent = Clock::now() - begin;
if ( time_spent < min_duration )
{
std::this_thread::sleep_for( min_duration - time_spent );
}
I find this clearer and easier to understand.
Consider using std::chrono::high_resolution_clock
as your default clock type
This stack overflow question should tell you why that's the preferred clock type for timing function execution.
template <typename TimeUnit, typename Clock = std::chrono::high_resolution_clock>
class time_sink;
Concise usage with the std::chrono_literals
namespace and a template function
This would simply make the code shorter. Note that there is a repetition, as we must define time_sink
's time unit type.
using namespace std::chrono_literals;
using time_sink = time_sink<std::chrono::milliseconds>;
time_sink{ 500ms };
In order to make usage fully concise (avoid repetition), we can use a template function and implement the idea proposed by 5gon12eder, in this comment which simply creates the time_sink
object and uses template function type deduction to fill in the details.
template <typename TimeUnit, typename Clock = std::chrono::high_resolution_clock>
time_sink<TimeUnit, Clock> make_time_sink( TimeUnit const minimum_duration )
{
return time_sink<TimeUnit, Clock>{ minimum_duration };
}
This allows us to create a time_sink
in a short and clear way:
using namespace std::chrono_literals;
make_time_sink( 500ms );
// rest of function code, etc.
The improved code
Here's what the final code could look like:
#include <chrono>
#include <thread>
template <typename TimeUnit, typename Clock = std::chrono::high_resolution_clock>
class time_sink
{
public:
time_sink( TimeUnit const minimum_duration ) :
min_duration{ minimum_duration },
begin{ Clock::now() }
{}
~time_sink()
{
auto time_spent = Clock::now() - begin;
if ( time_spent < min_duration )
{
std::this_thread::sleep_for( min_duration - time_spent );
}
}
private:
TimeUnit min_duration;
std::chrono::time_point<Clock> begin;
};
template <typename TimeUnit, typename Clock = std::chrono::high_resolution_clock>
time_sink<TimeUnit, Clock> make_time_sink( TimeUnit const minimum_duration )
{
return time_sink<TimeUnit, Clock>{ minimum_duration };
}
Sample usage
As 5gon12eder points out, you must also ensure that you keep the return of make_time_sink
in order to prevent time_sink
's destructor from running early, since destructors are called at the "end of the full expression, for nameless temporaries" (source).
void f()
{
using namespace std::chrono_literals;
// we keep the return of make_time_sink in order to prevent
// time_sink's destructor from executing before the end of the function
auto ts = make_time_sink( 500ms );
// ... do work ...
}
int main()
{
f();
}
-
\$\begingroup\$ Is it intentional that the "obvious choice" for the clock type is once
std::chrono::steady_clock
and oncestd::chrono::high_resolution_clock
? As for the function, is it meant to be a convenience wrapper around the constructor (then why the lambda?) or would it block for up tp the specified amount of time after calling the lambda (then why the name?)? \$\endgroup\$5gon12eder– 5gon12eder2015年12月16日 21:55:24 +00:00Commented Dec 16, 2015 at 21:55 -
\$\begingroup\$ 1. I changed it without really thinking about it, I'll add a note about it. Thanks for pointing this out. 2. I did leave this part kind of vague. I didn't see it as a wrapper, but now that you point it out, that's even better. Great suggestion. \$\endgroup\$user2296177– user22961772015年12月16日 22:46:47 +00:00Commented Dec 16, 2015 at 22:46
-
\$\begingroup\$ I like the updated answer except for one important detail: You should assign the result of
make_time_sink
to a variable, otherwise the destructor will block right away when the temporary is destroyed. It doesn't matter for themain
, however, because there is no other statement in its body anyway, but you should change it in the shorter snippet above. \$\endgroup\$5gon12eder– 5gon12eder2015年12月17日 01:49:29 +00:00Commented Dec 17, 2015 at 1:49 -
\$\begingroup\$ @5gon12eder Thanks for pointing out the oversight. I'll include a better example. \$\endgroup\$user2296177– user22961772015年12月17日 01:57:36 +00:00Commented Dec 17, 2015 at 1:57
-
\$\begingroup\$ You seem to use the braced initializer list everywhere. Is this the new way to initialize things? When would we resort to the "old" style? \$\endgroup\$moooeeeep– moooeeeep2015年12月17日 08:14:40 +00:00Commented Dec 17, 2015 at 8:14