A periodic implementation for C++ using Grand Central Dispatch

Question

I have a C++ project which needs a few periodic timers. I have created a timer class for this project using Grand Central Dispatch.

I tried using boost::asio, but for some company policy reasons, I cannot link with said library.

Here's the header file with the interface:

//
//  PeriodicTimer.hpp
//  

#pragma once

#include <thread>
#include <mutex>
#include <memory>

#ifdef __OBJC__
#import <dispatch/dispatch.h>
#else
using dispatch_queue_t  = void *;
using dispatch_source_t = void *;
#endif

namespace Test
{
#pragma mark -
#pragma mark TimerWorkloop
    
    /**
     @class         TimerWorkloop
     @brief         a workloop to host all the timer events
     */
    class TimerWorkloop
    {
    public:
        using Context = dispatch_queue_t;
        
    private:
        Context             mContext;
        
    public:
        /**
         @function          ctor
         */
        TimerWorkloop();
        
        /**
         @function          dtor
         */
        ~TimerWorkloop();
        
        /**
         @function          getContext
         @abstract          the loop used by the thread for the io context
         */
        Context             getContext()
        { return mContext; }
    };

#pragma mark -
#pragma mark PeriodicTimer

    /**
     @class         PeriodicTimer
     @brief         a timer that gives a periodic asynchronous callback
     
     @note          please note that the callback issued by this class is on the TimerWorkloop thread, and if there is significant
     work to be done in the callback, then the client should offload the work onto a separate thread
     */
    class PeriodicTimer
    {
    public:
        using CallbackFunc  = std::function<void(PeriodicTimer *)>;

    private:
        using Timer         = dispatch_source_t;
        
        bool                            mIsActive;
        uint32_t                        mPeriodMs;
        CallbackFunc                    mCb;
        std::shared_ptr<TimerWorkloop>  mWl;
        Timer                           mTimer;
        std::mutex                      mLock;

    public:
        static constexpr uint32_t       kMinPeriodMs = 5;
        
        /**
         @function          ctor
         */
        PeriodicTimer(std::shared_ptr<TimerWorkloop> inWl);
        
        /**
         @function          dtor
         */
        ~PeriodicTimer();
        
        /**
         @function          start
         
         @param             inPeriod        the period for the timer
         @param             inCb                 the callback function that will be called
         */
        void                start(uint32_t inperiodMs, CallbackFunc inCb);
        
        /**
         @function          stop
         */
        void                stop();

        /**
         @function          getperiodMs
         
         @return            current period in milliseconds
         */
        uint32_t            getPeriodMs() const { return mPeriodMs; }
        
        /**
         @function          isRunning
         
         @return            bool indicating whether the timer is running
         */
        bool                isRunning() const { return mIsActive; }
        
    private:
        /**
         @function          _timerHandler
         
         @param             inErrorCode     the error code used to indicate whether the timer has expired or cancelled
         */
        void                _timerHandler();
        
        PeriodicTimer(PeriodicTimer const &) = delete;
        PeriodicTimer(PeriodicTimer &&) = delete;
    };
}

And the implementation:

//
//  PeriodicTimer.mm
//  

#include "PeriodicTimer.hpp"
#include <stdexcept>

using namespace Test;

#pragma mark -
#pragma mark TimerWorkloop

TimerWorkloop::TimerWorkloop()
{
    mContext = dispatch_queue_create("com.test.timerworkloop",
                                     DISPATCH_QUEUE_SERIAL);
}

TimerWorkloop::~TimerWorkloop()
{
    
}


#pragma mark -
#pragma mark PeriodicTimer

PeriodicTimer::PeriodicTimer(std::shared_ptr<TimerWorkloop> inWl) :
mWl(inWl), mIsActive(false), mPeriodMs(0), mCb(nullptr)
{
    
}

PeriodicTimer::~PeriodicTimer()
{
    if (mIsActive)
    {
        stop();
    }
}

void
PeriodicTimer::start(uint32_t inPeriodMs, CallbackFunc inCb)
{
    if (!mIsActive)
    {
        if (inPeriodMs < kMinPeriodMs)
        {
            throw std::runtime_error("Too short a period for the timer");
        }
        
        if (inCb == nullptr)
        {
            throw std::runtime_error("Empty timer callback");
        }
        
        mPeriodMs = inPeriodMs;
        mCb = inCb;
        
        mIsActive = true;
        
        mTimer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, mWl->getContext());
        ThrowIfFalse(mTimer != nil, "Could not allocate a timer dispatch source");
        
        dispatch_source_set_timer(mTimer, DISPATCH_TIME_NOW, inPeriodMs * NSEC_PER_MSEC,
                                  NSEC_PER_MSEC);
        dispatch_source_set_event_handler(mTimer, ^{ this->_timerHandler(); });
        dispatch_resume(mTimer);
    }
}

void
PeriodicTimer::stop()
{
    if (mIsActive)
    {
        dispatch_source_cancel(mTimer);
        mIsActive = false;
    }
}

void
PeriodicTimer::_timerHandler()
{
    if (mCb != nullptr)
    {
        mCb(this);
    }
}

I have not used GCD before, and I am relatively new to Objective C as well. Please review my code.

EDIT: I did some performance measurements, and I am seeing some weird behavior:

The first time the timer fires, it fires too quickly, within <1ms of enabling, even if the period is quite high. In addition to this, even at some other times, the timer fires a bit sooner than expected (up to 5 ms sooner).

Why might this be?

Answer 1

TimerWorkLoop is useless

The class TimerWorkLoop does almost nothing. It initializes mContext and then you can get that variable. A lot of lines of codes have been written for this, but why not simply pass a Context to PeriodicTimer? So instead of:

auto workLoop = std::make_shared<TimerWorkLoop>();
PeriodicTimer timer(workLoop);

You could then write:

auto context = dispatch_queue_create("com.test.timerworkloop", DISPATCH_QUEUE_SERIAL);
PeriodicTimer timer(context);

If you don't have ARC enabled, I could see some value for this class if it also properly cleaned up after itself, by calling dispatch_release() in the destructor.

If you want to hide the work loop details so you can easily swap it out for some other implementation, then I would expect all functions that currently rely on knowing the type of context to be moved into TimerWorkLoop. That means most of PeriodicTimer::start() should become one or more member functions of TimerWorkLoop.

mLock is not used

You are not using mLock anywhere in your code, so you can remove it. I also see that you #include <thread> and <mutex>, those are not necessary either.

Consider starting a timer in its constructor

When you create a PeriodicTimer object it's not doing anything yet, you have to call start() first. It is quite likely that you'd want to start a timer as soon as you have created it, so I would add an overload for the constructor that takes a period and callback function and have it start the timer immediately. Note that you can have multiple constructors for a given class in C++, so you can also keep the original constructor to allow creating a timer that does not immediately start.

Use std::chrono::duration to store periods

I can see in your code that the period is supposed to be in milliseconds, but the Doxygen documentation for start() doesn't even mention that. But you can avoid any potential confusion by passing and storing the period as an appropriate std::chrono::duration, like so:

class PeriodicTimer
{
public:
    using Duration = std::chrono::nanoseconds;
    ...
private:
    Duration mPeriod;
    ...
public:
    static constexpr Duration kMinPeriod = std::chrono::milliseconds(5);
    ...
    void start(Duration inPeriod, CallbackFunc inCb);
    Duration getPeriod() const { return mPeriod; }
    ...
};

void PeriodicTimer::start(Duration inPeriod, CallbackFunc inCb)
{
    if (inPeriod < kMinPeriod)
    {
        throw std::runtime_error("Too short a period for the timer");
    }
    ...
    mPeriod = inPeriod;
    ...
    dispatch_source_set_timer(..., mPeriod.count(), ...);
    ...
};

The std::chrono::duration types can implicitly convert to another, so the code creating a timer can pass in milliseconds, seconds or whatever they want without having to care about what resolution PeriodicTimer uses, like so:

PeriodicTimer timer(...);
timer.start(std::chrono::minutes(1), [](PeriodicTimer *t){...});

Issues with the HeaderDoc documentation

It's great that you are using HeaderDoc to document your code. However, there are some issues I am seeing:

• @class and @function are rarely needed anymore; HeaderDoc will know comments belong to a given class or (member) function from the surrounding source code. It's easy to make a typo in @class and @function, and then the documentation would not match the source code anymore.
• I recommend you use @brief instead of @abstract, the latter is not supported by Doxygen (and it might be nice to have Doxygen also be able to parse your documentation).
• @param names not matching the actual parameter names, like @param inPeriod for start(uitn32_t inperiodMs, ...).
• @param for non-existing parameters, like @param inErrorCode.

It might also be a good idea to run headerdoc2html with the --paranoid option on your source code, and then fix all the problems it finds.

Unnecessary code

In the destructor of PeriodicTimer() you can call stop() unconditionally, as the latter will already check mIsActive.

In _timerHandler() you can also call mCb(this) unconditionally, as you already forbid starting a timer without a callback function.

Your destructors are not doing anything, so you can remove them completely, unless you want to provide a stable API and want to have the option open to add a destructor later.

First callback and timer granularity

The first time the timer fires, it fires too quickly, within <1ms of enabling, even if the period is quite high.

This is because the second argument to dispatch_source_set_timer() is the time when the timer should first fire. Since you set it to DISPATCH_TIME_NOW, it fires immediately. Either explicitly set the start time to now plus the period using dispatch_time(), or you could write some code to somehow ignore the first time the timer fires.

In addition to this, even at some other times, the timer fires a bit sooner than expected (up to 5 ms sooner).

Timers are not perfect and can fire a bit sooner or later depending on the granularity of the timer interrupt used by GCD. You just have to accept this, or use a different kind of timer. See Apple's documentation about high precision timers for more information.