tbb concurrent_bounded_queue multiple threads access

[Xr_L_]

I have the following code:

    tbb::concurrent_bounded_queue<Image> camera_queue_;
    camera_queue_.set_capacity(1);
    
    struct Image
    {
        int hour_;
        int minute_;
        int second_;
        int msec_;
        QImage image_;
    
        Image(){hour_ = -1; minute_ = -1; second_ = -1; msec_ = -1; image_ = QImage();}
        Image& operator=(Image const& copy)
        {
            this->hour_ = copy.hour_;
            this->minute_ = copy.minute_;
            this->second_ = copy.second_;
            this->msec_ = copy.msec_;
            this->image_ = copy.image_;
            return *this;
        }
    };

In a Qt Thread : 

ThreadA:

    tbb::concurrent_bounded_queue<Image> image_queue_;
    image_queue_.set_capacity(1);
    Image cur_image_;
    void Worker::process() {
    
        while(1)
        {
    
            if(quit_)
                break;
    
            {
                camera_queue_.pop(cur_image_);
                image_queue_.push(cur_image_);
            }
    
            emit imageReady();
        }
    
        emit finished();
    }
    
    Image Worker::getCurrentImage()
    {
        Image tmp_image;
        image_queue_.pop(tmp_image);
        return tmp_image;
    }

In Another Thread:

ThreadB:

    Producer::Producer(){
        work_ = new Worker();
        work_->moveToThread(workerThread_);
        QObject::connect(workerThread_, &QThread::finished, work_, &QObject::deleteLater);
        QObject::connect(this, &Producer::operate, work_, &Worker::process);
        QObject::connect(work_, &Worker::imageReady, this, &Producer::displayImage);
        QObject::connect(this, &Producer::stopDecode, work_, &Worker::stop);
        workerThread_->start();
        emit operate();
    }
    
    void Producer::process() {
    
        while(1)
        {
    
            if(quit_)
                break;
    
            {
                camera_queue_.push(GetImage());
            }
    
        }
    
    }
    
    
    void Producer::displayImage()
    {
        Image tmp = std::move(work_->getCurrentImage());
        widget_->showImage(tmp.image_);
    }

However, In main thread, I have a function that enables user to click a button to get current image:

    bool Producer::SaveImage()
    {
    
        Image img = std::move(work_->getCurrentImage());
        std::string fileName = std::to_string(img.hour_) + "-" + std::to_string(img.minute_) + "-" + std::to_string(img.second_) + "-" + std::to_string(img.msec_/1000) + ".jpg";
        std::string outFileName = folder + "/" + fileName;
    
        return img.image_.save(QString::fromStdString(outFileName));
    }

The problem is:

When user does not click the button to invoke Producer::SaveImage(), the Image Decoding and Showing runs smoothly. But when user invoke Producer::SaveImage(), the whole program will get stuck (Caton phenomenon ?). The GUI response becomes not that smooth. The more user invokes SaveImage, the slower the GUI response becomes.

Can anyone help to explain why ? Is there a way to solve that ?

[Alexei_K_Intel]

The current implementation of Producer::SaveImage causes starvation of Producer::displayImage. Every time when a new image is ready, Producer::displayImage is expected to process this image. However, Producer::SaveImage takes the image and Producer::displayImage needs to wait for the next one. So Producer::displayImage cannot process imageReady event and spent a lot of time in image_queue.pop making GUI unresponsible. As a work-around you can try to use try_pop instead of pop in Producer::displayImage to skip excessive imageReady events.

However, it is better to redesign the algorithm to resolve the dependence between Producer::SaveImage and Producer::displayImage. Could you explain the desired behavior of the algorithm that we can try to consider a better approach. Why the queues are limited with 1? It can cause inefficiency because Worker cannot push the next image until the previous image is consumed. 

P.S. std::move for the result of the function call seems unnecessary because the returned value is temporary and can be moved by default. However, the explicit std::move can prevent copy elision optimization and force move semantics. 

[Xr_L_]

Alexei K. (Intel) wrote:

The current implementation of Producer::SaveImage causes starvation of Producer::displayImage. Every time when a new image is ready, Producer::displayImage is expected to process this image. However, Producer::SaveImage takes the image and Producer::displayImage needs to wait for the next one. So Producer::displayImage cannot process imageReady event and spent a lot of time in image_queue.pop making GUI unresponsible. As a work-around you can try to use try_pop instead of pop in Producer::displayImage to skip excessive imageReady events.

However, it is better to redesign the algorithm to resolve the dependence between Producer::SaveImage and Producer::displayImage. Could you explain the desired behavior of the algorithm that we can try to consider a better approach. Why the queues are limited with 1? It can cause inefficiency because Worker cannot push the next image until the previous image is consumed. 

P.S. std::move for the result of the function call seems unnecessary because the returned value is temporary and can be moved by default. However, the explicit std::move can prevent copy elision optimization and force move semantics. 

Hi Alex, 

1. The reason for the capacity limitation is that I want the Producer::displayImage to be real time (as quickly as possible). But with your explanation, I think this is not a good idea to set the capacity to one.

2. "Producer::SaveImage takes the image and Producer::displayImage needs to wait for the next one". I expected that the GUI would get stuck for a short time when user invokes Producer::SaveImage, but what I cannot understand is that why invoking Producer::SaveImage multiple times will make the GUI become slower and slower. The GUI response will not recover. Removing the dependency is a good way but I also want to know the reason for this.

3. Can you give me more explanation why I should use try-pop instead of pop ?

[Alexei_K_Intel · Accepted solution]

Xr L. wrote:

2. "Producer::SaveImage takes the image and Producer::displayImage needs to wait for the next one". I expected that the GUI would get stuck for a short time when user invokes Producer::SaveImage, but what I cannot understand is that why invoking Producer::SaveImage multiple times will make the GUI become slower and slower. The GUI response will not recover. Removing the dependency is a good way but I also want to know the reason for this.

3. Can you give me more explanation why I should use try-pop instead of pop ?

Let us consider the example to understand the situation better:

1. ThreadA (Worker) creates an image and pushes it to the images queue;
2. The user clicks the button and SaveImage takes this image;
3. ThreadA (Worker) emits the ImageReady event;
4. The ImageReady event is processing by ThreadB and it gets stuck inside the pop method of the image queue;
5. ThreadA produces one more image and ThreadB request is satisfied.
6. However, ThreadA again emits the ImageReady event and we again have a starvation (steps 3 and 4).
7. Every time the user clicks the button we have bigger and bigger starvation (the number of unprocessed ImageReady events is increasing)
8. So each new GUI event (e.g. redraw) should wait for all ImageReady events to be processed (because the GUI thread gets stuck in the pop method of the image queue).
9. Therefore, each call of SaveImage increases the number of missed events and reduces the overall GUI responsiveness.

Strictly speaking, you should not use try_pop, it is a work-around to skip the missed events. If the queue is empty - there is no sense to wait for the next image because it will be processed by the next event (but our image is stolen by SaveImage).

[Xr_L_]

Alexei K. (Intel) wrote:

Quote:

Xr L. wrote:

2. "Producer::SaveImage takes the image and Producer::displayImage needs to wait for the next one". I expected that the GUI would get stuck for a short time when user invokes Producer::SaveImage, but what I cannot understand is that why invoking Producer::SaveImage multiple times will make the GUI become slower and slower. The GUI response will not recover. Removing the dependency is a good way but I also want to know the reason for this.

3. Can you give me more explanation why I should use try-pop instead of pop ?

Let us consider the example to understand the situation better:

1. ThreadA (Worker) creates an image and pushes it to the images queue;
2. The user clicks the button and SaveImage takes this image;
3. ThreadA (Worker) emits the ImageReady event;
4. The ImageReady event is processing by ThreadB and it gets stuck inside the pop method of the image queue;
5. ThreadA produces one more image and ThreadB request is satisfied.
6. However, ThreadA again emits the ImageReady event and we again have a starvation (steps 3 and 4).
7. Every time the user clicks the button we have bigger and bigger starvation (the number of unprocessed ImageReady events is increasing)
8. So each new GUI event (e.g. redraw) should wait for all ImageReady events to be processed (because the GUI thread gets stuck in the pop method of the image queue).
9. Therefore, each call of SaveImage increases the number of missed events and reduces the overall GUI responsiveness.

Strictly speaking, you should not use try_pop, it is a work-around to skip the missed events. If the queue is empty - there is no sense to wait for the next image because it will be processed by the next event (but our image is stolen by SaveImage).

Thanks for the reply. And one more question. Is it better to set those two queues's capacity to 2 to accelerate speed  ?

[Alexei_K_Intel]

It is a question of a memory consumption and how rendering and calculating threads are unbalanced. It will not improve the performance but it will improve the robustness if there is something unstable. So I suppose 2-4 images calculated in advance should be enough.