Speed up multiple setData() calls in model

panosk

Hi all,

I have connected a QAction to a slot in my model, which calls setData() for each row in the model. The slot simply copies the contents of one column to another if the other column's cell is empty.

void MyModel::copyAll()
{
    for (auto i=0; i < rowCount(); ++i) {
        QModelIndex sourceIndex = index(i,SourceColumn);
        if (sourceIndex.isValid()) {
            QModelIndex targetIndex = index(i,TargetColumn);
            if (targetIndex.data().toString().isEmpty()) {
                setData(targetIndex,sourceIndex.data(Qt::DisplayRole).toString(),
                        Qt::UserRole);
                //emit(dataChanged(targetIndex,targetIndex) // will call it when all OK
            }
        }
    }
}

Currently this operation is terribly expensive, considering that I fetch all rows from the model with fetchMore(), so even with 700-800 rows it takes several seconds to complete.

Is this a good case for using some parallelism, maybe QtConcurrent? If so, how should I move on with it?

Thanks in advance.

bsomervi

If this operation is internal to your model why not work on the underlying data rather than using the external public API of QAbstractItemModel to manipulate the data. You can use the protected virtual members to inform clients of the model that you have changed the underlying data in the model.

panosk

Thanks for your reply.
The underlying model is a subclassed QSqlRelationalTableModel and until now I didn't have to explicitly use a data structure to work on my data as it is not allowed to change the row or column count of the underlying tables. This forces me to use setData() as the only update mechanism as I can't work directly on a data structure. This will probably change in the near future as there's a possibility to allow adding or deleting rows, but for now it seems I'm stuck with setData().

bsomervi

OK. Are you able to setEditStrategy(QSqlTableModel::OnManualSubmit) on your model? If so that would allow you to maximize the caching done by the model.

SGaist

Hi,

Before going with threading, you should change your check, QVariant has a isNull method that can avoid the string conversion. The same goes for setData, why convert when you can directly copy the QVariant itself ?

After that, if you still need threading, you will have to ensure you are protecting your underlying data structure correctly with a mutex.

Don't forget that emitting dataChanged from another thread is not allowed.

panosk

@SGaist
You are right of course so I made the changes. There is some improvement but not enough. The operation takes around 10 secs for 700-800 rows. I don't mind blocking the UI as long as the operation can execute within 2-3 secs, I just want to speed it up.
The only private members in the model are 2 custom datatypes that are read-only, and a mutable QString and an int that are modified in the setData() function. I open and work on the database in another class.
So, can I wrap the setData() calls in a QtConcurrent::run in the loop, sth like this maybe, and lock a mutex inside the setData()? Just a reminder that the function where the multiple calls occur is a slot that is connected elsewhere to a QAction.

 QFuture<bool> future = QtConcurrent::run(this,&MyModel::setData,
                            targetIndex, sourceIndex.data(Qt::DisplayRole),Qt::UserRole);
              //emit(dataChanged(targetIndex,targetIndex) // I will move this to setData()

Thanks in advance for any hints.

kshegunov

@panosk
Consider using a proxy model, that way you won't need to copy data at all and the functions that are needed for displaying data will be called from Qt. In your current setup you do that not only for the data that's about to be displayed but for the whole data set, which might be quite large.

SGaist

@kshegunov proxy model is a good idea, e.g a QIdentityProxyModel would be adequate. One alternative would be to simply check the if the variant is null and in this case return the value of your other column. That would move the proxy model job in your current model.
The other advantage of the proxy is that you can keep a view on the "real" content of your table and show what you want through the proxy to other views.

panosk

@kshegunov , @SGaist

Thank you both for the suggestion, I will give it a try.

panosk

I spent a little time (not much though...) with QIdentityProxyModel but I can't really see a way to use it. I should probably have mentioned that the column I copy from and the column I copy to are in different tables. Also, I want the copied strings to be readily available to the user for editing.

Anyway, now I'm using a different approach which seems to work pretty fast: I use sql queries inside the copyAll() slot with a transaction. The problem now is that although the database is actually updated, the QTableView is not. I've tried using all possible "refresh" mechanisms inside my copyAll() slot, including dataChanged(), beginResetModel(), layoutChanged() but the view will not update itself.

mjsurette

Have you tried calling select() after all the updates are done? This should fall in the realm of crude but effective.

panosk

@mjsurette

That did the trick! I also removed the dataChanged() calls and now the operation executes instantly even with thousands of rows. It also seems there aren't any side effects, apart that I have to call again fetchMore() immediately after select(). I can now move on and implement some other batch operations like this without worrying about speed.

Thank you!