[Development] updating many QPersistentModelIndices

[Milian Wolff]

On Wednesday 03 June 2015 18:32:46 Milian Wolff wrote:
> Hey all,
> 
> for a customer I looked into the performance of a model/view application
> with many (and I mean, many!) add/remove/modify operations on a model with
> a QSortFilterProxyModel on top. The obvious solution to speed things up is
> batching, which works nicely paired with layout{AboutToBe,}Changed. So far
> so good, but the issue that was now reported comes when one now *also*
> wants to enable MultiSelection in the view, and the user "accidentally"
> presses CTRL + A to select all items in the view. This then completely
> kills the performance, due to bottlenecks inside QAbstractItemModel, all
> related to QPersistentModelIndex.
> 
> Some questions now from my side:

I forgot to mention: This targets Qt 4.7, but afaik all my findings below are 
equally valid to current Qt 5 code.

> a) Why is the hash of indexes in QAbstractItemModelPrivate::Persistent not
> unique - i.e. why is insertMulti required? I ask, b/c the "it + 1" operation
> in QAbstractItemModelPrivate::Persistent::insertMultiAtEnd is _extremely_
> slow for large lists of persistent model indices, due to the cache misses
> etc. pp. involved. The documentation says:
> 
>     "There should be only one instance QPersistentModelIndexData per index,
> but in some intermediate state there may be severals of PersistantModelIndex
> pointing to the same index, ..."
> 
> What intermediate state is that? When I look at the uses of this function,
> they always use the pattern
> 
> persistent.indexes.erase(...);
> ...
> persistent.insertMultiAtEnd(...);
> 
> I fail to see how/where this intermediate state can occur.

I just recompiled Qt with this patch applied:

diff --git a/src/corelib/kernel/qabstractitemmodel.cpp 
b/src/corelib/kernel/qabstractitemmodel.cpp
index 1eb709c..ec7006b 100644
--- a/src/corelib/kernel/qabstractitemmodel.cpp
+++ b/src/corelib/kernel/qabstractitemmodel.cpp
@@ -3464,8 +3464,10 @@ bool QAbstractListModel::dropMimeData(const QMimeData 
*data, Qt::DropAction acti
  */
 void QAbstractItemModelPrivate::Persistent::insertMultiAtEnd(const 
QModelIndex& key, QPersistentModelIndexData *data)
 {
+    Q_ASSERT(indexes.count(key) < 2);
     QHash<QModelIndex,QPersistentModelIndexData *>::iterator newIt =
-            indexes.insertMulti(key, data);
+            indexes.insert(key, data);
+    return;
     QHash<QModelIndex,QPersistentModelIndexData *>::iterator it = newIt + 1;
     while (it != indexes.end() && it.key() == key) {
         qSwap(*newIt,*it);

with a developer build, and assertions enabled, everything still works as 
expected (just much faster). I ran the following unit tests, all still pass, 
or behave just as without this patch: 

qabstractitemmodel
qabstractproxymodel
qsortfilterproxymodel
qtreeview
qlistview

My own test app also works without asserting. So again - why the multiHash - 
obsolete? If so, I'll push the required changes to Qt 4 and Qt 5 to remove 
QAbstractItemModelPrivate::Persistent::insertMultiAtEnd.

Bye
-- 
Milian Wolff | milian.wolff at kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts