QTimer / QThread question

[Galen]

Thanks. Well, "VeryBad(TM)" sounds bad, so I guess I'll just emit signals with a QString argument to the gui instead. What I meant was make methods in the main qwdiget to change the contents and lock a mutex at the beginning of it and unlock at the end. That would protect everything inside in a rather crude way, and as long as that is the only method that changes those objects that would be sufficient, right? But then again, this still involves function calls to the gui from another thread. Is this inherently problematic?

I think the slots must be running in the threadwatcher thread. Here's QThread::currentThread() at various points.

main 0x6f51a8
sw 0x6f51a8
sorter 0x6f51a8
tw-r 0x2222e40
tw-h 0x2222e40

Main creates a SortWindow (extends QWidget) and a Sorter (extends QObject) object. The latter has handlers for the buttons and such (I actually tried to make this a thread for awhile but realized this wasn't a great idea). The start button handler launched 6x sort threads and the threadWatcher thread. Tw-r is sampled in run() and Tw-h is in the finishedHandler slot. Pretty conclusive, no?

void ThreadWatcher::run() {
    QTextStream cout(stdout, QIODevice::WriteOnly);
    cout << "tw-r " << QThread::currentThread() << endl;
    QSignalMapper threadMapper;
    updateTimer = new QTimer();
 
    for (int i = 0; i < 6; i++) {
        connect(thread[i], SIGNAL(finished()), &threadMapper, SLOT(map()));
        threadMapper.setMapping(thread[i], i);}
 
    connect(&threadMapper, SIGNAL(mapped(int)), this, SLOT(finishedHandler(int)));
    connect(updateTimer, SIGNAL(timeout()), this, SLOT(timeoutHandler()));
 
    connect(this, SIGNAL(refresh(int)), window, SLOT(draw(int)));
 
    updateTimer->start(timerVal);
 
    QThread::exec();
 
    delete updateTimer;
}
void ThreadWatcher::timeoutHandler() {
 
    for(int i=0; i<6; i++) {
        if (alive[i])
            emit refresh(i); }
 
    if ((threadsLeft==0) || (control[1])) {
        QThread::exit(0); }
 
    else if (control[0]) {
        lock->lock();
        updateTimer->stop();
        condition->wait(lock);
        lock->unlock();
        updateTimer->start(timerVal); }
}
void ThreadWatcher::finishedHandler(int t) {
    QTextStream cout(stdout, QIODevice::WriteOnly);
    cout << "tw-h " << QThread::currentThread() << endl;
    emit refresh(t);
    alive[t] = false;
    threadsLeft--;
 
    seconds[t] = (float)runTime[t] / 1000.0f;
    if (seconds[t] == 0.0f)								
        seconds[t] = 0.001f;
    time[t]->setText("Time: "+QString::number(seconds[t],'g',3));      
    if (isFirst) {
        isFirst = false;
        long minTime = runTime[t];
        min = t;
        long x;
 
        for (int k=0; k<6; k++) {interval)
            x = runTime[k];
            if ((x > -1) && (x < minTime)) {
                minTime = x;
                min = k; } }
 
        if (min != t) {
            seconds[min] = (float)runTime[min] / 1000.0f;
            if (seconds[min] == 0.0f)
                seconds[min] = 0.001f; }
 
                if (min == t)
                    ratio[t]->setText("Ratio: 1x");
                else {
                    ratio[t]->setText("Ratio: "+QString::number(seconds[t]/seconds[min],'g',3)+"x"); } }
 
    else
        ratio[t]->setText("Ratio: "+QString::number(seconds[t]/seconds[min],'g',3)+"x");
}

void ThreadWatcher::run() {
    QTextStream cout(stdout, QIODevice::WriteOnly);
    cout << "tw-r " << QThread::currentThread() << endl;
    QSignalMapper threadMapper;
    updateTimer = new QTimer();

    for (int i = 0; i < 6; i++) {
        connect(thread[i], SIGNAL(finished()), &threadMapper, SLOT(map()));
        threadMapper.setMapping(thread[i], i);}

    connect(&threadMapper, SIGNAL(mapped(int)), this, SLOT(finishedHandler(int)));
    connect(updateTimer, SIGNAL(timeout()), this, SLOT(timeoutHandler()));

    connect(this, SIGNAL(refresh(int)), window, SLOT(draw(int)));

    updateTimer->start(timerVal);

    QThread::exec();

    delete updateTimer;
}
void ThreadWatcher::timeoutHandler() {

    for(int i=0; i<6; i++) {
        if (alive[i])
            emit refresh(i); }

    if ((threadsLeft==0) || (control[1])) {
        QThread::exit(0); }

    else if (control[0]) {
        lock->lock();
        updateTimer->stop();
        condition->wait(lock);
        lock->unlock();
        updateTimer->start(timerVal); }
}
void ThreadWatcher::finishedHandler(int t) {
    QTextStream cout(stdout, QIODevice::WriteOnly);
    cout << "tw-h " << QThread::currentThread() << endl;
    emit refresh(t);
    alive[t] = false;
    threadsLeft--;

    seconds[t] = (float)runTime[t] / 1000.0f;
    if (seconds[t] == 0.0f)								
        seconds[t] = 0.001f;
    time[t]->setText("Time: "+QString::number(seconds[t],'g',3));      
    if (isFirst) {
        isFirst = false;
        long minTime = runTime[t];
        min = t;
        long x;

        for (int k=0; k<6; k++) {interval)
            x = runTime[k];
            if ((x > -1) && (x < minTime)) {
                minTime = x;
                min = k; } }

        if (min != t) {
            seconds[min] = (float)runTime[min] / 1000.0f;
            if (seconds[min] == 0.0f)
                seconds[min] = 0.001f; }

                if (min == t)
                    ratio[t]->setText("Ratio: 1x");
                else {
                    ratio[t]->setText("Ratio: "+QString::number(seconds[t]/seconds[min],'g',3)+"x"); } }

    else
        ratio[t]->setText("Ratio: "+QString::number(seconds[t]/seconds[min],'g',3)+"x");
}

To copy to clipboard, switch view to plain text mode 

[wysota]

What I meant was make methods in the main qwdiget to change the contents and lock a mutex at the beginning of it and unlock at the end. That would protect everything inside in a rather crude way, and as long as that is the only method that changes those objects that would be sufficient, right?

Unfortunately not. This protects code, not the shared resource. Qt will still access the widget from within other methods "behind your back" and you can't protect that.

But then again, this still involves function calls to the gui from another thread. Is this inherently problematic?

Yes, GUI resources just can't be protected from within application code.

I think the slots must be running in the threadwatcher thread. Here's QThread::currentThread() at various points.

The thread-watcher thread is redundant - all it does is to monitor other threads so it can be done from the main thread.

Take a look at QtConcurrent and especially QFutureWatcher, by the way.

[Galen]

Well, true, but its my redundant thread and I love it. Plus I want to use emit in this code somewhere. So now I've stripped out all the function calls. The emit signal system is really pretty much the same thing so no biggie. Everything seems to be running pretty well now, the inline function at the bottom fixes a problem whereby threads with longer running time somehow end up signalling first.

Thanks, I'll take a look at those classes, but this isn't even a real app so I probably won't be switching over even if it would run better. For this code sample I'm pretty much assuming an unlimited number of cores.

void ThreadWatcher::run() {
    QSignalMapper threadMapper;
    updateTimer = new QTimer();
 
    for (int i = 0; i < 6; i++) {
        connect(thread[i], SIGNAL(finished()), &threadMapper, SLOT(map()));
        threadMapper.setMapping(thread[i], i); }
 
    connect(&threadMapper, SIGNAL(mapped(int)), this, SLOT(finishedHandler(int)));
    connect(updateTimer, SIGNAL(timeout()), this, SLOT(timeoutHandler()));
    connect(this, SIGNAL(refresh(int)), window, SLOT(draw(int)));
    connect(this, SIGNAL(time(int, QString)), window, SLOT(setTimeLabel(int, QString)));
    connect(this, SIGNAL(ratio(int, QString)), window, SLOT(setRatioLabel(int, QString)));
 
    updateTimer->start(timerVal);
 
    QThread::exec();
 
    delete updateTimer;
}
void ThreadWatcher::timeoutHandler() {
 
    for(int i=0; i<6; i++) {
        if (alive[i])
            emit refresh(i); }
 
    if ((threadsLeft==0) || (control[1])) {
        QThread::exit(0); }
 
    else if (control[0]) {
        updateTimer->stop();
        lock->lock();
        condition->wait(lock);
        lock->unlock();
        updateTimer->start(timerVal); }
}
void ThreadWatcher::finishedHandler(int t) {
    emit refresh(t);
    alive[t] = false;
    threadsLeft--;
 
    seconds[t] = (float)runTime[t] / 1000.0f; 
    if (seconds[t] == 0.0f)	
        seconds[t] = 0.001f;
    emit time(t, "Time: "+QString::number(seconds[t],'g',3)); 
 
    if (isFirst) {
        isFirst = false;
        min = t;
        emit ratio(t, "Ratio: 1x"); }
    else {
        float timeRatio = seconds[t]/seconds[min];
        if (timeRatio < 1.0f)
            fixRatios(t);
        else
            emit ratio(t, "Ratio: "+QString::number(timeRatio,'g',3)+"x"); }
}
inline void ThreadWatcher::fixRatios(int t) {
    long minTime = runTime[min];
    long x;
 
    for (int i=0; i<6; i++) {
        if (!alive[i]) {
           x = runTime[i];
           if ((x > -1) && (x < minTime)) {
               minTime = x;
               min = i; } } }
 
    for (int j=0; j<6; j++) {
        if (!alive[j]) {
            if (j==min)
                emit ratio(j, "Ratio: 1x");
            else
                emit ratio(j, "Ratio: "+QString::number(seconds[j]/seconds[min],'g',3)+"x");  } }
}

void ThreadWatcher::run() {
    QSignalMapper threadMapper;
    updateTimer = new QTimer();

    for (int i = 0; i < 6; i++) {
        connect(thread[i], SIGNAL(finished()), &threadMapper, SLOT(map()));
        threadMapper.setMapping(thread[i], i); }

    connect(&threadMapper, SIGNAL(mapped(int)), this, SLOT(finishedHandler(int)));
    connect(updateTimer, SIGNAL(timeout()), this, SLOT(timeoutHandler()));
    connect(this, SIGNAL(refresh(int)), window, SLOT(draw(int)));
    connect(this, SIGNAL(time(int, QString)), window, SLOT(setTimeLabel(int, QString)));
    connect(this, SIGNAL(ratio(int, QString)), window, SLOT(setRatioLabel(int, QString)));

    updateTimer->start(timerVal);

    QThread::exec();

    delete updateTimer;
}
void ThreadWatcher::timeoutHandler() {

    for(int i=0; i<6; i++) {
        if (alive[i])
            emit refresh(i); }

    if ((threadsLeft==0) || (control[1])) {
        QThread::exit(0); }

    else if (control[0]) {
        updateTimer->stop();
        lock->lock();
        condition->wait(lock);
        lock->unlock();
        updateTimer->start(timerVal); }
}
void ThreadWatcher::finishedHandler(int t) {
    emit refresh(t);
    alive[t] = false;
    threadsLeft--;

    seconds[t] = (float)runTime[t] / 1000.0f; 
    if (seconds[t] == 0.0f)	
        seconds[t] = 0.001f;
    emit time(t, "Time: "+QString::number(seconds[t],'g',3)); 

    if (isFirst) {
        isFirst = false;
        min = t;
        emit ratio(t, "Ratio: 1x"); }
    else {
        float timeRatio = seconds[t]/seconds[min];
        if (timeRatio < 1.0f)
            fixRatios(t);
        else
            emit ratio(t, "Ratio: "+QString::number(timeRatio,'g',3)+"x"); }
}
inline void ThreadWatcher::fixRatios(int t) {
    long minTime = runTime[min];
    long x;

    for (int i=0; i<6; i++) {
        if (!alive[i]) {
           x = runTime[i];
           if ((x > -1) && (x < minTime)) {
               minTime = x;
               min = i; } } }

    for (int j=0; j<6; j++) {
        if (!alive[j]) {
            if (j==min)
                emit ratio(j, "Ratio: 1x");
            else
                emit ratio(j, "Ratio: "+QString::number(seconds[j]/seconds[min],'g',3)+"x");  } }
}

To copy to clipboard, switch view to plain text mode