[GNC-dev] Slow user interface in 3.x gnucash (for large files), and potential optimization

[John Ralls]

> On Jun 12, 2018, at 12:57 PM, Christian Stimming <christian at cstimming.de> wrote:
> 
> Dear all,
> 
> as discussed before, I am only recently starting to use the 3.x series of 
> gnucash in daily production work. However, there are still some issues that 
> keep bugging me, compared to the 2.6.x version of gnucash.
> 
> One of the issues is that the user interface in the register is surprisingly 
> slower compared to 2.6.x. I have a somewhat large data file [1], so I saw some 
> behaviour that is probably not the usual case. The slowness is that when 
> activating the auto-complete by typing the first letter in the txn 
> description, then pressing <tab>, there is a delay of approx. 1.5 seconds with 
> 100% CPU until the cursor appears in the next field and the data is filled in. 
> This delay is quite recognizable and it is annoying. In previous gnucash 
> versions, it was not there.
> 
> Hence I tried to narrow down the cause of this slowness using 
> valgrind/callgrind with switching on and off the instrumentation just around 
> this very keypress using callgrind_control. In the result, I recognized a 
> surprising high number of std::string constructor calls and similarly 
> destructor calls - which, for this single lookup operation, looked suspicious 
> to me.
> 
> Turns out that the std::string constructor and destructor is used a lot when 
> calling qof_instance_get_path_kvp() with a std::vector<std::string>, but the 
> std::vector is created on-the-fly using the { } syntax, AND (this is the 
> expensive part) all the std::string's inside that vector are also created on-
> the-fly from string literals that are expanded from #defines. It occurred to 
> me that this last construction/destruction can be avoided rather easily: Why 
> don't we declare std::string constants and use those, instead of the const 
> char* literals expanded from the #defines? I implemented a very simple test of 
> this idea, see
> https://github.com/cstim/gnucash/commit/99c97736dda4afb615aad4b70bd5b7bd761af459
> 
> Testing this change with callgrind again brought down the instruction count of 
> the described operation in the user interface from 3.2e9 to 2.6e9, which in my 
> opinion is a significant optimization. What do you think - would it be a good 
> idea to use kind of construct (no pun intended) in this and other places? Or 
> did I miss something here?

Christian,

A very good catch indeed. But pre-constructing the string in qofbook.cpp only saves two string constructions per invocation as the vector still has to make its own copies. I guess that its much worse for you because the ancient gcc on Ubuntu 14.04 (gcc4.8) doesn't do small-string optimization.

KVP paths aren't really a good use for std::string anyway: It's a lot of weight for something that's used as a human-readable index. Even static char[] is heavy for this purpose. We could get a huge boost if we use something like Glib's quarks [1]. They can be expanded to their string value for storage so that we don't break file/db compatibility.

Want to have a go at that?


[1] https://developer.gnome.org/glib/stable/glib-Quarks.html