How to eliminate msleep() from a worker thread and use a Timer in the GUI thread instead?

Diracsbracket

Hi,
I instantiate a SystemMonitor every time the PerformanceMonitor page gets pushed on my StackView, with SystemMonitor pushing several member functions to their own thread, each gathering some system statistics.

The whole things works reasonably well, except that for the moment, the updates are initiated from within the threads of SystemMonitor in updaters like the following:

...
void SystemMonitor::procUpdater()
{
    while (m_running)
    {
        {
            QMutexLocker locker(procInfoMtx);
 
            procInfo->update();

           //Somehow passing my model as signal arg does not work,
           //so retrieve it by callback... :-(
           //See https://forum.qt.io/topic/122095/qsortfilterproxymodel-as-signal-argument-is-undefined-in-qml 
            emit procModelReady();
        }

        QThread::msleep(UM_UPDATE_INTERVAL);
    }
}

One problem is that when the PerformanceMonitor gets popped from the
StackView and therefore the SystemMonitor gets destroyed, the UI blocks
for up to 1 second (UM_UPDATE_INTERVAL is set to 1000), depending in which part of the msleep() the threads are.

SystemMonitor::~SystemMonitor() {
   ...
    {
        QMutexLocker locker(procInfoMtx);
        procUpdaterThread->quit();
        procUpdaterThread->wait();
    }
}

One solution would be to initiate updates from the UI via a Timer in PerformanceMonitor instead of making the threads sleep using msleep(), but that would imply creating and starting a thread each time the timer triggers, something like:

void SystemMonitor::onRestartProcThreadRequested() {
    try {
        QMutexLocker lock(procInfoMtx)
        procUpdaterThread = 
        QThread::create(std::bind(&SystemMonitor::procUpdater, this));
        procUpdaterThread->start();
    }
    catch (...) {
        ...
    } 
}

and changing the threading function to something like:

void SystemMonitor::procUpdater()
{
    QMutexLocker locker(procInfoMtx);    
    procInfo->update();
    emit procModelReady(); 
}

There must be a there a better way to do this. Any suggestions will be much appreciated!

PS:
The rest of the implementation looks as follows:

PerformanceMonitor.qml:

Item {
    id: root
    ...
   SystemMonitor {
        id: sysmon
    }

    TableView {
        id: procTable
    }

    Connections {
        target: sysmon
        function onProcModelReady()  {
           //Somehow passing my model as signal arg does not work,
           //so retrieve it by callback... :-(
           //See https://forum.qt.io/topic/122095/qsortfilterproxymodel-as-signal-argument-is-undefined-in-qml  
           
            var model = sysmon.getProcProxy()
        
            if (model===null ||  model === procTable.model)
                return

            procTable.model = model
        }
    }
}

The system monitor is defined something as follows in C++:

class SystemMonitor : public QObject
{
    Q_OBJECT
    SystemMonitor(QObject* parent=nullptr);
    ~SystemMonitor();
    ...
signals:
    void procModelReady(); 
    ...

private:
    volatile bool m_running;
   ...
    QThread* procUpdaterThread;
    ProcInfo::pointer procInfo;
    QMutex* procInfoMtx;
    void procUpdaterDaemon();
}

SystemMonitor::SystemMonitor(QObject *parent)
    : QObject(parent)
    , m_running(true)
     ...
    , procUpdaterDaemonThread(nullptr)
    , procInfo(nullptr)
    , procInfoMtx(new QMutex())
{
    try {
        QMutexLocker locker(procInfoMtx);
        procInfo = ProcInfo::create(); 
        procUpdaterThread = QThread::create(std::bind(&SystemMonitor::procUpdater, this));
        procUpdaterThread->start();
    }
    catch(ProcessesParseError& err) {...}
}

In main.cpp:

    qmlRegisterType<SystemMonitor>("com.dirac", 1, 0, "SystemMonitor");
    qmlRegisterUncreatableType<ProcessProxyModel>("com.dirac", 1, 0, "ProcessProxyModel", "Type is not allowed to be instantiated");

Bob64

Have you considered using a QTimer in your thread, instead of sleeping? That would open up the possibility of being able to request the thread to terminate without it blocking before the next update.

Your thread would need its own event loop to allow QTimer to be used.

Diracsbracket

Thanks @Bob64!

@Bob64 said in How to eliminate msleep() from a worker thread and use a Timer in the GUI thread instead?:

Have you considered using a QTimer in your thread, instead of sleeping?

That is also what the Qt QThread doc suggests, but I didn't really understand how to achieve that.

@Bob64 said in How to eliminate msleep() from a worker thread and use a Timer in the GUI thread instead?:

Your thread would need its own event loop to allow QTimer to be used.

How would I go about that? Any pointers on how to implement such an event loop inside a thread?

Edit: The Qt docs says that start(...) calls run() and the default implementation of run() simply calls exec() which

enters the event loop and waits until exit() is called

So, by default there seems to be an event loop associated with a QThread. How can I the use a QTimer with it to replace msleep()?

SGaist

Hi,

If you really need threading then use the worker object approach. No need to subclass QThread at all.

Depending on the number of monitors you have you might even be able to just have them all using a single thread.

Diracsbracket

@SGaist
I think I am starting to see the light ...
So in the Controller part of the Worker example of the Qt doc, all I need to do then is to create a QTimer and configure that one to emit operate(...) signals at the desired rate (or directly connect the timer timeout signal to doWork)

Thanks a lot!

Diracsbracket

I just completed the conversion to the Worker object method, and it works beautifully.

@SGaist said in How to eliminate msleep() from a worker thread and use a Timer in the GUI thread instead?:

Depending on the number of monitors you have you might even be able to just have them all using a single thread.

That works indeed, and the code is now much cleaner and concise.
Thanks again @SGaist!