QProcessDialog with multiple QFutureWatchers

Tranquil

Hello everyone,

I would need assistance for multi-threading a function. Imagine you had a class Data, and its object data. You have names of files in vector of strings data.names, and their content in a map data.content. Now, there is a function doProcessThread, that should be run in each spawned thread, while proper arguments are passed (int, Data, vector). The function modifies the content of the data (each thread modifies one element of the map). I wrote the code and it does the job just fine. However, reporting back to the QProcessDialog is not correct. As you see in the code below, I am generating multiple QFutureWatcher objects (pointers actually), and connecting all of them with a dialog. As you may guess, the dialog closes with the first finished thread, even though others are still running. If I place it after the loop for waitForFinished, it will never go away on its own. Also, if I press cancel on the dialog, it ignores. It is not showing any percentage or incrementing. Could you give me some pointers what would be the best way to address this? Thank you.

void Processer::doProcessMulti(Data& data){

    QProgressDialog dialog;
    dialog.setLabelText(QString("Processing files..."));

    vector<QFutureWatcher<void> *> watchers;

    for (unsigned int i = 0; i < data.names.size(); i++) {

            QFutureWatcher<void>* futureWatcher = new QFutureWatcher<void>();

            QObject::connect(&dialog, SIGNAL(canceled()), futureWatcher, SLOT(cancel()));
            QObject::connect(futureWatcher, SIGNAL(finished()), &dialog, SLOT(reset()));
            QObject::connect(futureWatcher, SIGNAL(progressRangeChanged(int,int)), &dialog, SLOT(setRange(int,int)));
            QObject::connect(futureWatcher, SIGNAL(progressValueChanged(int)), &dialog, SLOT(setValue(int)));
            
            futureWatcher->setFuture(QtConcurrent::run(this, &Processer::doProcessThread, i, std::ref(data), someVector));
            watchers.push_back(futureWatcher);
    }

    dialog.exec();

    for (unsigned int i = 0; i < watchers.size(); i++) {
        watchers[i]->waitForFinished();
    }
}

Chris Kawa

This type of task is better handled by QtConcurrent::map than multiple calls to QtConcurrent::run. With map you get a single future with progress indication. With run you'd need to handle that manually but why re-invent the wheel?
To use map, if possible, convert your Data structure to more processing friendly form i.e. a single container of stuff instead of stuff of two containers.
That would be great but if that's not possible for some reason you can create a lightweight "adapter" struct like this:

struct Params {
    const QString* fileName; //pointing to an element in the data.names container
    WhateverContent* content; //pointing to an element in the data.content container
    Output* output; //pointing to an element in someVector
};

and then adapt your Processer::doProcessThread to take that as a single parameter.

Other comments:

Consider switching to the new connect syntax (a lot less error prone).
Mind your object ownership. You are leaking all the QFutureWatcher instances. Either give them parents or delete them manually. Better yet use map and create only one on the stack.
Don't reset when you mean close. Result is the same in this case but it's better to explicitly state the intent.

So a skeleton code for this (may have bugs, flows out of my head ;) ):

void Processer::doProcessMulti(Data& data){
    using Fut = QFutureWatcher<void>; //just because it's less typing later
    using Pd  = QProgressDialog;

    Pd dialog; //you should probably give it a parent to have proper windowing order, parents are not only for resource mangement
    dialog.setLabelText("Processing files..."); //let automatic conversion kick in, you don't need to spell out QString

    Fut watcher; //just make one on the stack so you don't leak it

    //If you're in a QObject you don't need to spell out QObject::connect every time
    connect(&dialog,  &Pd::canceled,              &watcher, &Fut::cancel);
    connect(&watcher, &Fut::finished,             &dialog,  &Pd::close);
    connect(&watcher, &Fut::progressRangeChanged, &dialog,  &Pd::setRange);
    connect(&watcher, &Fut::progressValueChanged, &dialog,  &Pd::setValue);

    QVector<Params> stuff = adaptFromData(data); //make adaptFromData create the "helper" structs
    watcher.setFuture(QtConcurrent::map(stuff, &Processer::doProcessThread)); //assuming doProcessThread takes &Params

    dialog.exec();
}

Tranquil

Hi Chris,

Thanks a lot for your reply, I really appreciate all your comments and help!
My Data class cannot really be converted into a container of any sort, that is why I kept using QtConcurrent::run (I could feed it with the whole object). Can QtConcurrent::map use QMap for iteration?

I followed your suggestions carefully and created an "adapter" struct, and re-defined doProcessThread function (so that it takes a reference to that struct). I used QVector to store instances of that struct, placed the vector as a first parameter in ::map, but I got an error of not matching for call to ‘(QtConcurrent::MemberFunctionWrapper1<void... I looked it up, and tried with lambda expression, but got another error: undefined reference to `QtConcurrent::ThreadEngineBase::~ThreadEngineBase().

In the end, I decided not to complicate it further, but to think of a better design for my containers and the functions that modify them. I will get back to this post once I try it again. I will need to re-organize all my functions and variables so that I can use multi-threading in such a way.

Chris Kawa

The doProcessThread signature must look something like this to work (map will not work with non-static members):

class Processer{
public:
   static void doProcessThread(Params& params);
};

If it can't be static and you need that instance objects then a lambda adapter like this should work:

//assuming "this" is the Processer instance you want to use, otherwise adjust
watcher.setFuture(QtConcurrent::map(stuff,  [this](Params& params){ doProcessThread(params); }));

Can QtConcurrent::map use QMap for iteration?
It can use any iterator based container. The gotcha with QMap is that, unlike in std::map, its iterator's operator* returns a reference to the value, so there's no way to get to the key this way. So if you used a map like QMap<String, Stuff> you would get Stuff& parameter in the map call with no way to get to the string.
In contrast, the std::map's iterator is a pair of key/value, so with std::map<QString, Stuff> you would get a std::pair<QString, Stuff>& parameter in the map call.