Update QProgressBar from multiple QThreads

Question

I found several tutorials online explaining how to update a QProgressBar during some long calculation. One of them is: use a QThread to do the calculation, then emit a signal that is connected to progressBar.setValue(int).

I thought this must also work for several QThread's that run at the same time, but something is not working correctly.

So, here is what I do: I want to calculate the trajectories of several particles (each with a long loop). To use multi-core processing, I create a QThread for each of these particles and let it call the respective calculation method. This works fine, all cores are used and the calculation finishes in roughly a quarter of time than before.

I wrote a Worker class based on this tutorial http://mayaposch.wordpress.com/2011/11/01/how-to-really-truly-use-qthreads-the-full-explanation/. The header looks like this: (worker.h)

#include <world.h>

class Worker: public QObject
{
    Q_OBJECT

public:
    explicit Worker(World *world = 0, double deltaTau = 0., double maxDist = 0., double iterations = 0., double index = 0);

public slots:
    void process();

signals:
    void finished();
    void eror(QString err);

private:
    World *w;
    double my_deltaTau;
    double my_maxDist;
    int my_iterations;
    int my_index;
};

And the source like this: (worker.cpp)

#include "worker.h"

Worker::Worker(World *world, double deltaTau, double maxDist, double iterations, double index)
{
    w = world;
    my_deltaTau = deltaTau;
    my_maxDist = maxDist;
    my_iterations = iterations;
    my_index = index;
}

void Worker::process()
{
    w->runParticle(my_deltaTau, my_maxDist, my_iterations, my_index);
    emit finished();
}

Within world.cpp I have a function run that starts all the threading and the function runParticle that is called by the Worker:

void World::run(double deltaTau, double maxDist, int iterations)
{
    globalProgress = 0;
    for (int j = 0; j < particles->size(); j++) { //loop over all particles
        QThread *thread = new QThread;
        Worker *worker = new Worker(this, deltaTau, maxDist, iterations, j);
        worker->moveToThread(thread);
        connect(thread, SIGNAL(started()), worker, SLOT(process()));
        connect(worker, SIGNAL(finished()), thread, SLOT(quit()));
        connect(worker, SIGNAL(finished()), thread, SLOT(deleteLater()));
        connect(thread, SIGNAL(finished()), worker, SLOT(deleteLater()));
        thread->start();
    }
}

void World::runParticle(double deltaTau, double maxDist, int iterations, int index)
{
    for (int i = 0; i < iterations; i++) { //loop over iteration steps
        if (i % 1000 == 0) { //only update the progress bar every 1000th iteration
            emit updateProgress(++globalProgress);
            qApp->processEvents(); // <--- I added this line, no effect!
        }
        [...] // <--- do my calculations for the particle's trajectories
    }
}

The public slot updateProgress(int) is called here every 1000th iteration step. It is connected to the QProgressBar in my MainWindow like this:

progressBar->setValue(0);
progressBar->setMaximum(nrPart * iter / 1000); //number of particles * number of iteration steps / 1000
connect(world, SIGNAL(updateProgress(int)), progressBar, SLOT(setValue(int)));
world->run(timeStep, dist, iter);

My problem is that the progress bar doesn't move until all the calculation is finished and then I see it move to 100% quite quickly.

Does anyone see my mistake or knows how to properly do this?

EDIT

I made the following changes:

(worker.h)

#include "world.h"

class Worker: public QObject
{
    Q_OBJECT

public:
    explicit Worker(World *world = 0, Particle *particle = 0, QList<MagneticField> *bfields = 0, double deltaTau = 0., double maxDist = 0., int iterations = 0);

public slots:
    void process();

signals:
    void finished();
    void updateProgress(int value);
    void ProcessParticle();
    void eror(QString err);

private:
    int i;
    Particle *p;
    QList<MagneticField> *magneticFields;
    double my_deltaTau;
    double my_maxDist;
    int my_iterations;
};

(worker.cpp)

#include "worker.h"

Worker::Worker(World *world, Particle *particle, QList<MagneticField> *bfields, double deltaTau, double maxDist, int iterations)
{
    i = 0;
    const World *w = world;
    p = particle;
    magneticFields = bfields;
    my_deltaTau = deltaTau;
    my_maxDist = maxDist;
    my_iterations = iterations;
    connect(this, SIGNAL(updateProgress(int)), w, SLOT(updateTheProgress(int)));
    connect(this, SIGNAL(ProcessParticle()), this, SLOT(process()), Qt::QueuedConnection);
}

void Worker::process()
{
    const int modNr = my_iterations / 1000;
    QDateTime start = QDateTime::currentDateTime();
    while (i < my_iterations) { //loop over iteration steps
        [...] // <--- do my calculations
        //handle progress
        emit updateProgress(1);
        if (QDateTime::currentDateTime() > start.addMSecs(300)) {
            emit ProcessParticle();
            ++i; //ensure we return to the next iteration
            return;
        }
        i++;
    }
    qDebug() << "FINISHED"; // <--- I can see this, so finished() should be emitted now...
    emit finished();
}

(part of world.h)

public slots:
    void threadFinished();
    void updateTheProgress(int value);

signals:
    void updateProgress(int value);

(part of world.cpp)

void World::threadFinished()
{
    particleCounter++;
    qDebug() << "particles finished: " << particleCounter; // <--- this is NEVER called !?!?
    if (particleCounter == particles->size()) {
        hasRun = true;
    }
}

void World::updateTheProgress(int value)
{
    globalProgress += value;
    emit updateProgress(globalProgress);
}

void World::run(double deltaTau, double maxDist, int iterations)
{
    globalProgress = 0;
    particleCounter = 0;
    hasRun = false;
    for (int i = 0; i < particles->size(); i++) { //loop over all particles
        QThread *thread = new QThread;
        Worker *worker = new Worker(this, &(*particles)[i], bfields, deltaTau, maxDist, iterations);
        worker->moveToThread(thread);
        connect(thread, SIGNAL(started()), worker, SLOT(process()));
        connect(worker, SIGNAL(finished()), thread, SLOT(quit()));
        connect(worker, SIGNAL(finished()), thread, SLOT(deleteLater()));
        connect(worker, SIGNAL(finished()), this, SLOT(threadFinished())); // <--- this connection SHOULD make sure, I count the finished threads
        connect(thread, SIGNAL(finished()), worker, SLOT(deleteLater()));
        thread->start();
    }
}

(somewhere in MainWindow.cpp)

progressBar->setValue(0);
progressBar->setMaximum(nrPart * iter);
connect(world, SIGNAL(updateProgress(int)), progressBar, SLOT(setValue(int)));
world->run(timeStep, dist, iter);
while (!world->hasBeenRunning()) {} //wait for all threads to finish

As I marked in the code above, I never get notification when the threads have finished and I end up in an infinite loop in the MainWindow. Is there still something wrong with the World <-> Worker connections?

Answer 1

The problem is that in order for the emitted signal to be processed, you need to allow the event loop to run on the new thread. By remaining in the for loop in runParticle, that's not happening, until the function has finished.

There is a crude method to fix this, which is to call QApplication::processEvents every once in a while, during your loop.

A better method would be to redesign the object so that a number of iterations are processed, before exiting and allowing the event loop to run naturally.

So, to create a time slice for processing, in your for loop you'd time how long the iterations are taking. If the time has exceeded 1/30th of a second, then call a signal with type QueuedConnection to call your slot function again and exit the for loop.

The QueuedConnection will ensure that any events are processed and then your function is called again.

Assuming runParticle is a slot:-

void Worker::runParticle(...)
{
    static int i = 0;

    QDateTime start = QDateTime::currentDateTime();

    for(i; i < iteration; ++i)
    {
        // do processing


        emit updateProgress(++globalProgress);

        // check if we've been here too long
        if(QDateTime::currentDateTime() > start.addMSecs(300))
        {
          emit ProcessParticle(); // assuming this is connected to runParticle with a Queued Connection
          ++i; // ensure we return to the next iteration
          return;
        }
    }
}

On a different note, when an object is moved to a new thread, all its children are moved too, where a child is part of the QObject hierarchy.

By having the Worker object hold a pointer to the World, it's directly calling the World's runParticle function, which is still on the main thread. While this is unsafe, it also means that the runParticle function is being processed on the main thread.

You need to move the runParticle function to the Worker object, which is on the new thread.

Answer 2

IMO this is wrong. Creating a thread is costly and you want create a lot of them. First of all you should use QThreadPool, your case exactly matches functionality of this class.

There are also QtConcurrent methods which will reduce boiler plate code drastically (this is abandoned feature of Qt so it is recommended to use QThreadPool, but you can try it works quite well).