Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimization request: initialization of CardView lacks feedback #192

Open
emyoulation opened this issue Jun 10, 2022 · 16 comments
Open

Optimization request: initialization of CardView lacks feedback #192

emyoulation opened this issue Jun 10, 2022 · 16 comments

Comments

@emyoulation
Copy link

The 1st time a CardView mode is activated after loading a large tree, there is significant delay before the Gramps app begins a redraw.

This gives the impression that the mode change has been ignored. Impatient users may select another mode instead of allowing the view to initialize.

There should be some sort of indication that "stuff" is being populated. Maybe the Statusbar could have a initializing message or a progress-bar to avoid the "not responding" automatically appearing in the Titlebar after 5 seconds. (Is that related to gsettings set org.gnome.mutter check-alive-timeout ?)

Ideally, maybe threaded CardView housekeeping task could be assigned to an idle core of a multi-core processor when a new tree is loaded? The high overhead could be pre-processed on a low priority and that initial delay would disappear unless a CardView was the active view mode during a tree load.

Note:
Dunno if this stackoverflow thread answer has any bearing:
You can use gobject.idle_add to have a function be executed and still keep your windows responsive

@cdhorn
Copy link
Owner

cdhorn commented Jun 10, 2022

Without LastChanged Gramplet? That one even with yeild and large trees is problematic.

The Gramplet rendering logic and I think status bar updates leverage idle_add as does my deferred config update logic.

Really need gramps-project/gramps#1119 first to think about how might use threads and if makes sense as Gramps was never designed with that in mind.

@emyoulation
Copy link
Author

Serge added 2 Gramplet freezing experiments.
https://gramps-project.org/bugs/view.php?id=0010850

Perhaps the Last Modified Gramplet could be included by default but frozen with a defrosting hint watermark?

Not only is Gramps not designed with threading in mind, neither was Python. It won't be until October that a fully multi-threading Python will be release. And an unknown number of years until Gramps is rev'd to use that prerequisite version.

A threading functionality would probably need to be reversed when that is Python version is required. (Hopefully, the bottombar & sidebar container will direct their contents to automatically use different cores from the main view & GUI.)

@fxtmtrsine
Copy link

The 1st time a CardView mode is activated after loading a large tree, there is significant delay before the Gramps app begins a redraw.

Even with the fix from #184 (thank you) I ended up removing the Gramplet permanently (was waiting 17 seconds between view changes!), my suggestion is don't install it by default to give a better experience overall for people with larger trees?

Is it possible to cache the results between Gramps sessions ( exiting totally and opening/closing different family trees ) as the Gramplet seems to rescans each time?

@cdhorn
Copy link
Owner

cdhorn commented Jun 11, 2022

Not only is Gramps not designed with threading in mind, neither was Python.

The GIL limits things, but there are lots of scenarios where it is more than fine. It all depends on what you need to do.

Lots of big projects reach certain points where the weight of the technical debt needs to be considered and how best to deal with it. Do you keep trudging along? Do you try to refactor enough of it to keep things moving forward without breaking compatibility? Or is it time to set the codebase aside, although maybe cherry picking it, to write something new from scratch taking all the lessons learned with you to start with a more solid foundation?

If the Gramps team was to do it all over again, with all the existing experience behind them, what would everyone do differently?

my suggestion is don't install it by default

Yes I agree I should probably remove it as a default and will soon. It can be useful, but with large trees it can be pretty painful on startup or if it needs to rescan.

It would be possible to stash the results and pull them in again, but then one has to worry about stuff getting out of sync with the db and not being sure although I do force rescan on delete if I recall. Maybe with last db change time can work around some of that uncertainty and if I kept a deeper history I'd not worry about rescan on delete. If time permits I might look at it.

@emyoulation
Copy link
Author

The dashboard locked up Gramps for 76 seconds while gathering statistics when switching trees from the Example.gramps tree to my 45,000 person tree. During the time it was gathering data, the stats from the previous tree remained visible.

The GUI was completely unavailable. I could not interrupt to switch to another view or even turn off the sidebar/Bottombar.

It needs so interrupts so it plays nice. Or at least a 'busy'.

Maybe it could zero the stats before starting to gather?

Another progressbar-like option might be to make the tile section headers all have red backgrounds before their stats are gathered. And, as each section is gathered, don't populate the values, merely change the background to normal.

@cdhorn
Copy link
Owner

cdhorn commented Jun 17, 2022

Yes I am painfully aware it is an issue, no different than the last changed Gramplet.

I am spending a lot of time thinking about how to best handle this.

Mind my asking how many events, citations and notes you have in your big tree?

@emyoulation
Copy link
Author

emyoulation commented Jun 17, 2022

This session: (Need to download a new version and see if the background color of the Bookmark statistics column initialized to other than white.)
render_page: dashboard 54.49921631813049
image

@emyoulation
Copy link
Author

in Themes Adwaita: Dark Variant & Preferences Colors tab's Color scheme: Dark colors

image

@cdhorn
Copy link
Owner

cdhorn commented Jun 17, 2022

Yeah that's the #199 issue. I need to spin up the Windows VM to look at that at some point. It isn't happening for me on Fedora.

@PQYPLZXHGF
Copy link

Yes I am painfully aware it is an issue, no different than the last changed Gramplet.

I am spending a lot of time thinking about how to best handle this.

I know it is not a Gramplet but maybe consider doing something similar to how minimizing a Gramplet will prevent it from updating. (force_update: set True to have the gramplet update, even when minimized) and then by default have almost all of the statistics sections closed and users could show all or hide all like others mention and have card view remember between sessions what the user wants? So opening the section would start the information gathering of that type and you can have a timer icon or word message showing its in progress?

@cdhorn
Copy link
Owner

cdhorn commented Jun 21, 2022

I have yet to look at the UI side of this but think I have a better understanding of how to go about things there now.

On the back side where I have been experimenting a bit it is an interesting dilemma. On my large tree that takes 12 seconds on average to process I have managed to cut it down to 6 seconds. This uses a thread to spawn a separate tool that in turn uses the multiprocessing module to spawn a process per object type with logic that side steps the lock file so that they can all query the database concurrently. This seems fine to me as it is just concurrent reads. I had to take this round about approach as if trying to leverage the multiprocessing module within Gramps I ended up with a GUI window launching per subprocess and it was an ugly mess.

While this sounds great, the overhead of starting the Python interpreter is pretty hefty at about .65-.75 seconds and then of spawning the workers another .3-.4 seconds after that. This is longer than the .7 seconds it takes to process and render example.gramps inline on my machine. I also suspect the overhead on Windows will be even worse. So for large trees there is a benefit, but for smaller ones it actually slows things down, taking a little more than twice as long for example.gramps. This on a 4 core / 8 thread Ryzen 5 machine with NVME backing store.

I'm unsure what makes the most sense to do here, or how this sort of approach will behave on other machines and particularly non-Linux operating systems.

I also don't want to trigger a full rescan to update this every time a db change is done as that is a lot of overhead. I was thinking perhaps load it on startup but then provide a refresh button instead of trying to update it dynamically all the time. But then I was hoping to leverage the last changed statistics as this gathers that in the same pass, and that we would want to try to keep current and really no reason for the Gramplet and this to both iterate across the database separately.

@emyoulation
Copy link
Author

I think that the big question is: even if the Gramplet takes a bit longer on smaller trees, does the main window gain in responsiveness? The latency due to overhead of the Gramplets tends to make the main view unusable if it is processor intensive too.

@cdhorn
Copy link
Owner

cdhorn commented Jun 21, 2022

This is the view, not the Gramplet, but in both cases it is the same issue.

Threading is a natural way to deal with the UI responsiveness. A thread is dispatched to do the work and the main UI thread can continue to run it's event loop without blocking.

In all of Gramps there is only a single peice of code that runs in a separate thread in gramps.gui.utils:

class AvailableUpdates(threading.Thread):
    def __init__(self, uistate):
        threading.Thread.__init__(self)
        self.uistate = uistate
        self.addon_update_list = []

    def emit_update_available(self):
        self.uistate.emit('update-available', (self.addon_update_list, ))

    def run(self):
        self.addon_update_list = available_updates()
        if len(self.addon_update_list) > 0:
            GLib.idle_add(self.emit_update_available)

I guess this is probably because of all the technical debt around the Berkley database.

Note in my large database that takes 12 seconds on average to render I ran some experiments. I found it took 5-5.5 seconds to read the data in from the database and instantiate the objects without doing any actual work. It took 1.2 seconds to read the raw objects in, but that includes unpickling the serialized data. When I examined that I found it took .8 seconds to either pickle or unpickle the whole data set, so it actually only took .4 seconds to read through the entire database.

So almost half the processing time is spent doing nothing meaningful with the actual data.

@cdhorn
Copy link
Owner

cdhorn commented Jun 25, 2022

I guess this subprocess/multiprocessing approach is not going to work with the Windows AIO package. It seems to use a launcher of some sort and doesn't actually have a python executible I can call to run something externally.

@emyoulation
Copy link
Author

Well, it was a slim hope and thanks for exploring it.

Hopefully it works for your Linux installation though?

@cdhorn
Copy link
Owner

cdhorn commented Jun 26, 2022

Collection is done in a separate thread now and cards updated when statistics are ready. Quick test on Windows VM it seems to work okay.

There is much left to be done with this. ie refreshing stats, handling close db properly when thread collecting data, update last changed gramplet, and stuff like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants