[Development] Evolving Qt's multithreading API

[Sze Howe Koh]

On 21 February 2013 02:16, Corentin Jabot <corentin.jabot at gmail.com> wrote:
> Hi. I'm the one Olivier mentioned :p
>
> I didn't have time to pursue further the work I started, but I intend
> to, someday.
>
> The plan, as suggested by thiago was to have a QThread::run(functor)
> method acting exactly like QtConcurrent::run, but using a new QThead.
> A similar QThreadPool::run function would call a functor in a thread
> allocated in its pool.

I think the the plan is good; QtConcurrent::run() always felt
out-of-place with the filter-map-reduce API. We just need to be
mindful of how we integrate it into Qt -- the solution should cover
all 3 of functors, QRunnable, and parallel event loops.

I notice you also have QThread::run(QRunnable*). Let's take into
consideration the existing methods QThreadPool::start() and
QThreadPool::tryStart(). It's inconsistent to have:

    QThread::run(QRunnable*)
    QThreadPool::start(QRunnable*)

(Look for "static polymorphism" in
http://doc.qt.digia.com/qq/qq13-apis.html) I think we should either
make the new functions match the old names, or rename the old
functions to match the new names. (rename = create new names,
deprecate old names)


> Those function return QFuture and do not require event loop so this
> silly snippet work:
>
> int main() {
>     auto future = QThread::run( []() { qDebug() << "Hello world" ; } );
>     future.waitForFinished();
> }
>
> My goal was to make it as simple as I could.

Sounds good


> Anyway, I don't think we should returns a QThread*. the underlying
> implementation does not mater and shouldn't mater.
> What would you do with a QThread* you can't do with a QFuture ?

Apologies, I didn't realize that you had a version that returns
QFuture; I only saw the one that returned void. (Line 115,
https://codereview.qt-project.org/#patch,sidebyside,45297,5,src/corelib/thread/qthread.h)
I think returning QThread* is better than void. I still can't decide
if QThread* or QFuture is better for these new methods, however.

Technically, returning a QThread does hide the implementation. The
programmer only sees the QThread, and doesn't know that you've
implemented a QRunnableThread underneath. The programmer already knows
about QThread (since s/he called QThread::run()), so getting a
QThread* won't be a big deal.


About QThread vs. QFuture... QThread:
- Lets the programmer postpone starting the thread
- Lets the programmer query and manipulate the thread using
event-driven programming (signals and slots)

On the other hand, QFuture makes it easy to retrieve the function's
return value.

I leaned towards returning QThread* because QtConcurrent::run()
couldn't use most of QFuture's features anyway, and QThread* can be
returned from all 3 static functions:

    static QThread* QThread::setupSimpleThread(QRunnable *runnable);
    static QThread* QThread::setupSimpleThread(Function func, Args arg, ...);
    static QThread* QThread::setupEventLoop(QObject* worker);

...but I'm not sure if losing the functor's return value is worth it.


> The Thread is started right-away, you can't really interrupt the
> function until its done, etc.... but you could delete the thread while
> it's running, so yeah, I'd rather not a return a pointer to the
> thread.

I don't think we have to worry about people deleting the QThread while
it's running. If we did, we'd have similar problems the
QAudioInput::start() (returns a QIODevice*) and
QNetworkAccessManager::get() (returns a QNetworkReply*).


> I find the signature "QThread::run(function)", quite straightforward,
> there is no confusion about what it does.
> I don't think the fact there is also the "static void run()" method is
> confusing, but maybe that's just me.

I agree that, by itself, "run()" is a good name for this feature. The
issue, however, is that QThread already has something completely
different, also called run(). Thus, our choices become more limited.

I disagree that calling them all "run()" is non-confusing. It produces
an inconsistent API. Imagine that a new user finds a class with these
functions:

    public static QFuture<T> run(QFunction&);
    public static void run(QRunnable*);
    protected void run();

Will this new user be reasonably able to guess their differences?


> That said, there is no real reason to put these functions in QThread,
> except the name.
> These functions could be put elsewhere, I actually started to work
> with a bunch of free functions before integrated them to QThread

I think we should try to keep it inside QThread if possible; creating
another class/namespace feels excessive


> The implementation use the same sort of generator that QtConcurrent.
> Thiago suggested making this feature only compatible C++11, which
> would make it easier to maintain. I actually envisaged to send a mail
> about that particular issue.
> Can c++03 really be dropped for that particular feature ?
> Also, I we were to make a c++11 only feature, what would be the
> benefits over std::async ?

I would love to see an implementation using C++11. I think that
decision can't be made lightly though, and should be a separate
discussion -- it has implications for all other Qt modules.

Compared to std::async, a Qt-based solution would also work for
QRunnable and support signalling.


> About running QObject* methods on a separate thread, it can become
> quite complex.
> Which slot is called when the thread start ?

The workerObject->moveToThread() approach seems to work quite well. No
slot should need to be called when the thread starts. The programmer
can choose to connect any signal (including QThread::started()) to any
slot, if the QThread* is available.


> Which signal makes the thread quit ?

The programmer can choose to connect any signal to QThread::quit(), if
the QThread* is available.


> Should the object be deleted afterwards ? Moved back to
> its original thread ?

In principle, the thread should only finish when the object has
finished is job. So, I'd imagine that the end of a thread would be
followed by the destruction of its objects. Of course, being a
low-level solution, the programmer can decide. Can someone find a use
case for an object to change threads multiple times?


> Considering the number of scenario, I'm not sure we would benefit from
> a function - the only factorizable part is
>
> QThread* t = new QThread();
> obj->moveToThread(t);
> connect(t, SIGNAL(finished()), t, SLOT(deleteLater()));

I suggested QThread::setupEventLoop() to make it easy to compare with
QThread::setupSimpleThread(), and to make it clear that the former has
an event loop while the latter doesn't. The goal here was to unify the
API. I'm not 100% happy with these names because they're quite long,
but I noticed that Qt traditionally chose clear names over short names


> Maybe we should first agree on how QThread sould work before trying to
> add yet-another-way.

Agreed.

Currently, QThread is Qt's fundamental, low-level thread-control
interface. It has the ability to run a parallel event loop, and
provides access to the thread's event dispatcher. Its API lets us
query and manipulate the state of a thread using event-driven
programming (i.e. signals + slots).

When I wrote my proposal, my goal was only to extend and unify the API
for QThread and QThreadPool; I didn't consider changing the classes'
roles/behaviour. Which of these characteristics should we incorporate
into the new API? Is there anything we should deprecate?


Regards,
Sze-Howe