Motivation for multithreading in my App

Absurd

Wow.
I gotta say, this sounds really promising...
I like the idea that I have one signal/slot channel (void message(QString, QVariantMap)).

3 more more follow up questions:

@MrShawn said in Motivation for multithreading in my App:

This allows me to not have to define numerous signals and slots, and instead based on the message type look at the relevant data from the map and do work with it. There are pros and cons to this i am sure but at least you are not constantly redefining your interface.

Is the QString in void message(QString, QVariantMap) used as a key for QVariantMap? if so how?
What are the Cons here?
I would very much love to not have to redefine the Device interface and make it "ad-hoc" for the specific requests of that GUI.

@MrShawn said in Motivation for multithreading in my App:

What is nice about your interface inheriting QObject is you can use moveToThread() method so your code to set up your module would look something like this.

"MyModule" here, I assume, refers to a module that has access to the Device, so my question is: how am I supposed to keep the MyModule thread "alive"?
What I mean is, what does it suppose to do while waiting for signals?

For example, currently, in my single-thread implementation, the App is kept "alive" thanks to:

int main(int argc, char* argv[]) {
    QApplication app(argc, argv);

    GUI gui;
    gui.show();

    return app.exec(); // keeps the main-thread "alive" by running the event-handling loop
}

In other words, I guess my question is: How would MyModule look like?

MyModule (aka "Worker") will subclass QObject
MyModule will implement a "run()" method that will be the entry point for the Worker's stack (its "main()"), and will also instantiate the Device
I will connect the QThread::started() signal to my MyModule::run() method
What should MyModule::run() do in order to be kept "alive"? Does it also run QApplication::exec()?

JonB

@Absurd

What should MyModule::run() do in order to be kept "alive"? Does it also run QApplication::exec()?

http://doc.qt.io/qt-5/qthread.html#run
http://doc.qt.io/qt-5/qthread.html#exec

By default run() will just call exec() for you, Note that is QThread::exec(), not QApplication::exec()!

MrShawn

@Absurd

No, the first QString is like a "key" for your message type, so that you know what keys to look for or are available in your QVariantMap.

2.Your slot that takes the message basically will be filled with if statements to do operations based on the QString messageType. if you over generalize everything into this one signal it is going to be less clear to anyone which signal is actually being emited and what is being done about it. For example a signal "requestData" is clear right from its declaration what it is for. But a signal sendMessage(QString type, QVariantMap data) could mean anything. Plus whatever overhead your losing on QVariantMap and the constant operations that need to be done to determine what to do with the message.

there are numerous ways to leverage QThread, and from when i search there are not "right" or "wrong" ways but it appears some ways are better for other scenarios than others.
They way I tend to use it and the way I envision would be best for your scenario you describe is to use the moveToThread method.

So basically your main would look something like this:

int main(int argc, char* argv[]) {
     QApplication app(argc, argv);

     GUI gui;
     gui.show();

     QThread moduleThread;  //event loop in another thread
     MyModule *myModule = LoadModule();
     myModule->moveToThread(&moduleThread);
     myModule.setupParams(params);
     //connect signals and slots
     QObject::connect(&moduleThread,&QThread::started,myModule,&MyModule::start);  //start may be your function that instantiates some other objects within your module.   you want to call this after you move to thread or else the objects could be made in the wrong thread and you're going to have some issues.
     QObject::connect(&gui,&GUI::sendMessage,myModule,&MyModule::getMessage);  // gui can now send a message to your device module 
     QObject::connect(&myModule,&MyModule::sendMessage,&gui,&GUI::getMessage);  //your device module can now send a message to your GUI
     QObject::connect(&moduleThread,&QThread::stopped,myModule,&MyModule::stop);
     QObject::connect(&moduleThread,&QThread::stopped,myModule,&MyModule::deleteLater); // you may need to play around with this, but you want to make sure your module is removed from memory properly and your program exits cleanly.  There are some "about to quit signals" and other ones you may want to look at to set this up properly, cant recall off the top of my head (even if you need to do something).  
     moduleThread->start();

     return app.exec(); // keeps the main-thread "alive" by running the event-handling loop
}

this code alone will keep that event loop "alive"

Absurd

Thank you both for your help!
@JonB :
I think I want to go with @MrShawn 's solution (not subclassing QThread, but rather moving MyModule to a QThread with moveToThread()), and in that solution I don't have the QThread::run() available in my MyModule (I have to subclass QThread to inherit QThread::run()...)

@MrShawn
From your code sample I can see that invoking moduleThread->start() will send a SIGNAL to MyModule::start(), but what should MyModule::start() do in order to be kept alive?

JonB

@Absurd
I didn't say anything about sub-classing! You would only need to do that if you needed to replace/change the behaviour of QThread::start/run/exec(), which you do not, since the default behaviour already suits you.

@MrShawn
From your code sample I can see that invoking moduleThread->start() will send a SIGNAL to MyModule::start(), but what should MyModule::start() do in order to be kept alive?

QThread moduleThread;  //event loop in another thread
...
moduleThread->start();

That's exactly as I said. I suggested you look at theQThread reference links I posted. You would see that QThread::start() calls QThread::run() calls QThread::exec() by default, unless you do something else about it. It's the QThread::exec() that is providing the event loop/keep alive which I think you are struggling with.

I don't know, maybe if you read carefully through the really simple usage in https://wiki.qt.io/QThreads_general_usage, which is the same as @MrShawn's, the explanation would help.

P.S.
I wonder if you're thinking myModule->moveToThread(&moduleThread); "replaces" the QThread. It does not, it just moves your module into the thread (its variables/slots/where it runs).

QObject::connect(&moduleThread,&QThread::started,myModule,&MyModule::start);  //start may be your function that instantiates some other objects within your module.

This is just going to call your slot via a signal when the thread starts, for you to do your own initializations, at the end of which your slot should simply return not call something else. It will still be going QThread::start() -> QThread::run() -> QThread::exec(). MyModule::start has nothing to do with QThread::start. Try renaming to MyModule::myStart and going QObject::connect(&moduleThread,&QThread::started,myModule,&MyModule::myStart); and it will work just the same. Does that clarify for you?

Absurd

@JonB Yes!
That clears everything up.

Thank you

Absurd

Ok, this is how I transformed my code:

In main.cpp:

int main(int argc, char* argv[]) {
    Controller controller(argc, argv);
    return controller.runApplication();
}

And in Controller:

Controller::Controller(int argc, char* argv[]) :  _app(argc, argv) {
    // does nothing
}

void Controller::initModule() {
    _devicesManager = new DevicesManager;
    _moduleThread = new QThread(this); // Controller will be the parent of QThread - and will be responsible for deleting it
    _devicesManager->moveToThread(_moduleThread);

    _moduleThread->start();
}

void Controller::initView() {
    _gui = new GUI;
}

int Controller::runView() {
    _gui->show();
    return _app.exec();
}

void Controller::connectSignals() {
    connect(_moduleThread, SIGNAL(started()), _devicesManager, SLOT(start()));
    // more to come, for now just connecting QThread's 'started' signal to  DevicesManager::start()
}

int Controller::runApplication() {
    initModule();
    initView();
    connectSignals();
    return runView();
}

For now, DeviceManager does absolutely nothing, except this:

void DevicesManager::start() {
    qDebug() << "Starting!";
}

(that's why I didn't bother to connect QThread::finished to DeviceManager::deleteLater for now - because it holds no memory allocation)

But I am experiencing two problems:

When I run it, the GUI runs fine, but then when I close the GUI by clicking the x button of the window, I get:
QThread: Destroyed while thread is still running
I don't see the "Starting!" printed out to the console, which makes me think that start didn't really run, not to mention running on a different thread...

J.Hilk

hi, @Absurd

you start your QThread object before you do the connection -> started signal is emitted before the connection is made -> slot gets not invoked.

_moduleThread->start();

Absurd

@J.Hilk ohh... how silly of me... :
Thank you.

Do you know what could be the cause for the second thing?
QThread: Destroyed while thread is still running

kshegunov

@Absurd said in Motivation for multithreading in my App:

@J.Hilk ohh... how silly of me... :
Thank you.

Do you know what could be the cause for the second thing?
QThread: Destroyed while thread is still running

Call QThread::quit and after that QThread::wait before you exit the application.

Addendum:

What is a good justification to use threads anyway?

Latency. Having a slow, blocking or otherwise high latency operation you don't need or want to wait for - you need to thread it, otherwise you do not.

Absurd

@kshegunov thanks for your reply. appreciate it.

@kshegunov said in Motivation for multithreading in my App:

Call QThread::quit and after that QThread::wait before you exit the application.

I should call QThread::quit & QThread::wait from Controller's destructor, right?

kshegunov

@Absurd said in Motivation for multithreading in my App:

I should call QThread::quit & QThread::wait from Controller's destructor, right?

Yes, that is correct. You should also free your worker objects:

QObject::connect(_moduleThread, &QThread::finished, _devicesManager, &QObject::deleteLater);

Absurd

@kshegunov why not just delete _deviceManager after Qthread::wait() has returned?

@MrShawn said:

No, the first QString is like a "key" for your message type, so that you know what keys to look for or are available in your QVariantMap.

Can you please give a usage example for that?
I couldn't figure out why isn't void message(QString, QVariant) enough...

kshegunov

@Absurd said in Motivation for multithreading in my App:

@kshegunov why not just delete _deviceManager after Qthread::wait() has returned?

It's possible, but it still belongs to the worker thread, you may get warnings.

MrShawn

@Absurd

void message(QString msgType, QVariantMap data);

QVariantMap not QVariant.

QVarantMap is a map with QStrings for keys, so like say you emit a signal that is declared like above, it may look something like this.

QVariantMap params;
int param1 = 2;
double param2 = 2.2;
params.insert("param1",param1);
params.insert("param2",param2);
emit message("runSomeSpecificMethod",params);

Then the receiving slot may look something like this

void getMessage(QString messageType, QVariantMap data)
{
     if (messageType.compare("runSomeSpecificMethod",Qt::CaseInsensitive) == 0){
          SomeSpecificMethod(data.value("param1").toInt(),data.value("param2").toDouble());
     }
}