Avoid "Not Responding" when adding rows to QTableWidget

[JonB]

@hbatalha
If you are now committed to a QSqL... model, you might test out responsiveness if you attached a QTableView directly to it instead. QSqlTableModel also has support for various EditStrategys and submitting,

[hbatalha]

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

[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.