lazygit icon indicating copy to clipboard operation
lazygit copied to clipboard

Reduce memory consumption when loading large number of commits

Open stefanhaller opened this issue 2 years ago • 4 comments

When pressing > in the commits panel to trigger loading all the remaining commits past the initial 300, memory consumption is a pretty big problem for larger repositories.

The two main causes seem to be

  1. the cell memory from rendering the entire list of commits into the gocui view
  2. the pipe sets when git.log.showGraph is on

This PR addresses only the first of these problems, by not rendering the entire view, but only the visible portion of it. Since we already re-render the visible portion of the view on every layout call, this was relatively easy to do.

Below are some measurements for our repository at work (261.985 commits):

master this PR
without Graph 855 MB 360 MB
with Graph 3.1 GB 770 MB

And for the linux kernel repo (1.170.387 commits):

master this PR
without Graph 5.8 GB 1.2G
with Graph Killed by the OS after it reached 86.9 GB 39.9 GB

The measurements were taken after scrolling all the way down in the list of commits. They have to be taken with a grain of salt, as memory consumption fluctuates quite a bit in ways that I find hard to make sense of.

As you can see, there's more work to do to reduce the memory usage for the graph, but for our repo at work this PR makes it usable already, which it wasn't really before.

I'm setting this to draft for now, because the gocui changes would have to be vendored if we want to go with this approach.

stefanhaller avatar Mar 31 '23 12:03 stefanhaller

Uffizzi Ephemeral Environment deployment-20904

:cloud: https://app.uffizzi.com/github.com/jesseduffield/lazygit/pull/2533

:page_facing_up: View Application Logs etc.

What is Uffizzi? Learn more!

github-actions[bot] avatar Mar 31 '23 12:03 github-actions[bot]

Nice work. I agree with the approach, though there is a problem: our search functionality (invoked by pressing forward-slash) depends on the content of the view. With this PR, we're only be able to search for content in our commits view within the current page of content.

I spent a bit of time trying to improve our search functionality a while back (so that rather than searching the view we're actually filtering down the list) but I didn't get far enough to commit anything and now it's too divergent to resurrect. More recently however, I did successfully get list filtering working in lazydocker, which does not depend on the contents of the view. A couple relevant files: https://github.com/jesseduffield/lazydocker/blob/master/pkg/gui/panels/side_list_panel.go https://github.com/jesseduffield/lazydocker/blob/master/pkg/gui/panels/filtered_list.go

So I'm thinking we first implement that filtering functionality, then go ahead with this PR afterwards. FWIW I'd wanna merge my big refactor PR before any filtering functionality is added. What do you think?

jesseduffield avatar Apr 01 '23 00:04 jesseduffield

As for filtering in general: yes, filtering is one the things that I miss the most in lazygit, so it would be very much appreciated; however, I want this only in the branches list (and tags, maybe), but not in the commits list. For the commits list I'd prefer to not filter it down when searching; the reason is that when hopping from one search result to the next it helps me see the context of the commit I arrived at, e.g. see what branch it is a part of, and maybe type up/down arrow a bit to look at the surrounding commits too.

But I suppose searching could still be implemented in terms of the model data rather than the view content. I haven't looked at the searching code yet to get a feeling for how much work that would be; let me know if you think it would make sense for me to look in that direction, and whether that should be done before or after the big refactor.

stefanhaller avatar Apr 01 '23 08:04 stefanhaller

I'm on board with that approach. Feel free to look into how things are currently done (there's code related to searching in vendor/github.com/jesseduffield/gocui/view.go in the searcher struct. But yes, I'd want those changes to wait for the big refactor first

jesseduffield avatar Apr 02 '23 01:04 jesseduffield

Ah damn, this still has bugs. Setting back to draft for now.

stefanhaller avatar Jun 06 '24 16:06 stefanhaller

@jesseduffield The bug is fixed, this is back in review. There are several things here that I'm not very happy with, curious what you think about these.

stefanhaller avatar Jun 06 '24 19:06 stefanhaller