Speedup sort

Summary:
Use a lambda function to use only one QCollator initialized only once.

This requires a workaround for https://bugreports.qt.io/browse/QTBUG-69361
Just a single comparison to force the clean state of QCollator.

Test Plan:
Sorting in a directory with 82874 images:
[TIME] Sorting: 19883  (before)
[TIME] Sorting: 4198 (after)

kfileitemmodelbenchmark before: ..............   Passed   29.36 sec
kfileitemmodelbenchmark after:    ..............   Passed   20.39 sec

Reviewers: #dolphin, #frameworks, markg, elvisangelaccio, bruns

Reviewed By: #dolphin, markg, elvisangelaccio

Subscribers: elvisangelaccio, apol, bruns, markg, kfm-devel

Tags: #dolphin

Differential Revision: https://phabricator.kde.org/D13814
This commit is contained in:
Jaime Torres 2018-07-13 18:54:49 +02:00
parent 1ba972adf5
commit 765cc968c9
2 changed files with 9 additions and 46 deletions

View file

@ -826,6 +826,9 @@ void KFileItemModel::loadSortingSettings()
default: default:
Q_UNREACHABLE(); Q_UNREACHABLE();
} }
// Workaround for bug https://bugreports.qt.io/browse/QTBUG-69361
// Force the clean state of QCollator in single thread to avoid thread safety problems in sort
m_collator.compare(QString(), QString());
} }
void KFileItemModel::resortAllItems() void KFileItemModel::resortAllItems()
@ -1709,63 +1712,24 @@ bool KFileItemModel::lessThan(const ItemData* a, const ItemData* b, const QColla
return (sortOrder() == Qt::AscendingOrder) ? result < 0 : result > 0; return (sortOrder() == Qt::AscendingOrder) ? result < 0 : result > 0;
} }
/**
* Helper class for KFileItemModel::sort().
*/
class KFileItemModelLessThan
{
public:
KFileItemModelLessThan(const KFileItemModel* model, const QCollator& collator) :
m_model(model),
m_collator(collator)
{
}
KFileItemModelLessThan(const KFileItemModelLessThan& other) :
m_model(other.m_model),
m_collator()
{
m_collator.setCaseSensitivity(other.m_collator.caseSensitivity());
m_collator.setIgnorePunctuation(other.m_collator.ignorePunctuation());
m_collator.setLocale(other.m_collator.locale());
m_collator.setNumericMode(other.m_collator.numericMode());
}
~KFileItemModelLessThan() = default;
//We do not delete m_model as the pointer was passed from outside ant it will be deleted elsewhere.
KFileItemModelLessThan& operator=(const KFileItemModelLessThan& other)
{
m_model = other.m_model;
m_collator = other.m_collator;
return *this;
}
bool operator()(const KFileItemModel::ItemData* a, const KFileItemModel::ItemData* b) const
{
return m_model->lessThan(a, b, m_collator);
}
private:
const KFileItemModel* m_model;
QCollator m_collator;
};
void KFileItemModel::sort(QList<KFileItemModel::ItemData*>::iterator begin, void KFileItemModel::sort(QList<KFileItemModel::ItemData*>::iterator begin,
QList<KFileItemModel::ItemData*>::iterator end) const QList<KFileItemModel::ItemData*>::iterator end) const
{ {
KFileItemModelLessThan lessThan(this, m_collator); auto lambdaLessThan = [&] (const KFileItemModel::ItemData* a, const KFileItemModel::ItemData* b)
{
return lessThan(a, b, m_collator);
};
if (m_sortRole == NameRole) { if (m_sortRole == NameRole) {
// Sorting by name can be expensive, in particular if natural sorting is // Sorting by name can be expensive, in particular if natural sorting is
// enabled. Use all CPU cores to speed up the sorting process. // enabled. Use all CPU cores to speed up the sorting process.
static const int numberOfThreads = QThread::idealThreadCount(); static const int numberOfThreads = QThread::idealThreadCount();
parallelMergeSort(begin, end, lessThan, numberOfThreads); parallelMergeSort(begin, end, lambdaLessThan, numberOfThreads);
} else { } else {
// Sorting by other roles is quite fast. Use only one thread to prevent // Sorting by other roles is quite fast. Use only one thread to prevent
// problems caused by non-reentrant comparison functions, see // problems caused by non-reentrant comparison functions, see
// https://bugs.kde.org/show_bug.cgi?id=312679 // https://bugs.kde.org/show_bug.cgi?id=312679
mergeSort(begin, end, lessThan); mergeSort(begin, end, lambdaLessThan);
} }
} }

View file

@ -499,7 +499,6 @@ private:
// and done step after step in slotCompleted(). // and done step after step in slotCompleted().
QSet<QUrl> m_urlsToExpand; QSet<QUrl> m_urlsToExpand;
friend class KFileItemModelLessThan; // Accesses lessThan() method
friend class KFileItemModelRolesUpdater; // Accesses emitSortProgress() method friend class KFileItemModelRolesUpdater; // Accesses emitSortProgress() method
friend class KFileItemModelTest; // For unit testing friend class KFileItemModelTest; // For unit testing
friend class KFileItemModelBenchmark; // For unit testing friend class KFileItemModelBenchmark; // For unit testing