3
\$\begingroup\$

I have two threads, one is the producer and other is consumer. My consumer is always late (due to some costly function call, simulated in below code using sleeps) so I have used ring buffer as I can afford to loose some events.

I am looking to see if my locking looks alright and general c++ review comments.

#include <iostream>
#include <thread>
#include <chrono>
#include <vector>
#include <atomic>
#include <boost/circular_buffer.hpp>
#include <condition_variable>
#include <functional>

std::atomic<bool> mRunning;
std::mutex m_mutex;
std::condition_variable m_condVar;

class VecBuf {
    private:
    std::vector<int8_t> vec;

    public:
    VecBuf() = default;
    VecBuf(std::vector<int8_t> v)
    {
        vec = v;
    }
};

std::vector<int8_t> data{ 10, 20, 30 };

class Detacher {
    public:
    template<typename Function, typename ... Args>
    void createTask(Function &&func, Args&& ... args) {
        m_threads.emplace_back(std::forward<Function>(func), std::forward<Args>(args)...);
    }

    Detacher() = default;
    Detacher(const Detacher&) = delete;
    Detacher & operator=(const Detacher&) = delete;
    Detacher(Detacher&&) = default;
    Detacher& operator=(Detacher&&) = default;

    ~Detacher() {
        for (auto& thread : m_threads) {
            thread.join();
        }
    }

    private:
    std::vector<std::thread> m_threads;
};

void foo_1(boost::circular_buffer<VecBuf> *cb)
{
    while (mRunning) {
        std::unique_lock<std::mutex> mlock(m_mutex);

        m_condVar.wait(mlock, [=]() { return !cb->empty(); });

        VecBuf local_data(cb->front());
        cb->pop_front();
        mlock.unlock();
        if (!mRunning) {
            break;
        }
        //simulate time consuming function call and consume local_data here
        std::this_thread::sleep_for(std::chrono::milliseconds(16));
    }

    while (cb->size()) {
        VecBuf local_data(cb->front());
        cb->pop_front();
        if (!mRunning) {
            break;
        }
    }
}

void foo_2(boost::circular_buffer<VecBuf> *cb)
{
    while (mRunning) {
        std::unique_lock<std::mutex> mlock(m_mutex);

        while (cb->full()) {
            mlock.unlock();
            /* can we do better than this? */
            std::this_thread::sleep_for(std::chrono::milliseconds(100));
            mlock.lock();
        }
        cb->push_back(VecBuf(data));
        m_condVar.notify_one();
    }
}

int main()
{
    mRunning = true;
    boost::circular_buffer<VecBuf> cb(100);
    Detacher thread_1;
    thread_1.createTask(foo_1, &cb);
    Detacher thread_2;
    thread_2.createTask(foo_2, &cb);
    std::this_thread::sleep_for(std::chrono::milliseconds(20000));
    mRunning = false;
}
\$\endgroup\$
1
\$\begingroup\$
       /* can we do better than this? */

In a circular buffer context, the standard way to avoid busy waiting is to have two semaphores. First to block a producer when a buffer is full, and second to block a consumer when the buffer is empty. Once a process passes its semaphore, and does its job, it should signal the peer.


The circular buffer is good when the consumer is only sometimes late and you cannot afford loosing data. In your situation it looks like a wrong solution: the producer becomes throttled by the rate of consumption, and the consumer is a presented with the stale data.

A typical answer is to let the producer run at full speed, and triple-buffer the production (at least, it guarantees that the consumer would get the most recently produced data). Please forgive the shameless self promotion.

\$\endgroup\$
  • \$\begingroup\$ thanks for the review comment and the link. However, in my case my consumer only blocks some time. I think it still makes sense to use semaphores instead of my current solution using condition variable + lock. \$\endgroup\$ – noman pouigt Jul 30 at 5:46
  • \$\begingroup\$ @nomanpouigt "some time" is quite different from "always late". \$\endgroup\$ – vnp Jul 30 at 5:47
  • \$\begingroup\$ sorry, I mean the ring buffer size is chosen such a way that it is able to acomodate the delay. Wondering in c++ there are no semaphores and it is implemented using mutexes and condition variable so if it makes sense to use semaphores. \$\endgroup\$ – noman pouigt Jul 30 at 5:53
  • \$\begingroup\$ Can you also please elaborate your solution of tripple buffer and how would it apply in my case? Do you mean don't do any locking in producer side and let it run and producer buffer needs to be tripple-buffered so that consumer can get recent data in case of overruns? \$\endgroup\$ – noman pouigt Jul 30 at 6:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.