HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

C++11 Blocking Queue learning exercise

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
learningqueueexerciseblocking

Problem

As part of my own C++11 learning exercise I have implemented this Blocking Queue using the new C++11 thread API. One of the aspects I would like to improve is efficiency i.e. in the take member function while returning the Job template type to be returned by reference instead of by value.

/*
 * blockingqueue.h
 *
 *  Created on: May 20, 2016
 *      Author: bravegag
 */

#ifndef BLOCKINGQUEUE_H_
#define BLOCKINGQUEUE_H_

#include 
#include 
#include 
#include 
#include 
#include 
#include 

using namespace std;

template
class blocking_queue {
private:
    queue queue_;
    mutex mutex_;
    condition_variable not_empty_cond_;
    condition_variable not_full_cond_;
    int capacity_ = 0;

public:
    blocking_queue(int capacity) : capacity_(capacity) {
        // empty
    }

    void put(const Job& job) {
        unique_lock lock(mutex_);
        while (queue_.size() >= capacity_) {
            cout  lock(mutex_);
        while (queue_.empty()) {
            cout << "queue empty, waiting for jobs to be put" << endl;
            not_empty_cond_.wait(lock, [&]() { return !queue_.empty(); });
        }
        Job job = queue_.front();
        queue_.pop();
        not_full_cond_.notify_one();
        assert(queue_.size() < capacity_);
        return job;
    }

    int size() const {
        return queue_.size();
    }

    virtual ~blocking_queue() {
        // empty
    }
};

#endif /* BLOCKINGQUEUE_H_ */

Solution

One of the aspects I would like to improve is efficiency i.e. in the take member function while returning the Job template type to be returned by reference instead of by value.

So what you would like is:

Job& take()   ?


the problem is that you would return a reference to local variable which is Undefined Behaviour.

Just Job take() should be fine, compiler will use NRVO optimization so that no copy will be done, copy constructor call would be elided. You could return by moving: return std::move(job);, but that can prevent elision.

One place where you will have a copy constructor called is here:

Job job = queue_.front();


To prevent it you can implement move semantics into your Job class. Then replace above with:

Job job = std::move(queue_.front());


if you will not add any of the user defined constructor/destructor/assignment operator to your Job class, then compiler will add move constructor/assignment operator on its own - depending whether it is possible.

Code Snippets

Job& take()   ?
Job job = queue_.front();
Job job = std::move(queue_.front());

Context

StackExchange Code Review Q#128832, answer score: 4

Revisions (0)

No revisions yet.