PDA

View Full Version : Using QTimer in a QThread



Cruz
12th April 2011, 10:09
I don't get it. I want to start a thread that periodically executes a function using a timer.



Thread::Thread(QObject *parent) : QThread(parent)
{
start();
}

void Thread::run()
{
QTimer t;
connect(&t, SIGNAL(timeout()), this, SLOT(tick()));
t.start(100);
//t.moveToThread(this);
exec();
}

void Thread::tick()
{
qDebug() << currentThreadId() << "tick()";
}



class ThreadTest : public QWidget
{

Thread thread;

...
};

ThreadTest::ThreadTest(QWidget *parent)
: QWidget(parent)
{
qDebug() << QThread::currentThreadId() << "is the main thread.";
}



And the output I get is:


0x12b8 is the main thread.
0x12b8 tick()
0x12b8 tick()
0x12b8 tick()
0x12b8 tick()
...


Obviously, the timer is still running on the main thread. Using the moveToThread() doesn't make a difference. What's not right about this?

Changing the timer to an infinite loop with sleep() works though.



void Thread::run()
{
forever
{
tick();
sleep(1);
}
}




0xa04 is the main thread.
0xfa0 tick()
0xfa0 tick()
0xfa0 tick()
...

MarekR22
12th April 2011, 10:54
Problem is that you are doing that wrong (http://labs.qt.nokia.com/2010/06/17/youre-doing-it-wrong/)!


class TestClass : QObject
{
Q_OBJECT
public:
TestClass(QObject *parent=0);

public slots:
void tick();
};

TestClass::TestClass(QObject *parent) : QObject(parent)
{
QTimer *t = new QTimer(this);
connect(t, SIGNAL(timeout()), this, SLOT(tick()));
}

void TestClass::tick()
{
qDebug() << currentThreadId() << "tick()";
}

// and somewhere
QThread *thread = new QThread(something);
TestClass *object = new TestClass(); // can't pass parent!!!!!!
object->moveToThread(thread);
thread->start();

....
delete object;
thread->WaitFor();
// delete thread;

high_flyer
12th April 2011, 11:35
Problem is that you are doing that wrong!
I don't see what he is doing wrong, in respect to the link you posted, except for what he tried in the second bit, which is wrong because you need to be in the "other thread" to call moveToThread().
The link you provided is talking about using moveToThread() wrong usage, and that you should not always use a subclass of QThread for your threads.
But there are many cases where this is the RIGHT way to go.
Even if from an OOP design point of view this is wrong, from implementation point of view this still should work (the original code with out moveToThread()).
And if it doesn't we need to understand why.

His code is almost the same as the example code QThread docs, just that he uses a QTimer and not a QTcpSocket.
Since the QTimer is allocated in run() its should have the thread affinity of the new thread, and from what the OP shows, it looks like its affinity is to the main GUI thread, which is surprising.

My guess is that it has to do with calling start() in the thread constructor.
Try removing the start() form the constructor, and call it normally from outside, see if this changes things.

MarekR22
12th April 2011, 11:43
Read carefully QObject::connect especially about last 5ht argument (Qt::ConnectionType-enum)!

Added after 5 minutes:

Note that your QThread belongs to main thread so default connection here is redirected through posting an event.
Read article I gave you at beginning to avoid other mistakes.

high_flyer
12th April 2011, 11:48
Read carefully QObject::connect especially about last 5ht argument (Qt::ConnectionType-enum)!
Right as if you read carfully you will discover that the default is Qt::AutoConnection, which is:

Qt::AutoConnection 0 (default) If the signal is emitted from a different thread than the receiving object, the signal is queued, behaving as Qt::QueuedConnection. Otherwise, the slot is invoked directly, behaving as Qt::DirectConnection. The type of connection is determined when the signal is emitted.
EDIT:
However, now I see why the thread affinity is "wrong".
Its not - since the QThread object it self is in the main GUI thread, you are getting the main thread id.
The timer is correctly in the new thread, but the signal is evoking a slot in the main thread!


Note that your QThread belongs to main thread so default connection here is redirected through posting an event.
So, it has nothing to do with the fact the an object created in the QThread::run() should have the new thread affinity!


Read article I gave you at beginning to avoid other mistakes.
I have read it more than once, and I know it not from yesterday.
The article is good, but has nothing to do with the question in the first post! - unless you can show otherwise.

Conclution:
@OP: your code is behaving correctly.
Your timer IS in the new thread, but your slot is in the main thread.

Cruz
12th April 2011, 14:48
@OP: your code is behaving correctly.
Your timer IS in the new thread, but your slot is in the main thread.

Thanks, this is a very reasonable explanation. A simple thread.moveToThread(&thread) in the constructor of the gui did the trick and now the slot is executed in the worker thread.

MarekR22
12th April 2011, 14:51
NOOOOO!
Read article I gave you! NEVER EVER move thread to itself! You are corrupting handling of signals and slots from QThread which should be used to manipulate thread.
Look on my code!

Cruz
12th April 2011, 15:52
Ah sorry, I didn't study your instructions thoroughly enough. So if I understand right, moving the thread to itself means that the inherited signals and slots from QThread will also be handled in that event loop instead of in the main thread. So what? How does that corrupt things?

high_flyer
12th April 2011, 16:18
A simple thread.moveToThread(&thread) in the constructor of the gui did the trick and now the slot is executed in the worker thread.
Well, then comes the question of design.
Even though moving the the thread object to the thread the QThread object created is permited, its a clear sign of a design flaw.
Maybe if you explain what you are trying to do the correct way can be offered.


I want to start a thread that periodically executes a function using a timer.

If you need some functionality done in some time interval, then you better do things the other way around.
Have a timer in the main thread, and in the timeout slot create a new thread that does the work do be done.
You might want to have a look at QtConcurrent for a very simple way of doing that.
http://doc.qt.nokia.com/latest/qtconcurrent.html

Cruz
12th April 2011, 16:40
OK here is what I want to do. I have a GUI application that controls a robot via serial communication. The only requirement is simple. The communication loop with the robot should execute with a steady pace (every 10 ms), no matter how busy the gui keeps the CPU. The gui shows an interactive OpenGL 3D model of the robot, so when the user is rotating the model wildly and frequent redraws occur, the gui can get very cpu hungry.

high_flyer
12th April 2011, 17:08
In that case there are several options:
1. using your original construct you can use QtConcurrent::run() in the timer slot to run a method in which you handle the serial communication to the robot.
This will ensure that both your timer and the serial communication are not done in the GUI thread.
2. Put the serial communication functionality in a QObject, allocate that object also in the run() where your timer is allocated, and connect the timer timeout to a slot of your serial communication object.
3. handle the serial communication in run(), in a while loop, and sleep for 10ms every iteration, then you don't need signals slots and timers.
This you also can implement using QtConcurrent.

Cruz
12th April 2011, 22:35
When I implemented option 3. I found that sleeping in a loop gives me a much more steady timing than using a QTimer. Actually, I measure the runtime of each iteration using Windows' high performance counter and adjust the amount of time to sleep accordingly. This gives very satisfying results. Also, Sleep() seems to be a bit more accurate than msleep().

In option 1. I would use a thread to run the QTimer and then invoke another thread (or take one from a thread pool) each time I call QtConcurrent::run()? It sounds to me like it has one too many complications.

Option 2. can be used as a complementary technique along with either option 1 or 3. The only reason I would do that is to circumvent the "you are not doing it right" hack that was linked above. Personally, I find it very natural and intuitive to subclass QThread and do all thready things right there. And if it requires a moveToThread() call that is frowned upon, then so be it. Only if it really does mess something up I'm willing to introduce yet another object, but my question from my earlier post is still unanswered.

Also I'm hoping that using a QThread and giving it a high priority might improve the serialcom steadiness when the GUI is going nuts, but I haven't stress tested this assumption yet.

So, what's the advantage of using QtConcurrent::run() instead of a QThread::run()? Since QtConcurrent apparently takes threads from a pool, I assume it's faster if you start a lot of small threads all the time. But does it really matter for one thread that runs through in an infinite loop as in option 3.? The remark in the documentation saying "Note that the function may not run immediately; the function will only be run when a thread is available." is discouraging.

wysota
12th April 2011, 23:20
How steady is "steady"? If you are not running a real-time operating system then you can't get exact timings no matter how hard you try. Threading will not help you here in any way.

high_flyer
13th April 2011, 09:15
Only if it really does mess something up I'm willing to introduce yet another object,
If objects are designed correctly, such that each object encapsulates a specific functionality, they are correct to be used, even if they are many.
Otherwise, you get pieces of code sharing implementation of several functionalities, causing dependencies which is a bad thing.

Any way, I offered the three options, since I don't know your full requirements.
Each of them has advantages and disadvantages, dependent on your needs.
Pick the one you think best suits your needs.



So, what's the advantage of using QtConcurrent::run() instead of a QThread::run()?
The advantage is encapsulation, which is a very good thing when programming with OOP language.
And a slim code.


. Threading will not help you here in any way.
@wysota:
Threading helps him to avoid the GUI thread, which according to him, can get very busy, thus influencing the serial communication he needs.

wysota
13th April 2011, 09:22
@wysota:
Threading helps him to avoid the GUI thread, which according to him, can get very busy, thus influencing the serial communication he needs.
This is virtual safety. Threads are scheduled by the operating system. If it decides not to schedule the communication thread in a particular point in time, it won't. If the communication thread has little to do compared to the other thread, it will get less time from the operating system increasing a chance to lose the "steadiness" required. Only real-time operating systems give guarantees about times and scheduling.

high_flyer
13th April 2011, 09:28
only real-time operating systems give guarantees about times and scheduling.
True.
But if you put things in another thread you have more control over what is going on.
You can also prioritize the threads for example.
Of course you can't expect real time behavior in a non real time system (well, that too is a question of granularity requirements), but if you know the limits in which you are working, the accuracy of the system maybe acceptable.
At any rate, by putting the two pars in separate threads you are decoupling them, which I think was one of the main gaols here.

wysota
13th April 2011, 09:53
But if you put things in another thread you have more control over what is going on.
Again, that's "virtual" control.


You can also prioritize the threads for example.
Not really and not always.


Of course you can't expect real time behavior in a non real time system (well, that too is a question of granularity requirements), but if you know the limits in which you are working, the accuracy of the system maybe acceptable.
That's gambling. At any point another process may pop in and be given priority over yours so that your process is scheduled not often enough to reach the required communication frequency and you have totally no influence on that.


At any rate, by putting the two pars in separate threads you are decoupling them, which I think was one of the main gaols here.
Sure. Only that it's completely pointless.

high_flyer
13th April 2011, 10:00
Common witek, so what are you saying?
You should not implement tasks that need time resolutions in ms range in non RTOS?
I am sure you, just as I have, made many such usages of threads that are "real time like" under windows/linux.

MarekR22
13th April 2011, 10:09
Ah sorry, I didn't study your instructions thoroughly enough. So if I understand right, moving the thread to itself means that the inherited signals and slots from QThread will also be handled in that event loop instead of in the main thread. So what? How does that corrupt things?
Simple example:
lets say user starts thread by pressing button, so you connect buttons click signal with start slot of thread. Every think should work since is done by the book.
But it will not work since you made a queued connection (objects are in two diffident threads since you've moved thread to itself and auto-detection of queued connection was done) and signal to start thread is waiting in event loop of thread which is not started so you have dead lock.

high_flyer
13th April 2011, 10:16
@MarekR22:
You example is wrong.
If QThread object didn't call start(), its run() is not running, thus the read does not exist, and QThread object's affinity in to the thread in which is was created.
You can't moveToThread() that does not exist.

MarekR22
13th April 2011, 11:22
Maybe we misunderstand each-other. I'm trying to proof that thread shouldn't be moved to itself!
Here is test code (stupid example but it works):

#include <QThread>
#include <QApplication>
#include <QPushButton>
#include <QTimer>

int main(int argCount, char* args[])
{
QApplication app(argCount, args);

QPushButton button("Start");
QThread thread;
QTimer timer;
timer.setInterval(2000);
timer.moveToThread(&thread);
// thread.moveToThread(&thread); // this shouldn't be done

QObject::connect(&button, SIGNAL(clicked()),
&thread, SLOT(start()));

QObject::connect(&button, SIGNAL(clicked()),
&timer, SLOT(start()));

QObject::connect(&timer, SIGNAL(timeout()),
qApp, SLOT(quit()));

button.show();
app.exec();
thread.exit();
thread.wait(1000);

return 0;
}
Program terminates after 2 second push button was pressed.
Uncomment line 15 and program will not terminate 2 seconds after button was pressed you have a dead lock (checked on Ubuntu 10.04).

Cruz
13th April 2011, 11:24
@wysota

As usually, you state a radical opinion in a radical way. I'm curious what your approach would be to solve the task as stated above, that a steady high frequency communication (100Hz) has to be maintained while the CPU can be potentially maxed out at times with other tasks. RTOS is not an option, the target OS is Windows. Maybe it's a useful piece of information that one iteration of the communication takes between 2 and 3 milliseconds and it barely needs any CPU, since most of it is I/O time.

Another advantage I see in threads is that often there is more than one core and while one core is struggling with the gui, the other core can still maintain the steady communication frequency. This would not be possible with only one process.

JohannesMunk
13th April 2011, 11:40
Hello Cruz,

I'm running several 1-2ms precise threads under windows. What significantly improved matters is to lock the threads to specific cpu cores. By default windows "pushes them around" quite a bit, with negative influence on timing precision.



#ifdef Q_WS_WIN
# include "windows.h"
#endif

#ifdef Q_OS_UNIX
# include "unistd.h"
# include "sys/syscall.h"
# include <sys/types.h>
# include <sys/time.h>
# include <sys/statfs.h>
# include <stdio.h>
# include <sched.h>
#endif

void QntsHPC::setCpuAffinityMask(quint32 coreMask)
{
// check if there are multiple cores at all
if (QThread::idealThreadCount() > 1)
{
# ifdef Q_OS_WIN
SetThreadAffinityMask(GetCurrentThread(),coreMask) ;
# endif
# ifdef Q_OS_UNIX
//sched_setaffinity(gettid());
int proc_num = (int)(long)coreMask;
cpu_set_t set;
CPU_ZERO( &set );
CPU_SET( proc_num, &set );
sched_setaffinity( syscall(SYS_gettid), sizeof( cpu_set_t ), &set );
# endif
}
}The mask is a bit-vector, see documentation for SetThreadAffinityMask.

You probably will need only a subset of the linux includes for this. But no matter, your target is windows anyway :->

HIH

Johannes

high_flyer
13th April 2011, 11:58
@Marek:
this is not a dead lock, but a blocking call which never returns.
A dead lock is when two or more processes are each waiting for the other to release a resource, or more than two processes are waiting for resources in a circular chain.
In your example, since your thread didn't start, it is not processing the events it got, or events have no where to go.

When you perform a moveToThread on the QThread object, that is all you do - you move a QThread object to the thread it started (if it did start a new thread).
I don't see for what it could be good, but you need to understand the implications.
"shouldn't be done" is an assertion that is worth very little if you don't explain why, or under what conditions.

The only thing I am not sure about in the way your example works, is, that since you didn't call start() on the thread, there should be no "other thread", thus I am not sure what moveToThread() does in a case where the thread didn't start.
That is the only point I am not quite sure how the internals of moveToThead work.
@wysota:
If you are reading this maybe you can answer what happens if you move to a thread that hasn't been started yet?
I don't have Qt here.

Cruz
13th April 2011, 12:10
@Johannes

Thanks, I will try this! Did you evaluate how this technique works on Linux? While I am currently bound to Windows, my long term goal is to write something that works on both, Windows and Linux.

JohannesMunk
13th April 2011, 13:58
I measured the min/max/stddev of QThread::msleep intervals with the performance counter only under windows. I did not run the same tests on linux, because on my dev-machine it runs only virtually. But timing critical network connections between the virtual linux machine and other computers in the network seemed to work with good latencies, so I stopped tweaking.

Be aware, that if you use only msleep and no timers with intervals smaller than 20ms you will have to increase the windows timer resolution for msleep to work correctly.

See: http://www.qtcentre.org/threads/40317-frame-rate-and-QTimer-accuracy?p=185340#post185340

Johannes

wysota
13th April 2011, 15:52
Common witek, so what are you saying?
You should not implement tasks that need time resolutions in ms range in non RTOS?
I would say you should not aim to have precise timing on a system that can't give it to you.


I am sure you, just as I have, made many such usages of threads that are "real time like" under windows/linux.
There is a difference between real-time-like and expectations to have a slot called at exactly 10ms intervals. I'd say such expectations are not required in most cases and there using a regular system is fine. I would like to know why OP wants to have precisely 10ms intervals.

wysota
13th April 2011, 22:50
I'm curious what your approach would be to solve the task as stated above, that a steady high frequency communication (100Hz) has to be maintained while the CPU can be potentially maxed out at times with other tasks.
I would use a real time operating system or rethink my design.


RTOS is not an option, the target OS is Windows.
If it's not an option then just forget it and do something that doesn't require such rigid timing. If you can live with your app working properly "most of the time" or "sometimes" then just go ahead and keep your current approach. Just don't be surprised that if your system decides to do something besides scheduling your thread (say... defragment your harddisk) you will loose your "steady pace". People use real-time operating systems not because they are fun to use but because they are required to reach their goals. On a non-RT OS there is simply no way to force your OS to wake you up at constant intervals. You can use multimedia timers, bind process to a cpu or do anything else you want but this will give you no guarantees. You will be "close to 10ms" "usually" or even "most of the times" but not "always". If you can live with that then go ahead. But if you can live with that then maybe you don't need it after all.

If your robot was performing some kind of surgery or space exploration-thingy I would surely not put my life in its hands if it required time guarantees and you were using regular Windows to obtain it.


Another advantage I see in threads is that often there is more than one core and while one core is struggling with the gui, the other core can still maintain the steady communication frequency. This would not be possible with only one process.
Unless you have more cores than processes running in your system this is completely meaningless in this particular situation.


@wysota:
If you are reading this maybe you can answer what happens if you move to a thread that hasn't been started yet?
I don't have Qt here.

Nothing happens. It (the object in question) can't process events (including slots) until the thread event loop is started. Similarily it can't process events (and slots) after the thread event loop is stopped. Besides this is just bad design. It's like you were driving a car using remote control sitting in the driver's seat of the very same car.

high_flyer
14th April 2011, 09:02
Nothing happens. It (the object in question) can't process events (including slots) until the thread event loop is started.
This is clear, but its not what I asked.
moveToThread() takes a QThread pointer.
The QThread object it self does exist, and its thread affinity is the thread in which it was created.
I would think that moveToThread() "moves" the calling object to the actual thread the QThread object started. (which is what happens if QThread created a thread)
But if no thread has been started, where does moveToThread() move the object to? (as in Marek's examples).
Does it mean that once an object has been 'movedToThread()' that it will "complete" the moving once the thread has been created with start()?

Besides this is just bad design.
Ofcourse, but I still would like to understand it.

wysota
14th April 2011, 09:15
I would think that moveToThread() "moves" the calling object to the actual thread the QThread object started. (which is what happens if QThread created a thread)
But if no thread has been started, where does moveToThread() move the object to? (as in Marek's examples).
In reality what happens is that the object is checked for pending events and if such are found they are taken out of the event loop of the "old" thread and inserted into the event loop of the "new" thread. The event loop exists (it's actually an instance of QAbstractEventDispatcher that is keyed with a pointer to QThread) it's just not running. So the move suceeds.


Does it mean that once an object has been 'movedToThread()' that it will "complete" the moving once the thread has been created with start()?
More likely when you call QThread::exec(). But this doesn't change the fact that after the thread event loop stops and run() returns there is no way to call any slots in the QThread object since it lives in a thread that doesn't process events anymore (so it can't call slots) and you can't push the object to the main thread because there is no worker thread context anymore to invoke the push. Similarily you have no control over what happens before you start the thread event loop.

Cruz
20th April 2011, 16:45
The discussion has drifted off here. The original question why a timer started in a thread appears to run in the main thread. And one of the issues pointed out (the reason for which is still unexplained!) is that a thread shouldn't handle it's own events. (http://labs.qt.nokia.com/2010/06/17/youre-doing-it-wrong/). A clean way to use a timer in a thread was suggested like this:



class TestClass : QObject
{
Q_OBJECT
public:
TestClass(QObject *parent=0);

public slots:
void tick();
};

TestClass::TestClass(QObject *parent) : QObject(parent)
{
QTimer *t = new QTimer(this);
connect(t, SIGNAL(timeout()), this, SLOT(tick()));
}

void TestClass::tick()
{
qDebug() << currentThreadId() << "tick()";
}

// and somewhere
QThread *thread = new QThread(something);
TestClass *object = new TestClass(); // can't pass parent!!!!!!
object->moveToThread(thread);
thread->start();


However, if not a QTimer, but a while and sleep method is used, this code changes to:



class Thread : QThread
{
TestClass testClass;
}

Thread::Thread()
{
testClass.moveToThread(this);
start();
}

Thread::run()
{
forever
{
testClass.tick();
QApplication::processEvents();
}
}

main
{
Thread rhread;
... do mainy things...
}


The problem I have with this construction is that now TestClass is buried inside the QThread wrapper. When signals needs to be emitted from main to TestClass, some kind of a hack needs to be used because TestClass is unknown in main. This is not the case, however, if TestClass IS a QThread and is moved to its own thread. So how can you solve this elegantly without moving the thread to itself?

wysota
20th April 2011, 16:49
You either expose a getter that fetches a pointer to the object in the thread class or you provide a setter to set the object before the thread is executed or you embed the thread object in the TestClass object instead of embedding the TestClass object inside the thread object.

Cruz
20th April 2011, 19:44
...or you embed the thread object in the TestClass object instead of embedding the TestClass object inside the thread object.

But then how does the thread call the tick() method of TestClass?

Cruz
21st April 2011, 14:38
For completeness, like this it works:



class Thread : QThread
{
signals:
void tickOut();
}

Thread::Thread()
{
start();
}

Thread::run()
{
forever
{
emit tickOut();
QApplication::processEvents();
msleep(10);
}
}

class TestClass
{
Thread thread;

public slots:
void tickIn();
}

TestClass::TestClass
{
moveToThread(&thread);
connect(&thread, SIGNAL(tickOut()), this, SLOT(tickIn()));
}



Is this ok? No hack or anything?

wysota
21st April 2011, 19:45
It's not fine. You have a static time delay which means your real delay depends on how long it takes processEvents() to complete its work. The delay should be dynamically adjusted or it should be done using a timer. I don't see how calling sleep would be better than using a timer.

Cruz
21st April 2011, 20:09
The parameter to sleep is adjusted. I omitted it to simplify the code, because what I was asking about is if the construction is ok.
I looked at the steadiness of the QTimer based approach and the dynamically adjusted sleep approach and found the latter to be smoother.

wysota
21st April 2011, 20:57
How did you "look" at it?

Cruz
22nd April 2011, 16:44
I "looked" at it by "plotting" the iteration times produced by the dynamic sleep approach and the QTimer approach. I measured the iteration times using high performance counters on Windows. The following two plots show the different behaviors of the two approaches. Clearly, the QTimer based iterations are much more noisy.
6280 6281

I also "looked" at the effect of starting the thread in time critical instead of normal mode, and what happens if the cpu is stressed. The next plot shows the results of a series of 8 experiments. The 8 experiments are all combinations of dynamic sleep or QTimer, thread was started in time critical mode or not, and if the cpu was stressed or not. Red bars show dynamic sleep, blue bars show QTimers, each with mean and standard deviation "calculated" over 5 minutes runtime. I was aiming at an iteration time of 12 milliseconds. In the cpu stress experiments, both cores of my cpu were tortured by Prime95.
6279

wysota
25th April 2011, 11:29
How did you eliminate the influence of external conditions on the test you performed? Did you also do any stress testing? QTimer will show some variation because the resolution of timers used with it is usually greater than 1ms, especially on Windows. sleep() shouldn't suffer from such problems as it probably relies on capabilities of the scheduler. Also bear in mind both these approaches will fail if something (processEvents) occupies your app for the whole (or almost whole) desired interval or if something prevents the scheduler from running at the exact demanded point in time. You simply can't overcome this without RTOS.