From 19899d88eb17de4fc256d02cb4b67050e94389a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Tue, 22 Oct 2024 16:36:45 +0200 Subject: [PATCH] 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 (cherry picked from commit 1558811a8485f6dcc51a50a2bba0846091ca8bf6) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/itemmodels/qabstractitemmodel.cpp | 2 +- src/corelib/itemmodels/qabstractitemmodel_p.h | 22 ++++++++++++++++++- .../itemmodels/qsortfilterproxymodel.cpp | 14 ++++++------ 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/corelib/itemmodels/qabstractitemmodel.cpp b/src/corelib/itemmodels/qabstractitemmodel.cpp index 3d4d566d46c..3b929c8ec08 100644 --- a/src/corelib/itemmodels/qabstractitemmodel.cpp +++ b/src/corelib/itemmodels/qabstractitemmodel.cpp @@ -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(index.model()); - QMultiHash &indexes = model->d_func()->persistent.indexes; + QMultiHash &indexes = model->d_func()->persistent.indexes; const auto it = indexes.constFind(index); if (it != indexes.cend()) { d = (*it); diff --git a/src/corelib/itemmodels/qabstractitemmodel_p.h b/src/corelib/itemmodels/qabstractitemmodel_p.h index c2113fde9aa..d22a0a50823 100644 --- a/src/corelib/itemmodels/qabstractitemmodel_p.h +++ b/src/corelib/itemmodels/qabstractitemmodel_p.h @@ -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 indexes; + QMultiHash indexes; QStack> moved; QStack> invalidated; void insertMultiAtEnd(const QModelIndex& key, QPersistentModelIndexData *data); diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp index 73da00642a7..4076487d24a 100644 --- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp +++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp @@ -126,7 +126,7 @@ public: QModelIndex source_parent; }; - mutable QHash source_index_mapping; + mutable QHash source_index_mapping; void setSortCaseSensitivityForwarder(Qt::CaseSensitivity cs) { @@ -239,9 +239,9 @@ public: std::array sourceConnections; - QHash::const_iterator create_mapping( + QHash::const_iterator create_mapping( const QModelIndex &source_parent) const; - QHash::const_iterator create_mapping_recursive( + QHash::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::const_iterator index_to_iterator( + inline QHash::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::const_iterator it = + QHash::const_iterator it = source_index_mapping.constFind(static_cast(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::const_iterator it) const + QHash::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 IndexMap; +typedef QHash IndexMap; static bool operator&(QSortFilterProxyModelPrivate::Direction a, QSortFilterProxyModelPrivate::Direction b) {