Re: Big performance problem with QDateTime
Robert Krawitz <rlk-FrUbXkNCsVf2fBVCVOL8/[email protected]> Sun, 20 Sep 2020 17:43:26 -0400
| Newsgroups | gmane.comp.kde.kimdaba |
|---|---|
| Message-ID | <[email protected]> |
On 9/20/20 5:29 PM, Johannes Zarl-Zierl wrote: > Hi, > > Am Sonntag, 20. September 2020, 17:38:25 CEST schrieb Robert Krawitz: >>> And adding the ms representation since the epoch of the date and time as a >>> member variable, along with a boolean one that indicates if the cache >>> should be recomputed next time it's requested should be trivial to add. >>> Without breaking the API. >>> >>> Thanks for tracking this down :-) >> >> I'd like to get the patch in anyway, if you and Johannes don't object. Even >> if Qt fixes it, it's going to take a while for the fix there to make it >> out, and in the interim it's drastically slowing things down here. > > I'll sleep on it and see if we can encapsulate this a little better or make it > nicer to read - I fear that by the time Qt catches up and removes the > performance bottleneck we have no idea why we convert to seconds since epoch > in the first place. > > Apart from that, I concur that we should fix this on our side for now... Does this comment help? _______________________________________________ KPhotoAlbum mailing list [email protected] https://mail.kdab.com/mailman/listinfo/kphotoalbum
qdatetime-perf-1.patch
(text/x-patch, 5 KB)
diff --git a/XMLDB/XMLImageDateCollection.cpp b/XMLDB/XMLImageDateCollection.cpp
index 5fa052da..38bc2c79 100644
--- a/XMLDB/XMLImageDateCollection.cpp
+++ b/XMLDB/XMLImageDateCollection.cpp
@@ -20,19 +20,23 @@
#include <DB/FileNameList.h>
#include <DB/ImageDB.h>
+#include <stdint.h>
void XMLDB::XMLImageDateCollection::add(const DB::ImageDate &date)
{
- m_startIndex.insertMulti(date.start(), date);
+ if (date.start().isValid()) {
+ qint64 startMSecs = date.start().toMSecsSinceEpoch();
+ m_startIndex.insertMulti(startMSecs, date);
+ }
}
void XMLDB::XMLImageDateCollection::buildIndex()
{
StartIndexMap::ConstIterator startSearch = m_startIndex.constBegin();
#if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)
- QDateTime biggestEnd = QDate(1900, 1, 1).startOfDay();
+ qint64 biggestEnd = QDate(1900, 1, 1).startOfDay().toMSecsSinceEpoch();
#else
- QDateTime biggestEnd = QDateTime(QDate(1900, 1, 1));
+ qint64 biggestEnd = QDateTime(QDate(1900, 1, 1)).toMSecsSinceEpoch();
#endif
for (StartIndexMap::ConstIterator it = m_startIndex.constBegin();
it != m_startIndex.constEnd();
@@ -42,11 +46,14 @@ void XMLDB::XMLImageDateCollection::buildIndex()
// have to keep the last pointer as long as we find smaller end-dates.
// This should be rare as it only occurs if there are images that
// actually represent a range not just a point in time.
- if (it.value().end() >= biggestEnd) {
- biggestEnd = it.value().end();
- startSearch = it;
+ if (it.value().end().isValid()) {
+ qint64 endMSecs = it.value().end().toMSecsSinceEpoch();
+ if (endMSecs >= biggestEnd) {
+ biggestEnd = endMSecs;
+ startSearch = it;
+ }
+ m_endIndex.insert(endMSecs, startSearch);
}
- m_endIndex.insert(it.value().end(), startSearch);
}
}
@@ -87,11 +94,12 @@ DB::ImageCount XMLDB::XMLImageDateCollection::count(const DB::ImageDate &range)
// We start searching in ranges that overlap our start search range, i.e.
// where the end-date is higher than our search start.
- EndIndexMap::Iterator endSearch = m_endIndex.lowerBound(range.start());
+ EndIndexMap::Iterator endSearch = m_endIndex.lowerBound(range.start().toMSecsSinceEpoch());
+ qint64 endMSecs = range.end().toMSecsSinceEpoch();
if (endSearch != m_endIndex.end()) {
for (StartIndexMap::ConstIterator it = endSearch.value();
- it != m_startIndex.constEnd() && it.key() < range.end();
+ it != m_startIndex.constEnd() && it.key() < endMSecs;
++it) {
DB::ImageDate::MatchType tp = it.value().isIncludedIn(range);
switch (tp) {
@@ -117,8 +125,11 @@ QDateTime XMLDB::XMLImageDateCollection::lowerLimit() const
if (!m_startIndex.empty()) {
// skip null dates:
for (StartIndexMap::ConstIterator it = m_startIndex.constBegin(); it != m_startIndex.constEnd(); ++it) {
- if (it.key().isValid())
- return it.key();
+ if (it.key() != INT64_MIN) {
+ QDateTime answer;
+ answer.setMSecsSinceEpoch(it.key());
+ return answer;
+ }
}
}
#if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)
@@ -133,7 +144,9 @@ QDateTime XMLDB::XMLImageDateCollection::upperLimit() const
if (!m_endIndex.empty()) {
EndIndexMap::ConstIterator highest = m_endIndex.constEnd();
--highest;
- return highest.key();
+ QDateTime answer;
+ answer.setMSecsSinceEpoch(highest.key());
+ return answer;
}
#if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)
return QDate(2100, 1, 1).startOfDay();
diff --git a/XMLDB/XMLImageDateCollection.h b/XMLDB/XMLImageDateCollection.h
index 1d49e2ea..5eda6eee 100644
--- a/XMLDB/XMLImageDateCollection.h
+++ b/XMLDB/XMLImageDateCollection.h
@@ -40,8 +40,22 @@ public:
QDateTime upperLimit() const override;
private:
- typedef QMap<QDateTime, DB::ImageDate> StartIndexMap;
- typedef QMap<QDateTime, StartIndexMap::ConstIterator> EndIndexMap;
+ // Use qint64 rather than QDateTime because comparison between
+ // QDateTime (via operator<, used by QMap) is very slow and this
+ // leads to substantial delays at startup and when selecting a
+ // very large number of images.
+ //
+ // On a Xeon E3-1505v5, selecting all images from a set of about
+ // 380,000 takes 20 seconds or thereabouts; about 95% of the time
+ // is spent in QDateTime::operator< . Using qint64 (converting
+ // all of the QDateTime's to qint64's in one shot), it takes about
+ // 2 seconds. Even so, over 70% of that time is spent in the
+ // conversion to linear milliseconds.
+ //
+ // More background in https://bugreports.qt.io/browse/QTBUG-75585
+ // and https://bugreports.qt.io/browse/QTBUG-41714
+ typedef QMap<qint64, DB::ImageDate> StartIndexMap;
+ typedef QMap<qint64, StartIndexMap::ConstIterator> EndIndexMap;
void add(const DB::ImageDate &);