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

[Robert Krawitz]

On 9/22/20 3:56 AM, Martin Höller wrote:
> Mail was meant to go to mailinglist...
> 
> --- Begin forwarded message: ---
> 
> Date: Tue, 22 Sep 2020 08:56:49 +0200
> From: Martin Höller <martin at xss.co.at>
> To: Johannes Zarl-Zierl <johannes at zarl-zierl.at>
> Subject: Re: [KPhotoAlbum] Fwd: [Qt bugreports] Updates for QTBUG-75585: Massive performance regression in QML Date object
> 
> 
> Hi!
> 
> Just to add some ideas...
> 
> Am 21. Sep. 2020 schrieb Johannes Zarl-Zierl:
> [...]
>> My conclusion to the referenced discussion was that a proper solution would be 
>> to add timezone info to the database. A not quite correct solution that could 
>> be good enough is probably to convert everything to UTZ.
>> For the current issue, we would need UTZ data only in the database.
>>
>> So simple solution would be to convert to UTZ and be done with it.  
> 
> If times are stored in UTC we could consider not saving it as a string
> representation that would have to be parsed (e.g. "2012-08-16T12:01:09")
> but as (milli)seconds since epoch (e.g. "1345118469"). Parsing a simple
> number is much faster than parsing a date-string.
> 
> We did that in project an thus saved quite a huge amount of parsing time
> on each load (we had several 100 millions of timestamps).

It's remarkable how long it takes to interpret even milliseconds since epoch if UTC isn't specified.
 The problem's not the parsing, it's all the corner cases and such with interpreting dates and
times.  This based on measurement.

We've had a general principle that we want the index.xml file to be human-readable.  This would
break that.

My opinion: the database should store all dates in UTC, all the time.  Then there's no ambiguity.
It would solve the current problem, but we can't do this until we rev the database version.  When
someone selects images by date, of course, they expect to select it based on either local time or
"time when the shot was taken", and we do a conversion then just for filtering.

>> I still think that the proper solution to the problem would be to add real 
>> timezone support (in the future, more and more cameras will come with EXIF 
>> 2.31) and having proper timezone info will allow us to retain local time for 
>> display.  
> 
> I too think, that's the way to go. But I would save it as a separate,
> additional attribute in XML.

Per the Qt people, any timezone specification other than UTC is expensive to compare/convert to
milliseconds since epoch due to all of the DST stuff involved (I notice that each call does
getenv(), for example).

They say that the reason they don't want to cache the milliseconds since epoch is the memory
requirement (additional 8 bytes).  That would double the size.  Somehow 16 bytes for this total
doesn't strike me as excessive, considering how expensive the conversion is.  But not my call.

> While we are at it: it would probably be nice to have the opportunity so
> show photo-times as "local time" or as "time taken". You know i looks
> strange, when you see some midnight party photo and KPA says it was taken
> at 15:07:21 because you are in a different timezone when looking at the
> photo.

Well, legacy photos won't have a timezone, so we don't even know how to interpret "time taken".

>> Given that a generic solution in QDateTime seems unlikely, why not implement a 
>> wrapper that does the caching? This would not be all that different from our 
>> approach with DB::FileName. Just don't call it QuickTime ;-)  
> 
> This could be a good approach.

The approach I had in mind uses INT64_MIN as a distinguishing value indicating that the cached
timestamp is invalid.  If handling dates early in the Permian period is slow...tough.  But if we use
only UTC dates in the database (both on disk and in memory), we don't need to do this.

In the short term I think we should relieve the immediate pain point with my patch or something like
it, and add a note that we revert it when when we modify the database to be all UTC time.