[Development] Looking for Feedback QDeferred

[Denis Kormalev]

Hi,

Sorry for the kinda late response, but you probably would also like to check https://github.com/dkormalev/asynqro <https://github.com/dkormalev/asynqro> for ideas about API improvements. 

--
Regards,
Denis Kormalev

> On Feb 12, 2019, at 1:38 AM, Juan Gonzalez Burgos <juangburgos at gmail.com> wrote:
> 
> Hi,
> 
> Looking at it with the “Qt Creator” hat on, i.e. with the mindset of “could we replace what we do in Qt Creator with our extension of QtConcurrent".
> (http://code.qt.io/cgit/qt-creator/qt-creator.git/tree/src/libs/utils/runextensions.h <http://code.qt.io/cgit/qt-creator/qt-creator.git/tree/src/libs/utils/runextensions.h> adds the convenience and actual runnable based around QFuture and QFutureInterface.)
> I suppose this is a very UI-interaction focused, and high-level view on things ;) but it is something that the QFuture/QFutureInterface/QFutureWatcher API supports.
> 
> Wow, first of all thanks for taking the time for this awesome feedback. 
> I guess the QFuture/QFutureWatcher design was driven by Qt's needs as I developed QDeferred for my very own needs. I suppose one just have to use the tool that best fits the problem.
> 
> 1) I think the chaining/promises style is an improvement to the need to always use QFutureWatcher. We more often need to carry a QFutureWatcher member around than I like (even with a helper function Utils::onResultReady, the moment you need to handle various signals you’ll want to stick to a single QFutureWatcher)
> 
> Agree. There are use cases where QFutureWatcher is better.
> 
> 2) We use QFuture/QFutureInterface for a generic progress UI. Basically you tell a central progress UI manager about your QFuture, and that shows a progress bar for it, including a cancel button.
> 
> What about multiple “subscribers” to a task? The progress UI needs to act on progress info, and finished / success status changes. On a glance I didn’t see if that is possible with your API.
> 
> Yes is possible to have multiple subscribers, thank you for pointing at it, I forgot to mention it explicitly in the readme. It is done as follows:
> 
> 	QDeferred<int> defer = myMethod()
> 	.done([](int val) {
> 	
> 	})
> 	.fail([](int val) {
> 
> 	});
> 
> 	// we can pass "defer" around since is a explicitly shared object
> 	// ...
> 
> 	// subscribe elsewhere
> 	defer
> 	.done([](int val) {
> 
> 	})
> 	.fail([](int val) {
> 
> 	});
> 
> And if the QDeferred was already resolved/rejected when a new subscription is done, then the callback is called inmediatly (depending on the connection-type, more on this later). I will add this to the document. 
> 
> I didn’t see cancel functionality in your work, do you have thoughts on this?
> 
> I didn't think of this, haven't had the need but it is a great idea! I think it should not be too hard to implement. Maybe an API method called "cancel" and a callback called "cancelled" so we know when the process has actually been cancelled,
> 
> The implementation for progress seems to be a bit awkward in comparison to QFutureInterface, and doesn’t seem to be separate from the result type? Progress can be pretty separate from actual result producing, i.e. a file system search will be able to provide very fine grained progress information, but might only report a handful of results.
> 
> Yes and no, actually this was a hard decision for me. One main differentator with QDeferred is that there are N result types, so we could get around the issue as follows:
> 
> 	QDeferred<QByteArray, int, QString> defer = myMethod()
> 	.progress([](QByteArray result, int progress, QString message) {
> 		Q_UNUSED(result);
> 		qDebug() << "Progress" << progress << "% , details :" << message;
> 	})
> 	.done([](QByteArray result, int progress, QString message) {
> 		Q_UNUSED(progress, message);
> 		// do something with the QByteArray results
> 	});
> 
> The fact that you have N result types does not mean you have to use all of them in every callback. You could use some of them for progress info and some others for results. I know this might come as "akward", but what actually constitute a "progress"? At some point I though of adding a specialized <int> or <QString> API for the progress but decided not to because it would be limiting. E.g. one of my use cases was to bring large chunks of historic data from a server, and the "progress" for that use case was partial data blocks which I could inmediatly display in a chart as they arrived, so one of my return types was a reference to that partial data block which I only used in the progress callback. Maybe there is a better way to achieve this, but I couldn't find one that met all my needs.
> 
> Another thing that QtConcurrent handles for us, it to guard against “too much progress reporting”. I.e. if a loop from 1 to 1000000 reports every single step as progress, this would block the UI/main thread with progress updating. QtConcurrent makes sure that actual progress reporting to the receiving thread only happens in “sensible” intervals.
> 
> This sounds like a good idea, but makes me wonder; isn't it the reponsibility of the user to create sensible reporting? I mean, I could drown my CPU with a for-loop, is it the fault of the for-loop or my fault for using it incorrectly? Nevertheless it is indeed always a good idea to program in a defensive way. Can I ask how does Qtconcurrent implements this protection?
> 
> One nice thing about QFuture/QFutureInterface is that one doesn’t actually need to create an _actual_ async task to use the same functionality. We use that at a few places for showing progress for things that are not actually running in a thread, but wait for other asynchronous tasks to finish (e.g. QProcess). But that’s just a convenience that avoids having a separate API for it.
> 
> Ah, something else I didn't document, I am sorry. QDeferred works similar to Qt signals and slots in the sense that it has a "Qt::ConnectionType" as an argument when defining each callback. So QDeferred also works within the same thread:
> 
> 	defer.progress([](int val) {
> 		qDebug() << "Counting in the same thread :" << val;
> 	}, Qt::DirectConnection);
> 
> 	for (int i = 0; i < 100; i++)
> 	{
> 		defer.notify(i + 1);
> 	}
> 
> The progress callback will be called, eventhough the notify is done in the same thread. 
> 
> 3) Reporting intermediate results is something that we heavily use for things like e.g. the search functionality. While the search is running, you want the UI to already present what was found so far.  
> 
> That is what I tried to achieve with the progress callback as it is, but I understand there is still room for improvement in the API. I will give it more thought.
> 
> Thanks again for the feedback.
> 
> On Tue, Feb 12, 2019 at 8:02 AM Eike Ziller <Eike.Ziller at qt.io <mailto:Eike.Ziller at qt.io>> wrote:
> Hi,
> 
> Looking at it with the “Qt Creator” hat on, i.e. with the mindset of “could we replace what we do in Qt Creator with our extension of QtConcurrent".
> (http://code.qt.io/cgit/qt-creator/qt-creator.git/tree/src/libs/utils/runextensions.h <http://code.qt.io/cgit/qt-creator/qt-creator.git/tree/src/libs/utils/runextensions.h> adds the convenience and actual runnable based around QFuture and QFutureInterface.)
> I suppose this is a very UI-interaction focused, and high-level view on things ;) but it is something that the QFuture/QFutureInterface/QFutureWatcher API supports.
> 
> 1) I think the chaining/promises style is an improvement to the need to always use QFutureWatcher. We more often need to carry a QFutureWatcher member around than I like (even with a helper function Utils::onResultReady, the moment you need to handle various signals you’ll want to stick to a single QFutureWatcher)
> 
> 2) We use QFuture/QFutureInterface for a generic progress UI. Basically you tell a central progress UI manager about your QFuture, and that shows a progress bar for it, including a cancel button.
> 
> What about multiple “subscribers” to a task? The progress UI needs to act on progress info, and finished / success status changes. On a glance I didn’t see if that is possible with your API.
> 
> I didn’t see cancel functionality in your work, do you have thoughts on this?
> 
> The implementation for progress seems to be a bit awkward in comparison to QFutureInterface, and doesn’t seem to be separate from the result type? Progress can be pretty separate from actual result producing, i.e. a file system search will be able to provide very fine grained progress information, but might only report a handful of results.
> Another thing that QtConcurrent handles for us, it to guard against “too much progress reporting”. I.e. if a loop from 1 to 1000000 reports every single step as progress, this would block the UI/main thread with progress updating. QtConcurrent makes sure that actual progress reporting to the receiving thread only happens in “sensible” intervals.
> 
> One nice thing about QFuture/QFutureInterface is that one doesn’t actually need to create an _actual_ async task to use the same functionality. We use that at a few places for showing progress for things that are not actually running in a thread, but wait for other asynchronous tasks to finish (e.g. QProcess). But that’s just a convenience that avoids having a separate API for it.
> 
> 3) Reporting intermediate results is something that we heavily use for things like e.g. the search functionality. While the search is running, you want the UI to already present what was found so far.
> 
> 
> Br, Eike
> 
> > On 11. Feb 2019, at 12:49, Juan Gonzalez Burgos <juangburgos at gmail.com <mailto:juangburgos at gmail.com>> wrote:
> > 
> > Hi guys,
> > 
> > Sorry to bother you with yet another promise/deferred library for Qt. I am looking for feedback.
> > 
> > https://github.com/juangburgos/QDeferred <https://github.com/juangburgos/QDeferred>
> > 
> > Thanks.
> > _______________________________________________
> > Development mailing list
> > Development at qt-project.org <mailto:Development at qt-project.org>
> > https://lists.qt-project.org/listinfo/development <https://lists.qt-project.org/listinfo/development>
> 
> -- 
> Eike Ziller
> Principal Software Engineer
> 
> The Qt Company GmbH
> Rudower Chaussee 13
> D-12489 Berlin
> eike.ziller at qt.io <mailto:eike.ziller at qt.io>
> http://qt.io <http://qt.io/>
> Geschäftsführer: Mika Pälsi,
> Juha Varelius, Mika Harjuaho
> Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B
> 
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> https://lists.qt-project.org/listinfo/development

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20190303/c370199b/attachment.html>