Re: Fw: Fwd: [Qt bugreports] Updates for QTBUG-75585: Massive performance regression in QML Date object
Johannes Zarl-Zierl <[email protected]> Fri, 25 Sep 2020 00:17:33 +0200
| Newsgroups | gmane.comp.kde.kimdaba |
|---|---|
| Message-ID | <2470859.lG1TQtc5xC@mani> |
Hi, Am Donnerstag, 24. September 2020, 03:02:30 CEST schrieb Robert Krawitz: > > (2) Store seconds since epoch for performance reasons > > I think this is the wrong place to search for performance gains. Robert > > has > > probably a better understanding of how much time we could save when > > reading > > the index.xml file, but I assume this to bring very slight gains. > > The problen's not with the database loading per se. The problem is with > sorting the list of dates for building the date bar Sorry, I did not make myself clear: I was talking about the suggestion to store seconds since epoch in index.xml. I'm not opposed to fixing the performance issue in the DateBar. Regarding that issue I only question is whether adding additional code "on the outside" of the date+time object is such a good idea. I.e. maintenance-wise, I think the code would stay cleaner if we make a wrapper that encapsulates the performance-tuning. Come to think of it, it might be possible to make the DateBar work with ImageDates instead of QDateTimes - this way we don't even need an additional wrapper class. (Take that last suggestion with a grain of salt - I didn't consult the code yet). > What I have in mind is creating a KPADateTime class that embeds (not > inherits from) QDateTime. It adds an additional qint64 that stores the > time since epoch. This additional member is mutable. The distinguished > value INT64_MIN is used to mean one of three things: > > 1) The KPADateTime is invalid. > > 2) The KPADateTime has been modified and the linear timestamp not yet > updated. > > 3) The KPADateTime really does represent a time back in the Permian period > (which I don't think EXIF can represent, anyway). > > We implement every method on KPADateTime that's actually used by KPhotoAlbum > (made easier by the fact that we're not deriving, we're wrapping, so > anything missing will result in a compile failure). All non-const > operations on the KPADateTime reset the stamp back to INT64_MIN. > Operations such as comparison, explicit time since the epoch, etc. behave > as follows: > > 1) If the linear stamp is not INT64_MIN, its value is used. > > 2) If it is INT64_MIN, we call isValid() on the embedded QDateTime. If it's > valid, we then extract the linear stamp and use it. If it's not valid, we > do the underlying call to the relevant function. > > Note that isValid() is fast, about 500 million checks per second for either > valid or invalid QDateTime on my system (vs. maybe 2.5 million conversions > to the linear timestamp). So that's not concern, especially since we're > not even calling it very much. One question about this: I assume that we read every datetime at least once and rarely alter a datetime. Why then do the whole dance with the mutable qint64? We could just as well initialize the qint64 on construction and on every modification... Cheers, Johannes