CODEWITHSUNDEEP
c++multithreadingboostopencv
Alexey
Alexey

Reputation: 5978

Thread safe implementation of circular buffer

Circular_buffer from boost library is not thread-safe. So I wrapped boost::circular_buffer object in a class as shown below. Mutual exclusion between the threads is achieved (I think) by using conditional variables, a mutex and a lock acquisition/release. Is this implementation thread safe? 

#include <boost/thread/condition.hpp>
#include <boost/thread/mutex.hpp>
#include <boost/thread/thread.hpp>
#include <boost/circular_buffer.hpp>

// Thread safe circular buffer 
template <typename T>
class circ_buffer : private boost::noncopyable
{
public:
    typedef boost::mutex::scoped_lock lock;
    circ_buffer() {}
    circ_buffer(int n) {cb.set_capacity(n);}
    void send (T imdata) {
        lock lk(monitor);
        cb.push_back(imdata);
        buffer_not_empty.notify_one();
    }
    T receive() {
        lock lk(monitor);
        while (cb.empty())
            buffer_not_empty.wait(lk);
        T imdata = cb.front();
        cb.pop_front();
        return imdata;
    }
    void clear() {
        lock lk(monitor);
        cb.clear();
    }
    int size() {
        lock lk(monitor);
        return cb.size();
    }
    void set_capacity(int capacity) {
        lock lk(monitor);
        cb.set_capacity(capacity);
    }
private:
    boost::condition buffer_not_empty;
    boost::mutex monitor;
    boost::circular_buffer<T> cb;
};

Edit This is now a template class, which accepts an object of any type (not just cv::Mat object).

Upvotes: 36

Views: 40911

Answers (6)

Marian K.
Marian K.

Reputation: 359

//This implementation above is broken. You also need condition variable
//boost::condition buffer_not_full; and wait in send on available space in the circular buffer.
enter code here

void send (T imdata) {
   lock lk(monitor);
   while (cb.full())
      buffer_not_full.wait(lk);
   cb.push_back(imdata);
   buffer_not_empty.notify_one();
}

T receive() {
     lock lk(monitor);
     while (cb.empty())
        buffer_not_empty.wait(lk);
     T imdata = cb.front();
     cb.pop_front();
     buffer_not_full.notify_one();
     return imdata;
}

Upvotes: 0

Yacov
Yacov

Reputation: 1070

Very old question :) Here is a dis gin with lock-free implementation Link

Here is a BSD-2 Lib for this Link

Upvotes: 1

Emile Cormier
Emile Cormier

Reputation: 29209

Your implementation is similar to the one shown by this blogger. You should read that blog to see if you missed anything in your implementation.

If your Mat objects are expensive to create/copy, you should avoid continuously creating/copying/deleting them. Instead, you should have a pool (aka free list) of Mat objects that continually get recycled in some kind of pipeline architecture. I describe this type of architecture in this answer to a related question.

In that answer, I suggested using a blocking stack to implement the pool, but you could also use your blocking circular_buffer. The reason I suggested a stack was because I thought it may be more cache-friendly, but I never actually measured to see if it would make a difference.

Upvotes: 2

Willem Hengeveld
Willem Hengeveld

Reputation: 2776

i think it looks fine, except that there is some pointless copies of Mat made in send. You don't need the new, you can directly push the argument of send to your cb.

Upvotes: 5

Daniel Roethlisberger
Daniel Roethlisberger

Reputation: 7058

Looks good at first glance, except that you are not using the buffer_not_full condition at all. You probably want to add code similar to the buffer_not_empty code.

Upvotes: 0

Yochai Timmer
Yochai Timmer

Reputation: 49251

Yes.
If you lock all the public methods with the same lock it will be threadsafe.

You could consider using read-write locks, which may have better performance if you have a lot of concurrent readers.

If you don't have a lot of readers, it will just add overhead, but may be worth checking the option and testing.

Upvotes: 18

Related Questions