c++ 11 std::chrono Measure time Elapsed

Question

I'm working on a Timer class that calls a function once every time interval. I have noticed that the clock is running slightly slow because the function is not taking into account the amount of thine the code took to operate when setting up the wait amount for the clock. I have been having trouble figuring out how to measure the amount of time that has elapsed during the function call and then subtract that from the interval time in order to produce an accurate wait time.

#include <iostream>
#include <chrono>
#include <thread>
#include <functional>

namespace Engine {
    template<class return_type,class...arguments>
    class Timer{
        typedef std::function<return_type(arguments...)> _function_t;
        typedef std::chrono::system_clock::time_point time_point;
        typedef std::chrono::duration<size_t,std::micro> _duration;
    public:
        Timer(size_t interval,bool autoRun,_function_t function,arguments...args){
            _function = function;
            _interval = interval;
            if (autoRun) {
                Enable(args...);
            }
        }
        ~Timer(){
            if (Running()) {
                Disable();
            }
        }
        void Enable(arguments...args){
            if (!Running()) {
                _running=true;
                enable(_interval, _function, args...);
            }
        }
        void Disable(){
            if (Running()) {
                _running=false;

                delete _thread;
            }
        }
        volatile bool const& Running()const{
            return _running;
        }
    protected:
        void enable(size_t interval,_function_t func,arguments...args){
            _thread = new std::thread([&,func,interval,args...](){

                while (_running) {
                    //measure time starting here
                    func(args...);
                    //end measurement here
                    //calculate interval- time elapsed
                    //use that value in the line below in place of "interval" 
                    std::this_thread::sleep_for(std::chrono::microseconds(interval));

                }
            });
            _thread->detach();
        }
    protected:
        _function_t _function;
        volatile bool _running;
        size_t _interval;
        std::thread* _thread;

    };

}

If anyone has a suggestion on how to do this using the std::chrono library please let me know. Please not boost though. I don't want to have do deal with it at the moment.

Thanks In Advance.

EDIT:

Here is the updated code:

#include <iostream>
#include <chrono>
#include <thread>
#include <functional>
#include <atomic>

namespace Engine {
    template<class return_type,class...arguments>
    class Timer{
        typedef std::function<return_type(arguments...)> _function_t;
        typedef std::chrono::system_clock::time_point time_point;
        typedef std::chrono::duration<size_t,std::micro> _duration;
    public:
        Timer(size_t interval,bool autoRun,_function_t function,arguments...args){
            _function = function;
            _interval = interval;
            if (autoRun) {
                Enable(args...);
            }
        }
        ~Timer(){
            if (Running()) {
                Disable();
            }
        }
        void Enable(arguments...args){
            if (!Running()) {
                _running=true;
                enable(_interval, _function, args...);
            }
        }
        void Disable(){
            if (Running()) {
                _running=false;
            }
        }
        std::atomic_bool const& Running()const{
            return _running;
        }
    protected:
        void enable(size_t interval,_function_t func,arguments...args){
            _thread =std::thread([&,func,interval,args...](){
                std::chrono::duration<long long,std::nano> inter(interval);
                auto _interval = std::chrono::microseconds(interval);
                auto deadline = std::chrono::steady_clock::now();
                while (_running) {
                    func(args...);
                    std::this_thread::sleep_until(deadline+=_interval);
                }
            });
            _thread.detach();
        }
    protected:
        _function_t _function;
        std::atomic_bool _running;
        size_t _interval;
        std::thread _thread;

    };

}

Thanks For the help.

Answer 1

            while (_running) {
                //measure time starting here
                func(args...);
                //end measurement here
                //calculate interval- time elapsed
                //use that value in the line below in place of "interval" 
                std::this_thread::sleep_for(std::chrono::microseconds(interval));

            }

Your comments are exactly correct. You can find documentation on std::chrono here: http://en.cppreference.com/w/cpp/chrono

            while (_running) {
                auto start = std::chrono::high_resolution_clock::now(); //measure time starting here
                func(args...);
                auto end = std::chrono::high_resolution_clock::now(); //end measurement here
                auto elapsed = end - start; //calculate interval- time elapsed
                //use that value in the line below in place of "interval"
                if (elapsed < interval)
                  std::this_thread::sleep_for(interval-elapsed);

            }

The above assumes you change interval to be a std::chrono::duration type. You really should avoid using generic integral types, because you don't get any type safety from them in terms of whether a tick represents a microsecond, a millisecond, or whatever. Users have to check the documentation and that doesn't work very well. Also if you template functions based on the duration then users can pass whatever duration type they like and you can handle any necessary conversion behind the scenes.

---

Some other comments.

The way you're using variadic templates does not enable perfect forwarding. You may get some extra copies of the parameters besides the one that's needed to ensure the arguments live long enough.

volatile does not enable atomic accesses. Your write to _running is not sequenced with the reads and therefore causes a data race, leading to undefined behavior. The simplest fix is std::atomic<bool>, but there are some other possibilities as well.

The bool autoRun parameter leads to the so called "boolean trap". Instead use an enum that will be more readable.

You don't need _thread to be a pointer. In fact since you immediately detach it and never use it for anything except delete, you don't need this member at all. But IMO you'd be better off using std::future and std::async instead of a detached thread.

There's no point in Enable() and Disable() using the public Running() function since they already have to know about the implementation of Running(). It's safer to just access _running directly. Another alternative would be to introduce a counterpart to Running() that is responsible for setting _running, and then Enable() and Disable() would not have to access _running directly at all.

The detached thread could continue running for a time after the timer object has been destroyed, causing it to access members of the Timer after they are no longer valid. If the thread is accessing member variables (such as _running) then you must wait for the thread to complete before destruction completes.

Disable() already checks if the task is running, so the extra check in the destructor is unnecessary.

The order of arguments in the constructor can be changed so that passing an interval and a function with no autorun or arguments defaults to not auto running and not using ...args. E.g. auto draw_loop = Timer(microseconds(10), DrawFunc); draw_loop.Enable(foo, bar);

It's a good idea to avoid default captures in lambdas because then you may not be sure what's getting captured. For example in your code use use reference capture by default, but then capture all the local variables by value. And since _runnable is a member variable it does not get captured by reference. Instead the lambda captures this by value and accesses _runnable through that.

You can use the _function and _interval member variables in your loop instead of capturing new copies.

Instead of using std::function and templating Timer on return_type and arguments you can simply template Timer on a generic Function type. That way you don't pay the expense of std::function and don't have unnecessary return_type and argument types that you don't use anywhere.

template<typename Function>
class Timer {
    using duration = std::chrono::nanosecond;

public:
    enum class AutoRun { no, yes };

    template<typename Duration, typename... Arguments>
    Timer(Duration interval, Function function, AutoRun run = AutoRun::no, Arguments &&...args)
      : _function(function)
      , _interval(std::chrono::duration_cast<duration>(interval))
      , _running(false)
    {
        if (AutoRun::yes == run) {
            Enable(std::forward<Arguments>(args)...);
        }
    }

    ~Timer(){
        Disable();
    }

    template<typename... Arguments>
    void Enable(Arguments &&...args){
        if (!_running) {
            _running=true;
            enable(std::forward<Arguments>(args)...);
        }
    }

    void Disable() {
        if (_running) {
            _running = false;
            _thread.get();
        }
    }

    volatile bool const& Running() const {
        return _running;
    }

protected:
    template<typename... Arguments>
    void enable(Arguments &&...args) {
        _thread = std::async([this] (Arguments &&...args_copy) {
            auto time_to_wake = std::chrono::steady_clock::now();
            while (_running) {
                _function(args_copy...);
                time_to_wake += _interval;
                std::this_thread::sleep_until(time_to_wake);
            }
        }, std::forward<Arguments>(args)...);
    }

protected:
    Function _function;
    duration _interval;

    std::atomic<bool> _running;
    std::future<void> _thread;
};

Answer 2

Use std::this_thread::sleep_until to keep the intervals between events as uniform as possible:

void enable(size_t interval,_function_t func,arguments...args){
    _thread = new std::thread([&,func,interval,args...]{
        auto interval = std::chrono::microseconds{this->interval};
        auto deadline = std::chrono::steady_clock::now();

        while (_running) {
            func(args...);
            deadline += interval;
            std::this_thread::sleep_until(deadline);
        }
    });
    _thread->detach();
}