[GNC-dev] More need for speed improvement around XML (Re: Introduction, a story, and 50% improvement in XML loading speed)

[John Ralls]

> On Dec 28, 2018, at 2:14 PM, Christian Stimming <christian at cstimming.de> wrote:
> 
> Am Montag, 24. Dezember 2018, 13:38:27 CET schrieb Chris Carson:
>> TL;DR:  hi!  I'm a programmer!  The attached patch to one line of code
>> gives a 50% reduction in XML load CPU use!  Skeptical?  I was.
> 
> Dear Chris,
> 
> thanks a lot for this patch! I'm using gnucash since many years, too, and I'm 
> still somewhat unhappy with some of the reaction times in the 3.x series of 
> gnucash. 
> 
> Now that you got a good catch on the XML loading time, maybe you are up for 
> some improvements on the XML saving time...? :-)  I ran some valgrind where I 
> could locate an expensive code section, but I'm a bit out of practice to know 
> the C++11 way of writing that code in a more efficient way. Here's what I 
> found:
> 
> 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
> 
> and this very last function seems significantly more expensive than needed, 
> slowing down saving considerably:
> 
> std::string
> GncDateTimeImpl::format(const char* format) const
> {
>    using Facet = boost::local_time::local_time_facet;
>    std::stringstream ss;
>    //The stream destructor frees the facet, so it must be heap-allocated.
>    auto output_facet(new Facet(normalize_format(format).c_str()));
>    // FIXME Rather than imbueing a locale below we probably should set 
> std::locale::global appropriately somewhere.
>    ss.imbue(std::locale(std::locale(""), output_facet));
>    ss << m_time;
>    return ss.str();
> }
> 
> Both the operator<< and the constructor std::locale(const char*) seem to be 
> quite costly operations. To me, it seems there is way too much construction/
> destruction going on for options that are constant during each saving 
> operation. Maybe even the recent commit 7f1a7115 (Frank?) has slowed down 
> things here even more. Maybe this could be checked in some of the unittests 
> more easily, but I'm a bit out of practice there, too. Maybe someone is up for 
> some performance boosting here...? Thanks a lot in advance!

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.

Regards,
John Ralls