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

[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. 

Regards,
John Ralls