[KPhotoAlbum] Big performance problem with QDateTime

[Robert Krawitz]

On 9/20/20 10:51 AM, Tobias Leupold wrote:
> Hi Robert!
> 
> I'm pretty sure the Qt guys are glad to see your analysis of this. At least
> they really shouls be. If a simple cache inside QDateTime can cheap out 3/4 of
> the computing time in our case, you can be sure it does for other use cases.

Well, we'll see.  It's not the first bug that's been filed on this, and their response has been
"comparing dates Is Complicated (tm)".  One would think that a cache would be obvious.  Emphasis on
"one would think".  So maybe there's some weird corner case like a time referenced to "local time
zone", but that changes due to someone taking their laptop elsewhere.  Still, I'd think there would
have to be some way of avoiding that kind of a huge cost.

> And adding the ms representation since the epoch of the date and time as a
> member variable, along with a boolean one that indicates if the cache should
> be recomputed next time it's requested should be trivial to add. Without
> breaking the API.
> 
> Thanks for tracking this down :-)

I'd like to get the patch in anyway, if you and Johannes don't object.  Even if Qt fixes it, it's
going to take a while for the fix there to make it out, and in the interim it's drastically slowing
things down here.

> Cheers, Tobias
> 
> Am Sonntag, 20. September 2020, 16:14:26 CEST schrieb Robert Krawitz:
>> 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
> KPhotoAlbum at mail.kdab.com
> https://mail.kdab.com/mailman/listinfo/kphotoalbum
>