1

Need help in implementing a ticker class with start/stop/pause functionality, assignable callback (onTick) with execution on a separate thread every interval span. Interval span is specifiable and update-able. Hopefully it should be cross-platform.

Here is my naive attempt, but it's not good (the while loop in start() is blocking currently, but ideally it should run on a separate thread but I can't figure out how to achieve it) as I'm pretty noob in C++ multithreading model:

#include <cstdint>
#include <functional>
#include <chrono>
#include <thread>
#include <future>

class Ticker {
public:
    typedef std::chrono::duration<int64_t, std::nano> tick_interval_t;
    typedef std::function<void()> on_tick_t;

    Ticker (std::function<void()> onTick, std::chrono::duration<int64_t, std::nano> tickInterval) 
    : _onTick (onTick)
    , _tickInterval (tickInterval)
    , _running (false) {}
    ~Ticker () {}

    void start () {
        if (_running) return;
        _running = true;
        while (_running) {
            std::async( std::launch::async, _onTick );
            std::this_thread::sleep_for( _tickInterval );
        }
    }
    void stop () { _running = false; }

private:
    on_tick_t           _onTick;
    tick_interval_t     _tickInterval;
    bool                _running;
};

I'm a completely wrong in my attempt or it's pretty close?

mikeg
  • 63
  • 3
  • 10
  • Refer to `boost::asio::deadline_timer`: http://www.boost.org/doc/libs/1_55_0/doc/html/boost_asio/reference/deadline_timer.html – ALittleDiff Jul 16 '14 at 11:04
  • Thanks, but I'm not sure if it's what I'm looking for. :) I'm looking for a ticker that will call callback on every tick. Did you looked at my attempt, I'm a completely wrong in my attempt or it's pretty close? – mikeg Jul 16 '14 at 11:16

1 Answers1

2

Consider the following code with example of usage. I hope I understood you correctly. Just run your while loop in a separate thread.

#include <cstdint>
#include <functional>
#include <chrono>
#include <thread>
#include <future>
#include <condition_variable>
#include <iostream>
#include <mutex>

class Ticker {
public:
    typedef std::chrono::duration<int64_t, std::nano> tick_interval_t;
    typedef std::function<void()> on_tick_t;

    Ticker (std::function<void()> onTick, std::chrono::duration<int64_t, std::nano> tickInterval) 
    : _onTick (onTick)
    , _tickInterval (tickInterval)
    , _running (false) {}
    ~Ticker () {}

    void start () {
        if (_running) return;
        _running = true;
        std::thread run(&Ticker::timer_loop, this);
        run.detach();
    }

    void stop () { _running = false; }

    void setDuration(std::chrono::duration<int64_t, std::nano> tickInterval)
    {
        _tickIntervalMutex.lock();
        _tickInterval = tickInterval;
        _tickIntervalMutex.unlock();
    }

private:
    void timer_loop()
    {
        while (_running) {
            std::thread run(_onTick );
            run.detach();

            _tickIntervalMutex.lock();
            std::chrono::duration<int64_t, std::nano> tickInterval = _tickInterval;
            _tickIntervalMutex.unlock();
            std::this_thread::sleep_for( tickInterval );
        }
    }

    on_tick_t           _onTick;
    tick_interval_t     _tickInterval;
    volatile bool       _running;
    std::mutex          _tickIntervalMutex;
};

void tick()
{
    std::cout << "tick\n";
}

void main()
{
    std::chrono::duration<int, std::milli> timer_duration1(1000);
    std::chrono::duration<int, std::milli> timer_duration2(500);
    std::chrono::duration<int> main_wait(5);

    Ticker ticker(std::function<void()>(tick), timer_duration1);
    ticker.start();

    std::this_thread::sleep_for(main_wait);
    ticker.setDuration(timer_duration2);
    std::this_thread::sleep_for(main_wait);
    ticker.stop();
}
Hvarnah
  • 141
  • 6
  • Hvarnah, thank you very much! It's exactly what I've been looking for! If you might, I have a question about your implementation, specifically about std::mutex _tickIntervalMutex. What if I wouldn't use any lock to update _tickInterval, then in the worst case I'll call sleep_for() with expired _tickInterval value few iterations in a loop? If it's the only side-effects I'll get, then I'll consider to remove this mutex as I heard they are pretty CPU heavy and in my situation I can live with this side-effect... What do you think? – mikeg Jul 16 '14 at 16:40
  • @mikeg Well, I don't recommend you to pass `_tickInterval` to function `sleep_for`, because it is passed by reference so I have no idea what is happening inside the function and I can't tell you how terrible result it can produce. Also it is not a good idea to leave a string `... tickInterval = _tickInterval;` without mutex lock and unlock. Imagine that you are trying to change `_tickInterval` just at the moment when the copy constructor of std::chrono::duration is executing. Then a half of the object will be copied from the old value and a half from the new value. Not good. – Hvarnah Jul 16 '14 at 17:40
  • @mikeg I'd recommend you to implement it in a different way. Move mutex locking and value copying outside the while loop. In that case you will invoke mutex operations just once for one launch but not at every timer tick so it will be ok for performance. But in that case you must restart the timer to change it's interval. Notice that `_running` doesn't have a mutex. Just because there is no way to copy a half of the value as in `_tickInterval`; it can be just true or false. – Hvarnah Jul 16 '14 at 17:41
  • Hvarnah, I progressed further and encountered one more issue relative to threading. Can you look [here](http://stackoverflow.com/q/24800204/3616446) please? – mikeg Jul 17 '14 at 09:49
  • Is there any need for the mutable object here? If the duration needs to be changed it would seem more appropriate to cancel and destroy the first timer, then replace it with one differently-configured. – boycy Jul 18 '14 at 08:55
  • Why keep re-creating the thread? Ok so overhead is small here, but it's premature pessimization. I'd be tempted to make the constructor start the thread and the only way to 'restart' a timer to move-assign some other running timer. (This follows the design model of `std::thread` btw.) It's worth noting that `volatile` isn't a thread-safety primitive, although it is safe in this scenario. I'd personally go for an atomic, synchronised value - `std::atomic` instead though (http://stackoverflow.com/questions/2484980/why-is-volatile-not-considered-useful-in-multithreaded-c-or-c-programming). – boycy Jul 18 '14 at 09:30