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

[Robert Krawitz]

On 9/23/20 6:47 PM, Johannes Zarl-Zierl wrote:
> Hi,
> 
> Answering to the whole thread in a single message because I'm quite swamped 
> and don't have too much time the next few weeks...
> 
> So there were several suggestions so far:
> 
> (1) Make everything UTZ in the DB, with a separate TZ info
> I'm not a huge fan of this solution (even though I also wrote before that we 
> may be able to get away with it). This separates the timezone from the image 
> date and complicates the handling of datetimes further.
> On the plus side, this complements suggestion (2). Yet if we stay with ISO 
> date/time representation it actually makes things more complicated.

Possibly, yes.

> (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 (the sort is done by building a map, which amounts to the same thing; it
uses a B-tree or red-black tree, I don't remember which; either way, it's N log2 n).  Each
comparison requires two conversions from the QDateTime representation to the linear milliseconds
since the epoch representation, and that conversion's very expensive.  With 380,000 images in my
database -- that's about 2^18.5 -- it amounts to 37 conversions * 380,000 images, or something more
than 12,000,000 date comparisons.  Each conversion takes about 500 nsec.  What's worse, it's done
twice, since we have fuzzy dates (start and end time), so that's more like 25 million times.

This one operation is done about 2.3 million times during loading of the database -- 772,000 in the
XMLDB::XMLImageDateCollection constructor, the same number in XMLImageDateCollection::buildIndex,
and the same number when initially drawing the date bar widget (which is what alerted me to the
problem in the first place).  That's with my workaround in the date bar widget.  So it's being
called about 4x as much as it needs to be, and amounts to 15% of the startup time.  So since kpa
takes about 11 seconds to start up for me, doing the full fix would probably save about a second of
startup time -- not a lot, but noticeable.  Where it would really save is in drawing the date bar.
With my fix, drawing the date bar takes about 2 seconds; 73% or so of the time is spent doing this
conversion once per date.  That would be a noticeable improvement.

> My money is on another suggestion by Robert (some time ago), which is to cache 
> the database in sqlite and only do a full read of the index.xml when it was 
> changed outside KPhotoAlbum. This would bring us bigger performance gains 
> *and* allow us to make use of SQL queries for searching.

That's not the problem (or at least, not this problem); QDateTime (specifically, conversion to
milliseconds since the epoch) is the problem.  I'm not thrilled about the 12 second startup time,
and I think we should look at that, but as a separate problem.

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.