Continuation not working with QtConcurrent::mapped

Asperamanca

I'm testing QtConcurrent with continuation ('then'). The following sequence does what I expect: It gives me a future with 5 results:

    QList<double> inputs{2.0, 3.0, 5.0, 10.0, 12.0};
    auto future = QtConcurrent::mapped(inputs, calculateArea);
    future.waitForFinished();

However, when I use a continuation to add a second stage to the pipeline, I only get the first result:

    QList<double> inputs{2.0, 3.0, 5.0, 10.0, 12.0};
    auto future = QtConcurrent::mapped(inputs, calculateArea)
                                      .then(formatArea);
    future.waitForFinished();

Any ideas? I would have expected either a compiler error, a runtime error, or the correct number of results.

JonB

@Asperamanca
Hi. I know little about QtConcurrent/QFuture, but thought I'd have a play.

I concur with your finding and puzzlement. If you put qDebug() into calculateArea() it is called 5 times, but a qDebug() in formatArea() is only called once. That is where the "problem" lies.

Even if I separate out the mapped() and then() stages with their own completed QFutures I get the same.

There are no examples out there of QFuture::then() used against mapped(), only against QFuture::run(). I don't know if that is significant.

Doubtless it will take an expert to explain what is going on, whether this is intended or a "bug", whether there is some way you can alter your code to make it do what you intend.

J.Hilk

The .then is applied on the returned QFuture not on each element individually.

I assume your formatArea accepts a double and not a QList<double> ?

Probably due to some template magic the first element of the list is passed on instead of a compiler error.

change the argument/signature of your formatArea and you should be fine

JonB

@J-Hilk
I believe I have done as you suggest. My then() function is now calculateArea3 which I have defined as:

QList<double> calculateArea3(QList<double> dlist)
{
    qDebug() << __FUNCTION__;
    for (double &d : dlist)
        d = d * -100.0;
    return dlist;
}

[Instead of previous double calculateArea2(double d).]
Then my call

    auto future = QtConcurrent::mapped(inputs, calculateArea)
                      .then(calculateArea3);

ends up with a complicated compile-time set of error messages on the .then(calculateArea3).

Is that what you meant for us to do? Have I done it right? Do you want to try and see the error messages?

JonB

@J-Hilk
Further (related?) question. When I break statement to just the mapped():

    /*auto*/ QFuture<double> future1 = QtConcurrent::mapped(inputs, calculateArea);

As you can see (and checked in the debugger) the type of that is QFuture<double>, and results() holds the list. I find this "confusing", I "kind of" expected the type to be QFuture< QList<double> >, then the then() could work on a list of data. How does QFuture<double> "work" in the first place, I thought mapped() would return a list/QFuture< QList<double> >?

Asperamanca

QFuture itself models a sequential container (see QList<T> results(), resultCount(), resultAt()...)
result() simply returns the first result - and in some cases (like when using QtConcurrent::run) there will only be one result.

And I think I know what's going on now...I noticed before that a QFuture<double> would implicitly convert to a double in some cases. And I think this is what happens in course of the continuation.
Now if this implicit conversion is built on top of QFuture::result(), it would only use the first result. I don't believe that is intended.

EDIT: I think I'll prepare a full example and post a bug report on this.

J.Hilk

@JonB do you use a QFutureIterator to iterate over the returned result ?

Asperamanca

@J-Hilk No, I just use a ranged-for loop. Since QFuture models a linear container, that should be equivalent.

Asperamanca

But resultCount() also yields 1 only.

J.Hilk

@Asperamanca it should,

I don't usually use QtConcurrent and nearly never .then, So I'm also kinda driving blind here :D

JonB

@J-Hilk said in Continuation not working with QtConcurrent::mapped:

@JonB do you use a QFutureIterator to iterate over the returned result ?

Nope. I work like @Asperamanca. And confirm resultCount() == 1.

I don't usually use QtConcurrent and nearly never .then, So I'm also kinda driving blind here :D

:D

@Asperamanca said in Continuation not working with QtConcurrent::mapped:

EDIT: I think I'll prepare a full example and post a bug report on this.

Please post link here and I will join, I too would like to understand whether this behaviour is intended or accidental.

SGaist

Hi,

The documentation of then mentions:

The continuation can also take a QFuture argument (instead of its value), 
representing the previous future. This can be useful if, for example, QFuture has 
multiple results, and the user wants to access them inside the continuation. Or the 
user needs to handle the exception of the previous future inside the continuation, 
to not interrupt the chain of multiple continuations.

Wouldn't that match your use case ?

JonB

@SGaist
I read all the stuff like this, and it did not enlighten me. If you think the OP's requirement --- calling a function in then() either on an accumulated QList<double> or on individual doubles and whatever wrapping in the way of QFuture around it --- please show an extract of code you propose which (a) compiles! and (b) processes all doubles returned. Using mapped() and then(). Because I cannot.

Asperamanca

@SGaist I don't see how that is very useful with longer pipelines and multiple results in practice. It puts the whole burden of handling multiple, parallel executions on the continuation function (=me). I could just as well use QtConcurrent::run with a QList<double> as single input, and write all my functions to handle a container instead of a single value.
All the advantages of optimizing the thread usage would be gone, because at every stage, you'd have to wait for all results to be in (even if I used a nested QtConcurrent::mapped inside my continuation)

Or you'd start writing something that can handle QFuture<QFuture<QFuture<double>>>, but where would that end?

Asperamanca

BTW, here's a kind of workaround when you want to run a continuation on a range of things:

auto fMakeFuture = [](const double input)
    {
        return QtConcurrent::run(calculateArea, input)
                           .then(formatArea);
    };

// We have to create the futures eagerly, otherwise the threads won't start
// views are lazy, use 'normal' ranges::transform instead
std::vector<QFuture<std::string>> vecFutures;
std::ranges::transform(vecInputs, std::back_inserter(vecFutures), fMakeFuture);


// Access the results lazily. We'll only block when we actually start using a result
auto viewResults = vecFutures
                   | std::views::transform([](const auto& future) { return future.result(); });

JKSH

@Asperamanca said in Continuation not working with QtConcurrent::mapped:

    QList<double> inputs{2.0, 3.0, 5.0, 10.0, 12.0};
    auto future = QtConcurrent::mapped(inputs, calculateArea)
                                      .then(formatArea);
    future.waitForFinished();

The API is not very nice... but the following should work (although it's less parallelized than your fMakeFuture approach):

auto future = QtConcurrent::mapped(inputs, calculateArea)
                           .then([](QFuture<double> intermediate) { return QtConcurrent::mapped(intermediate.results(), formatArea); })
                           .unwrap();
qDebug() << future.results(); // Automatically waits for finished

JKSH

@Asperamanca said in Continuation not working with QtConcurrent::mapped:

BTW, here's a kind of workaround when you want to run a continuation on a range of things:

I wrote my Rube-Goldberg solution above, then admired your solution, then realized:

auto future = QtConcurrent::mapped(inputs, [](double input) {
    return formatArea(calculateArea(input));
});

SimonSchroeder

I guess that the intention is to have something like 2 mapped calls chained, so that both operations are executed in parallel on the elements. You also expect that when one element is finished with the first mapping it is immediately available for the second mapping.

What we have figured out so far is that mapped() returns a future. The second mapped (if it somehow would work) would wait for the full future to complete.

Also remember that thread communication/synchronization has heavy overhead. If you chain two different mapped calls together you will have this overhead twice. This means it is quite sensible to put the two function calls into a single mapped call. I can't think of anything in the STL to merge two function calls to be passed as a new function pointer. You could come up with something like this (untested!):

template<class T, class U, class V>
auto chain(U(T) first, V(U) second) -> T(V)  
{
    return [=](T &arg){ return second(first(arg)); };
}

It can be called like this:

auto future = QtConcurrent::mapped(inputs, chain(calculateArea, formatArea));

(Only while writing this I noticed that it is really close to the other solutions using a lambda.)

BTW, performance will be really bad if 1) the amount of work to do for each mapping is just tiny or 2) elements of the array are tiny and the threads will battle caching on different cores. Both can be addressed by handling the mapping in chunks. A much easier way to do this (that I know of) is OpenMP:

const int size = inputs.size();
#pragma omp parallel for shared(inputs,ouputs)
for(int i = 0; i < size; ++i)
{
    double tmp = calculateArea(inputs[i]);
    outputs[i] = formatArea(tmp);
}

The #pragma omp handles everything. You can specify if you want to synchronize after the loop, you can pick from different schedules, and set the chunk size of the schedule. Also, the actual source code is still quite readable (it is just an annotation of regular C++ code with a few restrictions on the loop).

Asperamanca

@SimonSchroeder said in Continuation not working with QtConcurrent::mapped:

What we have figured out so far is that mapped() returns a future. The second mapped (if it somehow would work) would wait for the full future to complete.

Why? That would be up to the implementation. It could track individual results (if there are many), and start those that can start.

Also remember that thread communication/synchronization has heavy overhead. If you chain two different mapped calls together you will have this overhead twice. This means it is quite sensible to put the two function calls into a single mapped call. I can't think of anything in the STL to merge two function calls to be passed as a new function pointer. You could come up with something like this (untested!):

Overhead, sure. It's my responsibility to only start work this way where it makes sense to move it to separate threads. My example is certainly not a good candiate :-)

In general, I wouldn't mind if ".then" on a "mapped" future simply wouldn't compile (best), or the behavior was documented and I got a runtime error (also ok). That is silently drops everything but the first result is "unexpected behavior".

I wonder about the design decision to make a QFuture both a single value AND a container...but I guess, that ship sailed long ago.

Asperamanca

Bug posted as https://bugreports.qt.io/browse/QTBUG-133522