[GNC-dev] More need for speed improvement around XML

[Christian Stimming]

Am Sonntag, 30. Dezember 2018, 00:44:40 CET schrieb John Ralls:
> > On Dec 29, 2018, at 2:16 PM, Christian Stimming <christian at cstimming.de>
> > wrote:> 
> > Am Samstag, 29. Dezember 2018, 01:45:32 CET schrieb John Ralls:
> >>> When saving to XML file, for each transaction the call stack with the
> >>> expensive code walks down like this:
> >>> 
> >>> gnc_transaction_dom_tree_create
> >>> add_time64
> >>> time64_to_dom_tree
> >>> gnc_print_time64
> >>> GncDateTime::format
> >>> GncDateTimeImpl::format
> >> 
> >> Christian,
> >> 
> >> I don't have time to fully test it out right now, but give
> >> https://github.com/jralls/gnucash/tree/maint a go.
> >> 
> >> As noted in the HEAD commit it has a side effect of recording times in
> >> XML
> >> files in UTC, no timezones. Users who want to keep their files in version
> >> control will like that, but it needs to be checked for backward
> >> compatibility with 2.6 before it can be merged into maint.
> > 
> > Thanks for working at this. Indeed the main issue is that we don't need a
> > user-visible format conversion (which must respect the locale) but instead
> > a serialization to a fixed iso8601 format. Fixing this issue also means
> > that the XML files will no longer depend on the local timezone, which is
> > an issue that has been asked for a long time.
> > 
> > Thanks for your patch - this fixes performance issues with
> > gnc_time64_to_iso8601_buff() and all of its callers. However, the
> > Save-to-XML function calls gnc_print_time64() instead (from
> > time64_to_dom_tree), which is a different code path. Consequently, your
> > patch alone does not modify the saved XML (just verified). Your call to
> > GncDateTime::format_iso8601 needs to be used in gnc_print_time64, too,
> > like so:
> > 
> > xmlNodePtr
> > time64_to_dom_tree (const char* tag, const time64 time)
> > {
> > 
> >     xmlNodePtr ret;
> >     g_return_val_if_fail (time != INT64_MAX, NULL);
> > 
> > -    auto date_str = gnc_print_time64 (time, "%Y-%m-%d %H:%M:%S %q");
> > -    if (!date_str)
> > -        return NULL;
> > +    GncDateTime gncdt(time);
> > +    auto sstr = gncdt.format_iso8601();
> > +    gchar* date_str = g_strdup(sstr.c_str());
> > 
> > 
> > However, with this patch, all time64 timestamps in the XML file will then
> > change from time+timezone to UTC time only. To my surprise, although we
> > knew about this issue for such a long time and changed most timestamps
> > appropriately, there are still timestamps that will change date due to
> > this
> > change. With a quick glance, the "reconciled-date" timestamp contains
> > beginning-of-day in a local timezone. Also maybe user-entered commodity
> > prices. Others might still show up, oh well.
> > 
> > I haven't checked for backward compatibility with 2.6, but will do during
> > the next days.
> 
> Rats, I got the two functions scrambled.
> 
> Geert and I discussed a month or two ago expanding the use of time-neutral
> to everywhere that we currently use day-begin and day-end except searches,
> but neither of us has gotten to it yet.

For completeness: I checked loading such a file with gnucash-2.6, and as far 
as I can tell, this runs just fine. All transactions and their dates show up 
as usual even though the timestamps are now read as UTC instead of localtime.

Regards,
Christian