QThread signals|calls to QChart are blocking the GUI
-
Hello everybody,
My scenario is the following:
I'm currently working on a simulator project written in C++ with a Qt window as its GUI. Within the Main window there is a QGLWidget and a Dock which holds several custom QChartViews. Each eChart (Custom QChart based on Qt examples) represents 1 to n QValueSeries created from double lists from the simulation.
The axis would be x: iterations, y: selectedVariable_Value (eg Fitness or Age)
The problem arises when handling the lists which fill each chart. Since I have implemented it with a for loop, as the simulation goes forward, the task gets more and more computational expensive to achieve. My GUI totally freezes when adding big ammounts of robot variables when the iteration number is big (>500).
Implemented a QThread worker, in a similar way to the mandelbrot example, so each ViewerChart has a thread which makes the heavy calculations. I tried three ways so far:
- Implementing a worker which does all the hard work and sleeps/awakes like the example in mandelbrot(showed below).
- Originally the worker would call addPoint from the *chart directly stored on itself, but apparently that's a very bad approach.
- Created a thread event Loop refactoring the run() method, runing the initLoop and calling exec() at the end , but It wouldn't add any point to the chart.
Also tried to reduce the signals emited to the analytics module and in general. None of this seemed to improve the situation.
Please, take a look to my code:
@QThread.h
class GThread: public QThread{ Q_OBJECT public: GThread(QObject *parent = 0); ~GThread(); void threadUpdate(float x); // changes x to current_iteration and awakes the thread void initiate(params to initialize variables); signals: void addpoints(float x, float y); protected: void run() override; // handles work|sleep behaviour like in mandelbrot void iniLoop(); // fills the chart with all the values in the list void g_Step(); //fills the chart with the last iteration of the list std::vector<double> retOrdRoboStats(int n); //orders values to show by greater|lesser "mod" private: QMutex mutex; QWaitCondition condition; eChart * chart; //initially held the chart to be called (bad programming to do GUI things in other thread) std::vector<roboStat>* lista; //holds a list of robots: {std::vector<double> vector, string id} int cant, state; //list modifier & state of thread(0 init,1 step) std::string mod; // list modifier bool restart; //restart bool aborta; //"abort" seemed to be reserved };
@GThread.cpp
GThread::GThread(QObject *parent) : QThread(parent) { restart = false; aborta = false; } GThread::~GThread(){ mutex.lock(); aborta = true; condition.wakeOne(); mutex.unlock(); wait(); //using quit() in the third approach to close event loop } void GThread::initiate(params){ QmutexLocker locker(&mutex); //filling Internal Variables state = 0; if (!isRunning()){ start(NormalPriority); }else{ restart = true; condition.wakeOne(); } } void GThread::iniLoop(){ //////////////Heavy part//////////////// //for loop from i=0 to lista[robot1]->vectorSize // sort "cant" (modif 1) robots if needed //inner for loop from 0 to [1, nRobots] ) ////emit signal or call char->addPoint(i,value)////<-- main problem here ////////////////////////////////// [...] state = 1; restart = false; } void GThread::threadUpdate(float x){ QMutexLocker locker(&mutex); // mutex.lock(); it = x; // mutex.unlock(); condition.wakeOne(); } void GThread::g_Step(){ // just like initLoop with only the inner loop (just last item added to the list is pushed to chart) } void GThread::run(){ forever{ bool done = mutex.tryLock(2); // lock() usually get deathlock here... not sure why // qDebug() << "Hello" << "RUN THREAD" << "from" << QThread::currentThread(); if(state == 0) iniLoop(); else g_Step(); if (aborta) return; // exit the thread for its destruction condition.wait(&mutex); //waits until new update signal // exec(); //creating thread event loop }
I'm connecting (by using queued connections?) the gthread to the chart right before I call gthread.initiate(&list,modifiers) and the thread is notified when is needed to update the chart (new iteration) through a connect method right after that :
void MainWidget::createDialog(){ //create dialog and exec it //if QDialog::Accepted do the following; ViewerChart_instance.changeChart(params) connect(this,SIGNAL(chartUpdate(float)), ViewerChart_Instance, SLOT(update(float))); //ViewerChart.update(float) calls gthread.update(float) each new iteration. Didn't feel like using slots in the gthread from the main thread was a good thing. } void ViewerChart::changeChart(params){ //create chart connect(gthread, SIGNAL(addpoints(float,float)), chart, SLOT(addPoint(float, float)); gthread.initiate(list & modifiers); } //finally, addpoint is as easy as: void eChart::addPoint(float, float){ //some O(1) axis-range stuff *(c_seriesIterator)-> append(both floats); c_seriesIterator++; // if iterator is at the end then iterator = beginning }
I have a feeling that I must be overloading and blocking the event loop, after reading some posts like this and this, but I have no clue about where exactly is the problem or how to address it properly... Apparently what I'm currently doing shouldn't be a bad approach.
I'm I missing the way to properly initiate a event loop in the thread and send the addpoint signal to the main thread event loop? Or ... Could it be that I'm using way too many signals?
I'm mostly certain that the solution of this is related with the problem in the update part, which makes the simulation go slower and slower as the charts contain more and more data.
I've being struggling so hard with this, I'd appreciate any help I can get.
Thanks for reading this far :-)
-
After so much time lost, solved it!!
It seemed to be the fault of the default QSeries implementation... Series.append() apparently emit a couple of signals and resize the inner vector which contain the QPoints. This combined to a O(kn²) loop totally messes up the main event loop.
It was "as easy" as refractoring the code so the thread builds up a QVector of QPoints, then signals each vector to the chart and it calls to series.replace() instead of append(), that is actually faster and lighter.
I was about to give up after trying the moveToThread way without success, but I was lucky enought to get the solution after finding this.
So, not a matter of Qthreads after all.. Now I'm wondering whether refracting the code to create a dedicated event-loop was worth it or not.
-
Ok, I think I may have left a question really difficult to digest, lets see if I can put it easier to understand and read:
I have a GUI blocking operation which I'm trying to run subclasing QThread called from a run{ forever_loop{ compute and sleep until signal} method. Pretty much like the Mandelbrot example.
On the chart_helper_thread I need to iterate 1 to n growing lists each time I "initiate" the chart and then It will be receiving signals, to add the last values from the lists, from the main thread each time the app does an iteration (by a startTimer(30) ).
I saw on several posts that the trouble here is usually using moveToThread to moving the worker to a thread, where it runs with its own event loop and so. But I have not used that way yet.
From what I have read so far and what I tried I understand:
1- I shouldn't ever call GUI methods directly from a different thread than the GUI one.
2- I specifically need to use a thread event loop, since I'm probably blocking the main event-loop.
3- By subclassing the thread I am just posting events and signals to the main event-loop, I need to run exec() from the thread to avoid that.
4- Subclassing Qthread might not be the best if I need an event-loop within the thread? Anyway I'm just following the mandelbrot example...
5- The signals made before running the thread (by queue connection) are enough to let the event-loops to communicate to each other.
6- The forever loop approach will overload the event loop as well, using timer instead wont be that different to what I'm
currently doing, and using exec() wont let the signals to reach the slot in the main thread.To let things even clearer, my aim is to have a thread_helper waiting for the QChartViewer to calculate which points must be added to a chart, so it sends a signal to the chart without blocking the GUI. That's what I'm trying to do.
No idea at all why this isn't working properly though. Any ideas?
-
After so much time lost, solved it!!
It seemed to be the fault of the default QSeries implementation... Series.append() apparently emit a couple of signals and resize the inner vector which contain the QPoints. This combined to a O(kn²) loop totally messes up the main event loop.
It was "as easy" as refractoring the code so the thread builds up a QVector of QPoints, then signals each vector to the chart and it calls to series.replace() instead of append(), that is actually faster and lighter.
I was about to give up after trying the moveToThread way without success, but I was lucky enought to get the solution after finding this.
So, not a matter of Qthreads after all.. Now I'm wondering whether refracting the code to create a dedicated event-loop was worth it or not.
-
Hi @Md0ngo said in QThread signals|calls to QChart are blocking the GUI:
After so much time lost, solved it!!
It seemed to be the fault of the default QSeries implementation... Series.append() apparently emit a couple of signals and resize the inner vector which contain the QPoints. This combined to a O(kn²) loop totally messes up the main event loop.
It was "as easy" as refractoring the code so the thread builds up a QVector of QPoints, then signals each vector to the chart and it calls to series.replace() instead of append(), that is actually faster and lighter.
great that you were able to fix it yourself, and I learned something new and important about QCharts thank you ;-)
I was about to give up after trying the moveToThread way without success, but I was lucky enough to get the solution after finding this.
So, not a matter of Qthreads after all.. Now I'm wondering whether refracting the code to create a dedicated event-loop was worth it or not.
If an actual worker class is now replacing your infinite while loop, that you posted in the op, than yes it is, definitely!
Don't forget to set your topic to solved ;-)