Avoid "Not Responding" when adding rows to QTableWidget

JonB

@hbatalha Yup that's a QProgressDialog.

hbatalha

@JonB yeah when you mentioned it earlier I remembered seeing it in the Shotcut.

fcarney

Maybe I am reading this wrong, but wouldn't it be appropriate to do this in batches?
Each batch can be called by a zero timer that processes a few at a time. It in turn calls a zero time timer to get the next batch processed on the next event loop cycle. So that app stays responsive and you don't have threads.

In this what longuitask does?

jeremy_k

@hbatalha said in Avoid "Not Responding" when adding rows to QTableWidget:

@JonB I found a nice solution in an open source project Shotcut
longuitask.h
longuitask.cpp

That doesn't look like a viable solution for this task. QWidget-derived classes should only be accessed from QApplication's thread. QtConcurrent::run will violate that requirement.

JonB

@jeremy_k
I didn't even look in the .h file, just the .cpp. No mention there of threads! :)

hbatalha

@fcarney said in Avoid "Not Responding" when adding rows to QTableWidget:

In this what longuitask does?

No, longuitask just avoids the app unresponsiveness.

Each batch can be called by a zero timer that processes a few at a time. It in turn calls a zero time timer to get the next batch processed on the next event loop cycle.

From what I am picturing this would be the same as longuitask regarding app responsiveness since longuitask successfully avoids the "Not Responding" problem mentioned in OP.

But if you think it's different could you provide a simple code sketch that demonstrates your idea.

hbatalha

@jeremy_k said in Avoid "Not Responding" when adding rows to QTableWidget:

That doesn't look like a viable solution for this task. QWidget-derived classes should only be accessed from QApplication's thread. QtConcurrent::run will violate that requirement.

Using QtConcurrent::run was just an idea, a likely last resort solution in case none better was found.

hbatalha

@JonB Actually in '.h' file there's QtConcurrent. I just noticed it because I was focusing in the '.cpp'.
.h

#ifndef LONGUITASK_H
#define LONGUITASK_H

#include <QFuture>
#include <QProgressDialog>
#include <QtConcurrent/QtConcurrent>

class LongUiTask : public QProgressDialog
{
public:
    explicit LongUiTask(QString title);
    ~LongUiTask();

    template <class Ret>
    Ret wait(QString text, const QFuture<Ret>& future)
    {
        setLabelText(text);
        setRange(0, 0);
        while (!future.isFinished()) {
            setValue(0);
            QCoreApplication::processEvents();
            QThread::msleep(100);
        }
        return future.result();
    }

    template <class Ret, class Func, class Arg>
    Ret runAsync(QString text, Func&& f, Arg&& arg)
    {
        QFuture<Ret> future = QtConcurrent::run(f, arg);
        return wait<Ret>(text, future);
    }

    void reportProgress(QString text, int value, int max);
    static void cancel();
};

#endif // LONGUITASK_H

But I don't really understand how the wait and runAsync play a role in reporting progress.

jeremy_k

@JonB said in Avoid "Not Responding" when adding rows to QTableWidget:

@jeremy_k
I didn't even look in the .h file, just the .cpp. No mention there of threads! :)

I usually take the opposite approach, and look at the header for structure first.

In this case the .cpp doesn't mention threads, but also doesn't have anything that would help with a long running function call.

jeremy_k

@hbatalha said in Avoid "Not Responding" when adding rows to QTableWidget:

@jeremy_k said in Avoid "Not Responding" when adding rows to QTableWidget:

That doesn't look like a viable solution for this task. QWidget-derived classes should only be accessed from QApplication's thread. QtConcurrent::run will violate that requirement.

Using QtConcurrent::run was just an idea, a likely last resort solution in case none better was found.

I misread the problem description.

Using QtConcurrent to add rows to a database sounds fine, as long as it doesn't read the rows from the QTableWidget. It may be necessary to serialize the QtConcurrent tasks.

hbatalha

@jeremy_k said in Avoid "Not Responding" when adding rows to QTableWidget:

...but also doesn't have anything that would help with a long running function call.

It actually helps as it just solved my problem described in the OP.

    LongUiTask longTask(tr("Adding downloads"));
    for(int i = 0, len = titlesList.size(); i < len; ++i)
    {
        longTask.reportProgress(tr("Adding"), i, len);

// adding rows to the QTableWidget
    }

If I remove it the app becomes unresponsive starting from a certain number of rows.

hbatalha

@jeremy_k said in Avoid "Not Responding" when adding rows to QTableWidget:

@hbatalha said in Avoid "Not Responding" when adding rows to QTableWidget:

@jeremy_k said in Avoid "Not Responding" when adding rows to QTableWidget:

That doesn't look like a viable solution for this task. QWidget-derived classes should only be accessed from QApplication's thread. QtConcurrent::run will violate that requirement.

Using QtConcurrent::run was just an idea, a likely last resort solution in case none better was found.

I misread the problem description.

Using QtConcurrent to add rows to a database sounds fine, as long as it doesn't read the rows from the QTableWidget. It may be necessary to serialize the QtConcurrent tasks.

Probably but I will be using longuitask for now because aside from adding rows to database I still have two operations when adding rows to QTableWidget that I still haven't found a solution:
Add widget right aligned to a QTableWidget cell and
Add QPushButton to QTableWidget cell without using QTableWidget::setCellWidget

Edit: these two operations are very slow.

jeremy_k

@hbatalha said in Avoid "Not Responding" when adding rows to QTableWidget:

@jeremy_k said in Avoid "Not Responding" when adding rows to QTableWidget:

...but also doesn't have anything that would help with a long running function call.

It actually helps as it just solved my problem described in the OP.
    for(int i = 0, len = titlesList.size(); i < len; ++i)
    {
        longTask.reportProgress(tr("Adding"), i, len);

If I remove it the app becomes unresponsive starting from a certain number of rows.

LongUiTask::reportProgress() spins the event loop via QCoreApplication::processEvents(). It is making your app responsive because the function that it is called within doesn't return to the event loop quickly. It won't do anything for individual function calls within the loop. The preferred solution is to not do this, and instead return to the event loop.

If a large volume of records need to be processed, @fcarney's suggestion of using a zero timer works well.

hbatalha

@jeremy_k said in Avoid "Not Responding" when adding rows to QTableWidget:

If a large volume of records need to be processed, @fcarney's suggestion of using a zero timer works well.

To certify that I understood @fcarney solution proposal, say that I have 1000 records to process, I will do it in say 50 records batches, right?

The preferred solution is to not do this, and instead return to the event loop.

Could you provide a simple code example demonstrating that, I don't understand the part about returning the event loop. I thought I had it but when trying to actually code it I just got stuck.

In the doc QCoreApplication::processEvents() is discouraged and instead it's advised to use another thread for long operations. I could do that once I find the solutions for the two problem mentioned above.

Christian Ehrlicher

Use a proper model instead the convenience QTableWidget.

hbatalha

@Christian-Ehrlicher said in Avoid "Not Responding" when adding rows to QTableWidget:

Use a proper model instead the convenience QTableWidget.

How would that solve the problem described in the OP?

jeremy_k

@hbatalha said in Avoid "Not Responding" when adding rows to QTableWidget:

@jeremy_k said in Avoid "Not Responding" when adding rows to QTableWidget:

If a large volume of records need to be processed, @fcarney's suggestion of using a zero timer works well.

To certify that I understood @fcarney solution proposal, say that I have 1000 records to process, I will do it in say 50 records batches, right?

The preferred solution is to not do this, and instead return to the event loop.

Could you provide a simple code example demonstrating that, I don't understand the part about returning the event loop. I thought I had it but when trying to actually code it I just got stuck.

void batchExecute(QList<BatchTask> tasks)
{
    if (!tasks.isEmpty())
        tasks.takeFirst().process();
    if (!tasks.isEmpty()) {
        QTimer::singleShot(0, [tasks]() { batchExecute(tasks); });
    }
}

hbatalha

@jeremy_k said in Avoid "Not Responding" when adding rows to QTableWidget:

void batchExecute(QList<BatchTask> tasks)
{
    if (!tasks.isEmpty())
        tasks.takeFirst().process();
    if (!tasks.isEmpty()) {
        QTimer::singleShot(0, [tasks]() { batchExecute(tasks); });
    }
}

Thanks. I just tried it and even though it solves the app responsiveness problem it makes the app behave weirdly by kinda pausing it after each batch execution. Showing imo will give a better UX.

JonB

@hbatalha
I wrote originally:

You might use a QTimer to run the insertions in "batches"of 20 or whatever instead.

Here you might make your "task" executed each time round do a number of inserts at a time, if it is presently doing just one. Or fiddle with the size from 1 upward.

Having said that. I say again: if you are finding inserting these rows is causing "Not Responding", or "lumpiness", on what is a relatively small number I would investigate whether your QTableWidget architecture is causing the overhead. Could you perhaps try out QTableView + QSqlTableModel by temporarily hacking something together to see whether it is then worth changing over your real code?

hbatalha

@JonB said in Avoid "Not Responding" when adding rows to QTableWidget:

if you are finding inserting these rows is causing "Not Responding", or "lumpiness", on what is a relatively small number

No, inserting row is not problem, the problem is what occurs after each row is inserted, in my case it is adding data to database and adding widgets to the table cells using setIndexWidget and setCellWidget, they are what is causing the "Not Responding"
Below is the exact structure of my code:

for(int i = 0, len = titlesList.size(); i < len; ++i) // len == 200 starts causing some lumpiness (at leat leat in my pc)
{
    const int row = ui->table->rowCount();

    ui->table->setRowCount(row + 1);

    QPushButton* button1= new QPushButton(this);
    QPushButton* button2= new QPushButton(this);
    QPushButton* button3= new QPushButton(this);

    ui->table->setItem(row, 0, new QTableWidgetItem(title));
    ui->table->setItem(row, 1, new QTableWidgetItem("-"));
    ui->table->setItem(row, 2, new QTableWidgetItem("-"));
    ui->table->setItem(row, 3, new QTableWidgetItem("-"));
    ui->table->setItem(row, 4, new QTableWidgetItem("-"));
    ui->table->setCellWidget(row, 5, button1);// causes "Not Responding"
    ui->table->setCellWidget(row, 6, button2);// causes "Not Responding"


    for(int k = 0; k < 5; ++k)
    {
        QTableWidgetItem* item = ui->table->item(row, k);
        item->setFlags(item->flags() & ~ Qt::ItemIsEditable);

        if(k != 0)
        {
            item->setTextAlignment(Qt::AlignmentFlag::AlignCenter);
        }
    }

    QWidget* widget = new QWidget;

    button3->setFixedSize(10, 10);

    QHBoxLayout* hLayout = new QHBoxLayout;
    hLayout->addStretch();
    hLayout->addWidget(button3);
    widget->setLayout(hLayout);

    ui->table->setIndexWidget(ui->table->model()->index(row, 0), widget);// causes "Not Responding"

    Record* record = new Record(title, destinationFolder, format,
                                  qualityText, mRecordType);
    record->button1(button1);
    record->button2(button2);

    allRecords.append(record);
    activeRecords.append(record);
    database.add(record); // causes "Not Responding"
}

This is how my code works, I don't see how using QTableView + QSqlTableModel could solve the "Not Responding" issue. I never used QSqlTableModel before so I might be missing something.

As I said before it's pretty unlikely that there will be 200 insertions at once, but this number is totally up to the user so I just want the app to be prepared for such moments.