In the "view lots" dialog, when I click on a split and then click ">>" to add it to the lot, it takes a few seconds (on a fast late-2019 Macbook Pro) to move the split over. If I click in the splits list, then ">>", then list, etc multiple times, I can easily get the UI to freeze up for quite a while. This is happening even for accounts with just a few (4-5) splits in them. I profiled the UI freeze after clicking a bunch of times with Instruments. The 19s of main-thread CPU time breaks down as follows. Each line shows total time under function, then time in the function itself, if significant (I left out all the small ones), then the function name: 19.08s lv_add_split_to_lot_cb 13.32s gnc_lot_add_split 13.20s xaccTransCommitEdit 13.20s qof_commit_edit_part2 13.20s trans_cleanup_commit 12.99s qof_event_generate_internal(QofInstance_s*, int, void*) 12.96s gnc_gui_refresh_internal 12.37s refresh_handler 11.25s qof_query_run_internal(_QofQuery*, void (*)(_QofQueryCB*, void*), void*) 10.86s qof_query_run_cb(_QofQueryCB*, void*) 10.86s qof_collection_foreach 7.12s 1.48s g_list_foreach 5.60s 308ms check_item_cb(void*, void*) 3.49s xaccSplitGetBook 3.43s g_type_check_instance_cast 1.23s 418ms guid_match_predicate(void*, _QofParam*, _QofQueryPredData*) 1.88s g_hash_table_get_values 1.78s g_list_prepend 1.67s 1.45s g_slice_alloc 1.84s 1.81s g_slice_free_chain_with_offset 5.64s qof_commit_edit_part2 5.64s qof_event_generate_internal(QofInstance_s*, int, void*) 5.62s gnc_gui_refresh_internal 5.62s refresh_handler 5.09s qof_query_run_internal(_QofQuery*, void (*)(_QofQueryCB*, void*), void*) 4.91s qof_query_run_cb(_QofQueryCB*, void*) 4.91s qof_collection_foreach 3.24s 700ms g_list_foreach 2.53s check_item_cb(void*, void*) 1.56s xaccSplitGetBook 1.53s g_type_check_instance_cast 562.00ms guid_match_predicate(void*, _QofParam*, _QofQueryPredData*) If I invert the tree, so I can see the functions in which time is largely being spent (_in_, not _under_, unlike the previous data), I see: 5.12s g_type_check_instance_cast 2.68s g_slice_free_chain_with_offset 2.20s g_slice_alloc 2.18s g_list_foreach 600.00ms guid_match_predicate(void*, _QofParam*, _QofQueryPredData*) 515.00ms g_type_check_instance_is_a 472.00ms check_item_cb(void*, void*) 421.00ms qof_instance_get_guid 404.00ms g_type_instance_get_private 272.00ms qof_instance_get_book The book in question does have a lot of price history, in case that matters...
The hot-branch list in Instruments can be quite misleading and the inverted tree even more so; that seems to be the case here. Can you save the Instruments session and attach it here?
Created attachment 373933 [details] Instruments trace Here you go.
Comment on attachment 373933 [details] Instruments trace I can't open this in Big Sur's Instruments, nothing happens. What version of macOS are you using?
I'm on Catalina. Using Instruments 12.2, in case that matters. Just to make sure we're on the same page, here are the steps that work for me to open it in Instruments 12.2 (but on another Catalina machine): 1) Download that file. 2) tar xf Gnucash-slow-lots.tar.gz 3) Open Instruments 4) Cmd+O 5) Select the "Gnucash-slow-lots.trace" dir in the filepicker and click "Open".
Xcode 12.2's instruments had no trouble with it. The problem is that every commit of a QofInstance subclass (including split, transaction, and account, all of which are committed in lv_add_split_to_lot_cb) causes a register refresh which re-queries for the register contents. We don't need to call it 3 times (arguably we don't need to call it at all, but changing that would entail major surgery) so we can get an easy 3x improvement by suppressing the refresh until after the outer commit. Are you able to build GnuCash from source to test a fix?
Hmm. The last time I tried building on Mac it was pretty painful, starting with "create a separate account to do it under". I can try again, I guess... Are https://wiki.gnucash.org/wiki/MacOS/Quartz#Preliminaries and following the relevant directions?
Created attachment 373934 [details] Patch to refresh the register only once per split insertion. That's the way *I* do it, but I've made an easier and much faster way for the macOS CI builds: * Create a directory /Users/runner/gnucash and make your normal id the owner. cd to it. * Create a subdir inst. * Download https://downloads.sourceforge.net/gnucash/Dependencies/gnucash-4.x-mac-dependencies.tar.xz and untar it. * Create subdirectory 'source' and cd into it. * Download https://github.com/google/googletest/archive/release-1.10.0.tar.gz and untar it. Rename googletest-1.10.0 to googletest. * Clone the gnucash sources. Apply the patch. * Go back to the base dir (/Users/runner/gnucash), create build/gnucash and cd to it. * Config the build: cmake -GNinja -DCMAKE_INSTALL_PREFIX=/Users/runner/gnucash/inst -DCMAKE_PREFIX_PATH=/Users/runner/gnucash/inst -DGTEST_ROOT=/Users/runner/gnucash/source/googletest /Users/runner/gnucash/source/gnucash * Build: ninja You don't need to install, just run bin/gnucash and point Instruments at the running instance.
John, I finally got a chance to give that a shot; sorry for the lag. The binary I ended up with fails with a GTK assertion on startup, like so: (gnucash:17456): Gtk-WARNING **: 21:26:46.464: Could not load a pixbuf from /org/gtk/libgtk/icons/32x32/status/image-missing.png. This may indicate that pixbuf loaders or the mime database could not be found. ** Gtk:ERROR:/Users/john/gnucash/src/gtk+-3.24.20/gtk/gtkiconhelper.c:494:ensure_surface_for_gicon: assertion failed (error == NULL): Failed to load /org/gtk/libgtk/icons/16x16/status/image-missing.png: Unrecognized image file format (gdk-pixbuf-error-quark, 3) Bail out! Gtk:ERROR:/Users/john/gnucash/src/gtk+-3.24.20/gtk/gtkiconhelper.c:494:ensure_surface_for_gicon: assertion failed (error == NULL): Failed to load /org/gtk/libgtk/icons/16x16/status/image-missing.png: Unrecognized image file format (gdk-pixbuf-error-quark, 3) Any idea what's going on there? I don't have an /org/gtk/libgtk directory, obviously.....
If /Users/runner/gnucash/bin/gdk-pixbuf-query-loaders exists you can run gdk-pixbuf-query-loaders --update-cache and that will probably fix the problem.
Unfortunately, it doesn't look like gdk-pixbuf-query-loaders exists in any of the stuff I downloaded per comment 7. And if I try to copy it from /opt/local/bin and run it, it tries to write to write to /opt/local/lib, which doesn't seem helpful.... (I did that anyway, with sudo, but it did not help).
If you've got MacPorts why don't you just use it to build GnuCash?
Try (all on one line) GDK_PIXBUF_MODULEDIR=/Users/runner/gnucash/lib/gdk-pixbuf-2.0/2.10.0/loaders GDK_PIXBUF_MODULE_FILE=/Users/runner/gnucash/lib/gdk-pixbuf-2.0/2.10.0/loaders.conf /opt/local/bin/gdk-pixbuf-query-loaders --update-cache
> If you've got MacPorts why don't you just use it to build GnuCash? Good question. I got this working, and tested the patch this way. It does seem to help! Now each press on the ">>" button is ~1s instead of multiple seconds. For what it's worth, there was no lib/gdk-pixbuf-2.0/2.10.0/loaders anywhere in the things that comment 7 installed either....
Great. Several x improvement is good but ISTM 1s is still kind of long. Can you make a new profile? As for loaders, I guess it's not too surprising; they're not needed for running the unit tests.
Created attachment 373971 [details] Instruments trace with the patch applied
Note that the new trace is against the MacPorts-compiled version, which is using X, not Quartz, as its rendering backend, afaict. That said, the general shape of the profile is likely to still be useful.... Lots of time under qof_query_run_internal.
Sorry for the delay. I finally had a look at the profile. It looks like prevents 6s of calls to gnc_refresh_internal but there are still 14s of them remaining, but oddly called directly from lv_add_split_to_lot_cb, which it doesn't do. Maybe you should undo the patch and build again. The speed gain might be because of something different about the MacPorts build rather than the patch.
For what it's worth, I did test both with and without the patch back at comment 13 and with the patch is definitely faster visually. The total profile times are somewhat arbitrary: I just started Instruments, then did some clicking, then stopped instruments. So the only thing that can reasonably be compared across the two profiles is the percentages... As far as gnc_refresh_internal being called from lv_add_split_to_lot_cb, the latter does call gnc_resume_gui_refresh and that calls gnc_refresh_internal. If gnc_resume_gui_refresh is getting inlined, or if the call from gnc_resume_gui_refresh to gnc_refresh_internal gets compiled as a tail call, that would explain the profile...
I dunno, when I build it with -O3 gnc_resume_gui_refresh isn't inlined, but maybe it worked out differently for you. No matter, that's very likely what's calling gnc_refresh_internal. It's not a modal dialog box so ISTM that's the best we can do: Users would find it very frustrating to not be able to edit transactions just because the lot viewer is open.
Makes sense. Fwiw, I think the "real" fix here would be fixing bug 798042: then I could select the splits I want to add, add them all as a single click of the button, with one UI update to go with it, and things would be great.
And thank you very much for the fix here, to be clear! I really appreciate it.
Actually the real fix is to replace QofQuery with SQL query (to an in-memory SQLite3 DB for XML users). Nothing against your enhancement request in Bug 798042, it's just not something I want to prioritize ATM. Maybe Bob-IT can have a look.
