From ec5c3703fcaf133cb70e461250b5be716017511d Mon Sep 17 00:00:00 2001 From: David Faure Date: Sun, 9 Apr 2017 17:27:48 +0200 Subject: [PATCH] SearchDialog: improve performance by caching collection full paths. Finding out the full path is a very slow operation because we must locate the index of the collection in the big ETM, and this was done very often since it's the sort column. No precise measurements, but gut feeling, for displaying 3000 search results, is that it went from 30 seconds to 3. --- src/CMakeLists.txt | 1 - src/searchdialog/kmsearchfilterproxymodel.cpp | 69 ------------------- src/searchdialog/kmsearchfilterproxymodel.h | 40 ----------- src/searchdialog/kmsearchmessagemodel.cpp | 42 +++++------ src/searchdialog/kmsearchmessagemodel.h | 10 ++- src/searchdialog/searchwindow.cpp | 7 +- 6 files changed, 34 insertions(+), 135 deletions(-) delete mode 100644 src/searchdialog/kmsearchfilterproxymodel.cpp delete mode 100644 src/searchdialog/kmsearchfilterproxymodel.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 3fecde21a..065b2af56 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -69,7 +69,6 @@ set(kmailprivate_configureplugins_LIB_SRCS set(kmailprivate_searchdialog_LIB_SRCS searchdialog/kmsearchmessagemodel.cpp - searchdialog/kmsearchfilterproxymodel.cpp searchdialog/searchpatternwarning.cpp searchdialog/kmailsearchpatternedit.cpp searchdialog/searchwindow.cpp diff --git a/src/searchdialog/kmsearchfilterproxymodel.cpp b/src/searchdialog/kmsearchfilterproxymodel.cpp deleted file mode 100644 index 782ddb559..000000000 --- a/src/searchdialog/kmsearchfilterproxymodel.cpp +++ /dev/null @@ -1,69 +0,0 @@ -/* - Copyright (C) 2011-2017 Montel Laurent - - This program is free software; you can redistribute it and/or - modify it under the terms of the GNU General Public - License as published by the Free Software Foundation; either - version 2 of the License, or (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program; see the file COPYING. If not, write to - the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, - Boston, MA 02110-1301, USA. -*/ - -#include "kmsearchfilterproxymodel.h" -#include "kmsearchmessagemodel.h" -#include -#include - -using namespace KMail; - -KMSearchFilterProxyModel::KMSearchFilterProxyModel(QObject *parent) - : QSortFilterProxyModel(parent) -{ - setDynamicSortFilter(true); - setFilterCaseSensitivity(Qt::CaseInsensitive); -} - -KMSearchFilterProxyModel::~KMSearchFilterProxyModel() -{ -} - -bool KMSearchFilterProxyModel::lessThan(const QModelIndex &left, const QModelIndex &right) const -{ - if (right.model() && left.model() && left.column() == KMSearchMessageModel::Date) { - if (sourceModel()) { - const QDateTime leftData = - sourceModel()->data( - left.sibling(left.row(), KMSearchMessageModel::DateNotTranslated)).toDateTime(); - - const QDateTime rightData = - sourceModel()->data( - right.sibling(right.row(), KMSearchMessageModel::DateNotTranslated)).toDateTime(); - return leftData < rightData; - } else { - return false; - } - } - - if (right.model() && left.model() && left.column() == KMSearchMessageModel::Size) { - if (sourceModel()) { - const qint64 leftData = - sourceModel()->data( - left.sibling(left.row(), KMSearchMessageModel::SizeNotLocalized)).toLongLong(); - const qint64 rightData = - sourceModel()->data( - right.sibling(right.row(), KMSearchMessageModel::SizeNotLocalized)).toLongLong(); - return leftData < rightData; - } else { - return false; - } - } - return QSortFilterProxyModel::lessThan(left, right); -} diff --git a/src/searchdialog/kmsearchfilterproxymodel.h b/src/searchdialog/kmsearchfilterproxymodel.h deleted file mode 100644 index 495d825be..000000000 --- a/src/searchdialog/kmsearchfilterproxymodel.h +++ /dev/null @@ -1,40 +0,0 @@ -/* - Copyright (C) 2011-2017 Montel Laurent - - This program is free software; you can redistribute it and/or - modify it under the terms of the GNU General Public - License as published by the Free Software Foundation; either - version 2 of the License, or (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program; see the file COPYING. If not, write to - the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, - Boston, MA 02110-1301, USA. -*/ - -#ifndef KMSEARCHFILTERPROXYMODEL_H -#define KMSEARCHFILTERPROXYMODEL_H - -#include - -class QModelIndex; - -namespace KMail -{ -class KMSearchFilterProxyModel : public QSortFilterProxyModel -{ -public: - explicit KMSearchFilterProxyModel(QObject *parent); - ~KMSearchFilterProxyModel(); -protected: - bool lessThan(const QModelIndex &left, const QModelIndex &right) const Q_DECL_OVERRIDE; -}; -} - -#endif /* KMSEARCHFILTERPROXYMODEL_H */ - diff --git a/src/searchdialog/kmsearchmessagemodel.cpp b/src/searchdialog/kmsearchmessagemodel.cpp index 0adf3f29b..3ecd79b4a 100644 --- a/src/searchdialog/kmsearchmessagemodel.cpp +++ b/src/searchdialog/kmsearchmessagemodel.cpp @@ -57,7 +57,7 @@ KMSearchMessageModel::~KMSearchMessageModel() { } -QString toolTip(const Akonadi::Item &item) +static QString toolTip(const Akonadi::Item &item) { KMime::Message::Ptr msg = item.payload(); @@ -134,12 +134,22 @@ int KMSearchMessageModel::columnCount(const QModelIndex &parent) const } if (!parent.isValid()) { - return 8; // keep in sync with the column type enum + return 6; // keep in sync with the column type enum } return 0; } +QString KMSearchMessageModel::fullCollectionPath(Akonadi::Collection::Id id) const +{ + QString path = m_collectionFullPathCache.value(id); + if (path.isEmpty()) { + path = MailCommon::Util::fullCollectionPath(Akonadi::Collection(id)); + m_collectionFullPathCache.insert(id, path); + } + return path; +} + QVariant KMSearchMessageModel::data(const QModelIndex &index, int role) const { if (!index.isValid()) { @@ -159,17 +169,21 @@ QVariant KMSearchMessageModel::data(const QModelIndex &index, int role) const } Akonadi::Item item = itemForIndex(index); + + // Handle the most common case first, before calling payload(). + if ((role == Qt::DisplayRole || role == Qt::EditRole) && index.column() == Collection) { + if (item.storageCollectionId() >= 0) { + return fullCollectionPath(item.storageCollectionId()); + } + return fullCollectionPath(item.parentCollection().id()); + } + if (!item.hasPayload()) { return QVariant(); } KMime::Message::Ptr msg = item.payload(); if (role == Qt::DisplayRole) { switch (index.column()) { - case Collection: - if (item.storageCollectionId() >= 0) { - return MailCommon::Util::fullCollectionPath(Akonadi::Collection(item.storageCollectionId())); - } - return MailCommon::Util::fullCollectionPath(item.parentCollection()); case Subject: return msg->subject()->asUnicodeString(); case Sender: @@ -184,20 +198,11 @@ QVariant KMSearchMessageModel::data(const QModelIndex &index, int role) const } else { return KFormat().formatByteSize(item.size()); } - case SizeNotLocalized: - return item.size(); - case DateNotTranslated: - return msg->date()->dateTime(); default: return QVariant(); } - } else if (role == Qt::EditRole) { + } else if (role == Qt::EditRole) { // used for sorting switch (index.column()) { - case Collection: - if (item.storageCollectionId() >= 0) { - return MailCommon::Util::fullCollectionPath(Akonadi::Collection(item.storageCollectionId())); - } - return MailCommon::Util::fullCollectionPath(item.parentCollection()); case Subject: return msg->subject()->asUnicodeString(); case Sender: @@ -206,11 +211,8 @@ QVariant KMSearchMessageModel::data(const QModelIndex &index, int role) const return msg->to()->asUnicodeString(); case Date: return msg->date()->dateTime(); - case SizeNotLocalized: case Size: return item.size(); - case DateNotTranslated: - return msg->date()->dateTime(); default: return QVariant(); } diff --git a/src/searchdialog/kmsearchmessagemodel.h b/src/searchdialog/kmsearchmessagemodel.h index 7918337fc..40a2f956a 100644 --- a/src/searchdialog/kmsearchmessagemodel.h +++ b/src/searchdialog/kmsearchmessagemodel.h @@ -29,6 +29,7 @@ #define KMSEARCHMESSAGEMODEL_H #include +#include class KMSearchMessageModel : public Akonadi::MessageModel { @@ -41,9 +42,7 @@ public: Sender, Receiver, Date, - Size, - DateNotTranslated, - SizeNotLocalized + Size }; explicit KMSearchMessageModel(QObject *parent = nullptr); ~KMSearchMessageModel(); @@ -53,6 +52,11 @@ public: QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const Q_DECL_OVERRIDE; +private: + QString fullCollectionPath(Akonadi::Collection::Id id) const; + + mutable QHash m_collectionFullPathCache; + }; #endif diff --git a/src/searchdialog/searchwindow.cpp b/src/searchdialog/searchwindow.cpp index 2a09f2b15..b65b2ceac 100644 --- a/src/searchdialog/searchwindow.cpp +++ b/src/searchdialog/searchwindow.cpp @@ -33,7 +33,6 @@ #include "searchdescriptionattribute.h" #include "MailCommon/FolderTreeView" #include "kmsearchmessagemodel.h" -#include "kmsearchfilterproxymodel.h" #include "searchpatternwarning.h" #include "PimCommonAkonadi/SelectMultiCollectionDialog" #include @@ -51,6 +50,7 @@ #include #include "kmail_debug.h" #include +#include #include #include #include @@ -270,7 +270,10 @@ void SearchWindow::createSearchModel() } mResultModel = new KMSearchMessageModel(this); mResultModel->setCollection(mFolder); - KMSearchFilterProxyModel *sortproxy = new KMSearchFilterProxyModel(mResultModel); + QSortFilterProxyModel *sortproxy = new QSortFilterProxyModel(mResultModel); + sortproxy->setDynamicSortFilter(true); + sortproxy->setSortRole(Qt::EditRole); + sortproxy->setFilterCaseSensitivity(Qt::CaseInsensitive); sortproxy->setSourceModel(mResultModel); mUi.mLbxMatches->setModel(sortproxy);