QThread question

[ObiWanKenobe]

Hi,

I'm experimenting with threads.
My goal is to write a program, which exports data from a DB to XML.
I'm planing to create 2 or 3 worker threads, that does the job.
When a thread is finished with a DB table, a new table must be assigned to the thread.
So I have to know which thread object is finished.

I wrote the following thread class for testing:



#ifndef KCOUNTERTHREAD_H
#define KCOUNTERTHREAD_H

#include <QThread>

class KCounterThread : public QThread
{
Q_OBJECT
public:
explicit KCounterThread(int id, QObject *parent = 0, int maxValue = 100, int waitTime = 100);

void run();

int maxValue() const;
void setMaxValue(int value);
int waitTime() const;
void setWaitTime(int value);
int id() const;

signals:
void counterChanged(int value);
void threadFinished(KCounterThread*);

private:
int m_maxValue;
int m_waitTime;
int m_id;
};

#endif // KCOUNTERTHREAD_H



#include "kcounterthread.h"

KCounterThread::KCounterThread(int id, QObject *parent, int maxValue, int waitTime)
: QThread(parent), m_id(id), m_maxValue(maxValue), m_waitTime(waitTime)
{
}

void KCounterThread::run()
{
for (int n = 0; n < m_maxValue; n++)
{
emit counterChanged(n);
msleep(m_waitTime);
}
emit threadFinished(this);
}

int KCounterThread::maxValue() const
{
return m_maxValue;
}

void KCounterThread::setMaxValue(int value)
{
if (!isRunning())
m_maxValue = value;
}

int KCounterThread::waitTime() const
{
return m_waitTime;
}

void KCounterThread::setWaitTime(int value)
{
if (!isRunning())
m_waitTime = value;
}

int KCounterThread::id() const
{
return m_id;
}


As you can see I emit a self declared signal threadFinished(this) at the end of the run() method.
My test program looks like this:


void MainWindow::on_pushButton2_clicked()
{
counterThread1 = new KCounterThread(1, this, 500, 10);
counterThread2 = new KCounterThread(2, this, 500, 5);

ui->progressBar1->setMaximum(counterThread1->maxValue() - 1);
ui->progressBar2->setMaximum(counterThread2->maxValue() - 1);
ui->progressBar1->setValue(0);
ui->progressBar2->setValue(0);
connect(counterThread1, SIGNAL(counterChanged(int)), this, SLOT(counter1Changed(int)));
connect(counterThread2, SIGNAL(counterChanged(int)), this, SLOT(counter2Changed(int)));
connect(counterThread1, SIGNAL(finished()), this, SLOT(counter1Finished()));
connect(counterThread2, SIGNAL(finished()), this, SLOT(counter2Finished()));
connect(counterThread1, SIGNAL(threadFinished(KCounterThread*)), this, SLOT(counterFinished(KCounterThread*)));
connect(counterThread2, SIGNAL(threadFinished(KCounterThread*)), this, SLOT(counterFinished(KCounterThread*)));
counterThread1->start();
counterThread2->start();
}
void MainWindow::counter1Changed(int value)
{
ui->progressBar1->setValue(value);
}

void MainWindow::counter2Changed(int value)
{
ui->progressBar2->setValue(value);
}

void MainWindow::counter1Finished()
{
ui->plainTextEdit->appendPlainText("Thread 1 finished!!!");
}

void MainWindow::counter2Finished()
{
ui->plainTextEdit->appendPlainText("Thread 2 finished!!!");
}

void MainWindow::counterFinished(KCounterThread *counterThread)
{
ui->plainTextEdit->appendPlainText(QString("Thread %1 finished. (This is my signal)").arg(counterThread->id()));
}
I get this output:

Thread 2 finished. (This is my signal)
Thread 2 finished!!!
Thread 1 finished. (This is my signal)
Thread 1 finished!!!

My question is:
Is this the right way to do this or is there a better way?

Thanks in advance

[Lesiok]

No because with this run() method You don't have an event loop in thread. It should be something like this :

#ifndef KCOUNTERTHREAD_H
#define KCOUNTERTHREAD_H

#include <QThread>

class KCounterThread : public QThread
{
Q_OBJECT
public:
explicit KCounterThread(int id, QObject *parent = 0, int maxValue = 100, int waitTime = 100);

void run();

int maxValue() const;
void setMaxValue(int value);
int waitTime() const;
void setWaitTime(int value);
int id() const;

signals:
void counterChanged(int value);
void threadFinished(KCounterThread*);
void oneStep();//new slot definition
private:
int m_maxValue;
int m_waitTime;
int m_id;
int m_stepNumber;//new variable definition
};

#endif // KCOUNTERTHREAD_H




void KCounterThread::run()
{
m_stepNumber = 0;
QTimer::singleShot(0, this, SLOT(oneStep()));
QThread::run();
}

void KCounterThread::oneStep()
{
if( m_stepNumber++ < m_maxValue )
{
emit counterChanged(n);
QTimer::singleShot(m_waitTime, this, SLOT(oneStep()));
}
else
emit threadFinished(this);
}

[anda_skoa]

No because with this run() method You don't have an event loop in thread.

Why would the thread need an event loop?





void KCounterThread::run()
{
m_stepNumber = 0;
QTimer::singleShot(0, this, SLOT(oneStep()));
QThread::run();
}



Why would one want the thread to fire timer signals but do the process in the main thread? Usually the worker thread is the one that should to the work.



My goal is to write a program, which exports data from a DB to XML.
I'm planing to create 2 or 3 worker threads, that does the job.
When a thread is finished with a DB table, a new table must be assigned to the thread.
So I have to know which thread object is finished.


Sounds like you want to have a look at thread pools. i.e. QThreadPool

Cheers,
_

[stampede]

Isn't it simpler to separate the "thread" and "worker" logic ?

#include <QThread>
#include <QApplication>
#include <QDebug>
#include <QTimer>

class Worker : public QObject{
Q_OBJECT
public:
Worker(int njobs) : m_njobs(njobs){
}
public slots:
void doJob(){
qDebug() << "Working... " << m_njobs << QThread::currentThread();
--m_njobs;
if (m_njobs <= 0){
emit done();
} else{
QTimer::singleShot(0,this,SLOT(doJob()));
}
}
signals:
void done();
private:
int m_njobs;
};

int main(int argc, char ** argv){
QApplication app(argc,argv);
qDebug() << "GUI Thread " << QThread::currentThread();
Worker * worker = new Worker(5);
QThread * thread = new QThread();
worker->moveToThread(thread);
QObject::connect(thread, SIGNAL(started()), worker, SLOT(doJob()));
QObject::connect(worker, SIGNAL(done()), thread, SLOT(quit()));
QObject::connect(thread, SIGNAL(finished()), &app, SLOT(quit()));
thread->start();
int r = app.exec();
delete worker;
delete thread;
return r;
}

#include "moc_main.cpp"




// program output:
[New Thread 3892.0x2bc]
warning: GUI Thread QThread(0x187658a8)

[New Thread 3892.0x1a28]
warning: Working... 5 QThread(0x1876a338)

warning: Working... 4 QThread(0x1876a338)

warning: Working... 3 QThread(0x1876a338)

warning: Working... 2 QThread(0x1876a338)

warning: Working... 1 QThread(0x1876a338)


Program exited normally.

[ObiWanKenobe]

Thank you all for your replies.

@stampede:
Your solution keeps the GUI responsive, but I want to parallelize the table exports.
I have about 30 tables and I want to export 2-3 of them at the same time.
Everytime a thread finises the export a new table will be assigned to it from the list of tables (QStringList).

@anda_skoa:
QThreadPool could be an alternative. I have to study it in more detail.

[stampede]

... but I want to parallelize the table exports
I can't see any problems with this if you disconnect the threading logic and worker logic. Now you have a single object, which is a "worker-in-thread", I just want to point out that its more reusable to have a "worker" and "thread" separately. Imagine a Worker from my example being a single db table exporter. This way you can run all of them in GUI thread, assign separate thread to each Worker, or even push 2-3 Workers to the same thread. In order to reuse Workers, you can catch the "done" signal from the Worker and assign it next table (or set of tables).
This way you have everything nicely structured - if you decide to change the threading model later, you won't need to change the db table export code, because a Worker doesn't care if it works in main or any other thread.

[wysota]

For me QtConcurrent seems the easiest approach to take.


QStringList tables = QStringList() << "tableA" << "tableB" << "tableC" << ...;
QFuture<void> future = QtConcurrent::map(tables, myFuncThatExportsATable);
// ...

One can use QFutureWatcher to be notified about progress of operations using signals.