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

[Johannes Zarl-Zierl]

Am Samstag, 26. September 2020, 21:25:40 CEST schrieb Robert Krawitz:
> On 9/25/20 3:20 PM, Johannes Zarl-Zierl wrote:
> > Am Freitag, 25. September 2020, 03:53:21 CEST schrieb Robert Krawitz:
> >> So I've taken a crack at a FastDateTime (wrote the header file and made
> >> the
> >> changes elsewhere, but haven't actually implemented it).  However, it
> >> looks
> >> like I don't have ssh keys installed so I can't push now.
> >> 
> >> I've attached the header file for reference.
> > 
> > Looks good to me...
> 
> I've opened an MR for this and assigned it to Johannes:
> https://invent.kde.org/graphics/kphotoalbum/-/merge_requests/2

Thanks - I've taken the opportunity to try out doing a review using the gitlab 
UI...

While reading the code in XMLDB/Database.cpp I stumbled about the following 
comment that may be relevant to the discussion we had about adding timezone 
information to index.xml:

````
// During profiling of loading, I found that a significant amount of time was 
spent in Utilities::FastDateTime::fromString.
// Reviewing the code, I fount that it did a lot of extra checks we don't need 
(like checking if the string have
// timezone information (which they won't in KPA), this function is a 
replacement that is faster than the original.
Utilities::FastDateTime dateTimeFromString(const QString &str) { /*...*/}
````

I conclude from this comment that adding timezone info will come with some 
cost. Still, I see this as a worthwhile endeavour...


> It was quick enough to code up and I did some measurements both with
> /usr/bin/time and with kcachegrind.  It sped up startup by about 20-25%, a
> bit more than I expected based on the kcachegrind output.  While I was at
> it, I noticed that the start() and end() methods of ImageDate returned
> copied objects rather than const references; the only places in which those
> were used were using them in a const manner.
> 
> Startup improved from about 12 seconds (with an earlier version of Qt that
> apparently didn't have this problem) to 8.75; rebuilding the thumbnail bar
> took a few seconds with that version of Qt and about 20 seconds with the
> current version, now it's probably a few hundred ms (it took about 9
> seconds under callgrind, so that's a reasonable estimate).

Additional performance gains are always good news ;-)

> Note that I went simple with it, always calculating the linear timestamp
> whenever performing a non-const operation.  It looks like the place where
> large numbers of date/time objects are created is reading the database, and
> the timestamp is always used there to update the date bar, so no sense
> getting fancy here with a mutable linear timestamp.

LGTM.

Cheers,
  Johannes