Big performance problem with QDateTime
Robert Krawitz <rlk-FrUbXkNCsVf2fBVCVOL8/[email protected]> Sun, 20 Sep 2020 10:14:26 -0400
| Newsgroups | gmane.comp.kde.kimdaba |
|---|---|
| Message-ID | <[email protected]> |
Certain kpa operations, in particular startup and home (i. e. select all images) appear to have slowed down drastically recently. The problem is in the XMLImageDateCollection code; specifically, building the map between date and DB::ImageDate is very slow (the home operation is taking about 25 seconds on my system). This is done once at startup and whenever the selected set changes, for the date bar. The problem is that QDateTime comparisons, which need to be done extensively for map insertions, are very slow because to correctly handle timezone and DST issues the date/time has to be converted to milliseconds since the epoch. But there have been reports of this issue for quite a while (https://bugreports.qt.io/browse/QTBUG-41714), and apparently it's something that upstream can't or won't fix, and I would definitely have noticed this earlier. I suspect something has changed in the default behavior, possibly related to when I upgraded my laptop. Perhaps the linear time stamp had previously been cached and isn't now. There's a local fix to the datebar issue, namely use the time since the epoch as the key rather than the QDateTime itself. This results in the home operation now taking a second or two. Even so, the conversion to milliseconds is taking about 73% of the total time of that operation. For reference, the conversion had been done about 2.5E+07 times previously; it's now being done about 1E+06 times (with 380K images in my database). It also accounts for about 15% of the startup time, with the conversion being done about 1.4E+07 times (4 times per image). The fix really needs to be in QDateTime, to cache this. I don't want to rewrite every use of QDateTime in kpa, but this one case (see attached patch) clears up the most egregious behavior. There's an existing bug against Qt (https://bugreports.qt.io/browse/QTBUG-75585) about this that got closed, but I'm going to comment on it and maybe open a new one. _______________________________________________ KPhotoAlbum mailing list [email protected] https://mail.kdab.com/mailman/listinfo/kphotoalbum
datebar-performance.patch
(text/x-patch, 4.4 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..88286542 100644
--- a/XMLDB/XMLImageDateCollection.h
+++ b/XMLDB/XMLImageDateCollection.h
@@ -40,8 +40,10 @@ public:
QDateTime upperLimit() const override;
private:
- typedef QMap<QDateTime, DB::ImageDate> StartIndexMap;
- typedef QMap<QDateTime, StartIndexMap::ConstIterator> EndIndexMap;
+ typedef QMap<qint64, DB::ImageDate> StartIndexMap;
+ typedef QMap<qint64, StartIndexMap::ConstIterator> EndIndexMap;
+ typedef QHash<QDateTime, qint64> DateToSecHash;
+ typedef QHash<qint64, QDateTime> SecToDateHash;
void add(const DB::ImageDate &);