patterncppMinor
C++11 Blocking Queue learning exercise
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:
the problem is that you would return a reference to local variable which is Undefined Behaviour.
Just
One place where you will have a copy constructor called is here:
To prevent it you can implement move semantics into your Job class. Then replace above with:
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.
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.