Motivation for multithreading in my App

Absurd

I am very close to complete the implementation of a gui using Qt.
The gui is a collection of Qt Classes (let’s call that the “View”), and supposed to display some information regarding a device.
The device is represented by a class (called Device) that knows how to access the device through low level API provided by its device-driver.
I want to keep the gui implementation completely separated from the implementation of Device (actually, Device is an interface, not a concrete Class).
So I thought about having Device expose some general interface that can be used by the "View".
The “View” will then invoke those functions to fetch data regrading the device, and will display it.

So my questions are:
Is there a good justification to use multiple threads for that? (for example, one thread will run & handle the Gui, and the second one will query the device and will “service” the thread that runs the Gui)
What is a good justification to use threads anyway?

Please consider the following (don’t know if it’s relevant to the decision of using multithreading, but anyway…):

Access to the device may be very slow… I may want to have some cached data buffer that will be updated regularly by reading from the device
Device may be inaccessible and the gui needs to survive that
In case of using 2 threads - how will data be moved from one thread (the Device) to another (the "View")?

Thanks!

dheerendra

Looking at the 3 reasons you mentioned at the end, you can have multi threading. You can use the signals/slots for communicating between threads.

JonB

@Absurd
It is reasonable to use separate threads, keeping the UI quite separate from your device.

So I thought about having Device expose some general interface that can be used by the "View".
The “View” will then invoke those functions to fetch data regrading the device, and will display it.

The most obvious thought for that "general interface" would be Qt's signals & slots? You can signal/slot across threads happily. [Post just crossed with @dheerendra!]

Absurd

Is it reasonable to use Signals & Slots for the communication between the View and the Device?
If so, how will that work?
(subclass of) Device will be instantiated in one thread,
gui execution will be running in second thread,
Now suppose a button is clicked in the gui, that require fetching data from the device - who emits the signal and who will implement the slot?

MrShawn

So I have gotten some comments saying I shouldn't do it, but this is what I had to do to get Signals and Slots working with plugins...

I have my interface inherit from QObject. This allows me to define the signals and slots on the interface level, and then implement them when I actually build the implementation plugin. (Not that I am saying you are doing plugins, just interfaces inheriting QObject seem to be what is non standard).

So to try to answer your question you will largely go with an asynchronous approach to your design. For example, when pressing the button you mention on the gui to request information a signal is emitted by your GUI class requesting for the information (rather than accessing the methods directly). Your module implementation has a slot that picks up on that signals and then does what is needed to be done in order to get your information, probably just a slot around your current method. That information is then emitted via a signal from your module. Your Gui then has a slot designed to take that signal and its data and and do what the gui does with it.

A lot of times I will generically make a slot/signal using something like

void message(QString msgType, QVariantMap data);

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.

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.

QThread *moduleThread = new moduleThread;
MyModule *myModule = LoadModule();
myModule->moveToThread(moduleThread);
/*connect signals and slots
...
*/
moduleThread->start();

In general it seems to me that signals and slots are very powerful when used to their full potential. It allows you to decouple your classes, otherwise your GUI class would at least need a reference to your device module. Now your GUI exists without knowledge of that class or module, instead it just sort of asks for information and hopes that it gets it. It is of course up to you to make sure that the module returns the information requested.

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());
     }
}