[KPhotoAlbum] Fw: Fwd: [Qt bugreports] Updates for QTBUG-75585: Massive performance regression in QML Date object

[Johannes Zarl-Zierl]

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