Wrap QModelIndex in QSortFilterProxyModel for hash performance

The qHash implementation for QModelIndex is not very good resulting in
hundreds of collisions in the hash table. This means most lookups in the
table essentially becomes a linear search. This was 'generically' fixed
for Qt 7, but couldn't be fixed for 6 because of the qHash
implementation is inline and would result in different hash values for
the same QModelIndex in different translation units.

To work around this for this particular instance, we wrap the
QModelIndex in a struct that has the fixed qHash implementation.

Fixes: QTBUG-130309
Pick-to: 6.5
Change-Id: I586fb10cadf73900f3d644f421c37b381224a419
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit 1558811a8485f6dcc51a50a2bba0846091ca8bf6)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Mårten Nordheim 2024-10-22 16:36:45 +02:00 committed by Qt Cherry-pick Bot
parent 1ca3e4d04f
commit 19899d88eb
3 changed files with 29 additions and 9 deletions

View File

@ -34,7 +34,7 @@ QPersistentModelIndexData *QPersistentModelIndexData::create(const QModelIndex &
Q_ASSERT(index.isValid()); // we will _never_ insert an invalid index in the list
QPersistentModelIndexData *d = nullptr;
QAbstractItemModel *model = const_cast<QAbstractItemModel *>(index.model());
QMultiHash<QModelIndex, QPersistentModelIndexData *> &indexes = model->d_func()->persistent.indexes;
QMultiHash<QtPrivate::QModelIndexWrapper, QPersistentModelIndexData *> &indexes = model->d_func()->persistent.indexes;
const auto it = indexes.constFind(index);
if (it != indexes.cend()) {
d = (*it);

View File

@ -37,6 +37,26 @@ public:
static void destroy(QPersistentModelIndexData *data);
};
namespace QtPrivate {
// This class is just a wrapper so we can use the fixed qHash() function for QModelIndex.
struct QModelIndexWrapper // ### Qt 7: Remove again, use QModelIndex directly.
{
QModelIndex index;
Q_IMPLICIT QModelIndexWrapper(const QModelIndex &i) : index(i) { }
Q_IMPLICIT inline operator QModelIndex() const { return index; }
friend bool operator==(const QModelIndexWrapper &l, const QModelIndexWrapper &r) { return l.index == r.index; }
friend bool operator!=(const QModelIndexWrapper &l, const QModelIndexWrapper &r) { return !(operator==(l,r)); }
friend bool operator==(const QModelIndexWrapper &l, const QModelIndex &r) { return l.index == r; }
friend bool operator!=(const QModelIndexWrapper &l, const QModelIndex &r) { return !(operator==(l.index,r)); }
friend bool operator==(const QModelIndex &l, const QModelIndexWrapper &r) { return l == r.index; }
friend bool operator!=(const QModelIndex &l, const QModelIndexWrapper &r) { return !(operator==(l,r.index)); }
friend inline size_t qHash(const QtPrivate::QModelIndexWrapper &index, size_t seed = 0) noexcept
{
return qHashMulti(seed, index.index.row(), index.index.column(), index.index.internalId());
}
};
}
class Q_CORE_EXPORT QAbstractItemModelPrivate : public QObjectPrivate
{
Q_DECLARE_PUBLIC(QAbstractItemModel)
@ -111,7 +131,7 @@ public:
struct Persistent {
Persistent() {}
QMultiHash<QModelIndex, QPersistentModelIndexData *> indexes;
QMultiHash<QtPrivate::QModelIndexWrapper, QPersistentModelIndexData *> indexes;
QStack<QList<QPersistentModelIndexData *>> moved;
QStack<QList<QPersistentModelIndexData *>> invalidated;
void insertMultiAtEnd(const QModelIndex& key, QPersistentModelIndexData *data);

View File

@ -126,7 +126,7 @@ public:
QModelIndex source_parent;
};
mutable QHash<QModelIndex, Mapping*> source_index_mapping;
mutable QHash<QtPrivate::QModelIndexWrapper, Mapping*> source_index_mapping;
void setSortCaseSensitivityForwarder(Qt::CaseSensitivity cs)
{
@ -239,9 +239,9 @@ public:
std::array<QMetaObject::Connection, 18> sourceConnections;
QHash<QModelIndex, Mapping *>::const_iterator create_mapping(
QHash<QtPrivate::QModelIndexWrapper, Mapping *>::const_iterator create_mapping(
const QModelIndex &source_parent) const;
QHash<QModelIndex, Mapping *>::const_iterator create_mapping_recursive(
QHash<QtPrivate::QModelIndexWrapper, Mapping *>::const_iterator create_mapping_recursive(
const QModelIndex &source_parent) const;
QModelIndex proxy_to_source(const QModelIndex &proxyIndex) const;
QModelIndex source_to_proxy(const QModelIndex &sourceIndex) const;
@ -265,14 +265,14 @@ public:
filter_regularexpression.setValueBypassingBindings(re);
}
inline QHash<QModelIndex, Mapping *>::const_iterator index_to_iterator(
inline QHash<QtPrivate::QModelIndexWrapper, Mapping *>::const_iterator index_to_iterator(
const QModelIndex &proxy_index) const
{
Q_ASSERT(proxy_index.isValid());
Q_ASSERT(proxy_index.model() == q_func());
const void *p = proxy_index.internalPointer();
Q_ASSERT(p);
QHash<QModelIndex, Mapping *>::const_iterator it =
QHash<QtPrivate::QModelIndexWrapper, Mapping *>::const_iterator it =
source_index_mapping.constFind(static_cast<const Mapping*>(p)->source_parent);
Q_ASSERT(it != source_index_mapping.constEnd());
Q_ASSERT(it.value());
@ -280,7 +280,7 @@ public:
}
inline QModelIndex create_index(int row, int column,
QHash<QModelIndex, Mapping*>::const_iterator it) const
QHash<QtPrivate::QModelIndexWrapper, Mapping*>::const_iterator it) const
{
return q_func()->createIndex(row, column, *it);
}
@ -382,7 +382,7 @@ public:
bool recursiveParentAcceptsRow(const QModelIndex &source_parent) const;
};
typedef QHash<QModelIndex, QSortFilterProxyModelPrivate::Mapping *> IndexMap;
typedef QHash<QtPrivate::QModelIndexWrapper, QSortFilterProxyModelPrivate::Mapping *> IndexMap;
static bool operator&(QSortFilterProxyModelPrivate::Direction a, QSortFilterProxyModelPrivate::Direction b)
{