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

diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp
index 9db3a2e5a8d3f5ee531d00cb0d0279cc50f55be1..56c21f51ca50329eeac5c18166154b0fb3da78b4 100644 (file)
--- a/src/kitemviews/kfileitemmodel.cpp
+++ b/src/kitemviews/kfileitemmodel.cpp
@@ -826,6 +826,9 @@ void KFileItemModel::loadSortingSettings()
     default:
         Q_UNREACHABLE();
     }
     default:
         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,
                           QList<KFileItemModel::ItemData*>::iterator end) const
 {
 void KFileItemModel::sort(QList<KFileItemModel::ItemData*>::iterator begin,
                           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) {
         // Sorting by name can be expensive, in particular if natural sorting is
         // enabled. Use all CPU cores to speed up the sorting process.
         static const int numberOfThreads = QThread::idealThreadCount();
 
     if (m_sortRole == NameRole) {
         // Sorting by name can be expensive, in particular if natural sorting is
         // enabled. Use all CPU cores to speed up the sorting process.
         static const int numberOfThreads = QThread::idealThreadCount();

-        parallelMergeSort(begin, end, lessThan, numberOfThreads);
+        parallelMergeSort(begin, end, lambdaLessThan, numberOfThreads);

     } else {
         // Sorting by other roles is quite fast. Use only one thread to prevent
         // problems caused by non-reentrant comparison functions, see
         // https://bugs.kde.org/show_bug.cgi?id=312679
     } else {
         // Sorting by other roles is quite fast. Use only one thread to prevent
         // problems caused by non-reentrant comparison functions, see
         // https://bugs.kde.org/show_bug.cgi?id=312679

-        mergeSort(begin, end, lessThan);
+        mergeSort(begin, end, lambdaLessThan);

     }
 }
 
     }
 }
 

diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h
index 134c50245c897318467bdb4f2a21e5297b1f88d9..a931a28cc35d5b07954f72931d7fb50e93e42767 100644 (file)
--- a/src/kitemviews/kfileitemmodel.h
+++ b/src/kitemviews/kfileitemmodel.h
@@ -499,7 +499,6 @@ private:
     // and done step after step in slotCompleted().
     QSet<QUrl> m_urlsToExpand;
 
     // and done step after step in slotCompleted().
     QSet<QUrl> m_urlsToExpand;
 

-    friend class KFileItemModelLessThan;       // Accesses lessThan() method

     friend class KFileItemModelRolesUpdater;   // Accesses emitSortProgress() method
     friend class KFileItemModelTest;           // For unit testing
     friend class KFileItemModelBenchmark;      // For unit testing
     friend class KFileItemModelRolesUpdater;   // Accesses emitSortProgress() method
     friend class KFileItemModelTest;           // For unit testing
     friend class KFileItemModelBenchmark;      // For unit testing