fix(performance): plug memory leak

[imsnif]

Fixes #1625

For some reason (which I admittedly cannot fully explain) when we pre-allocate memory for the terminal viewport, scrollback and each line therein that memory is not deallocated when its container is dropped.

Removing this pre-allocation doesn't harm performance in a noticeable way and plugs this leak. If any Rustacean reading this has ideas for why this is happening, I'd be very curious to hear them. :)

[raphCode]

I would not consider myself a Rustacean, but it seems to me all this preallocation finally happens in the standard library, right?
Under the assumption that the stdlib knows how to clean up its (pre-allocated memory) when told to do so, I too do not understand how removing preallocation might fix the leak.
My fear at this point is that the preallocation maybe just exaggerated any leaks to the point where they became obvious?

[imsnif]

My fear at this point is that the preallocation maybe just exaggerated any leaks to the point where they became obvious?

That's what I thought as well, but allocating the same count of items by filling the buffer did not even make a dent in the resident memory utilization.

My best guess right now is that this is some combination of the elements being drained and the drain iterator leaking the memory (intentionally or unintentionally), or not being dropped itself or whatnot.

[imsnif]

Also, cool to see you're still around @raphCode :)

[raphCode]

That's what I thought as well, but allocating the same count of items by filling the buffer did not even make a dent in the resident memory utilization.

Alright, Fingers crossed that it helps!
Nonetheless it might be worth keeping the discrepancy in mind if leaks resurface.

Also, cool to see you're still around @raphCode :)

Of course :) Just too much to do lately sadly...
I got a ping on the memory leak issue that got closed by this PR, and I got curious how it was solved.

[mo8it]

@imsnif This issue is not solved. I think that only a symptom was hidden when testing with something like yes because it prints only one character y by default. By replacing with_capacity with new and then pushing y, we allocate less memory which means that we leak less memory afterwards, but this does not mean that we don't leak memory at all.

If you run something like

yes -- "-----------------------------------------------------------------------------------------------------------------------------------------------"

in multiple panes on the main branch (I did it with cargo xtask run) and then create a new clean pane and close all others, you will see the memory leak. For me, the leak was about 3.5 GB for about 10 panes with a scroll back max of 20000 lines.

See the reply by @imsnif below ❤️

[raphCode]

The whole preallocation discussion made me think... maybe it can be useful for hunting down which collection exactly is leaked and when?
Deliberately using a huge preallocation in specific places should reveal itself in the memory usage.

[imsnif]

@mo8it - thanks for chiming in. For your test case with yes, please see the issue comment here: #1625 (comment)

In short - I don't think that's a memory leak, but just the default rust linux allocator doing its thing. Just to make sure, I tried reproducing this and did not see consistent memory climbs (please be sure to test this in only one pane, otherwise it can very easily be mistaken for the aforementioned allocator behavior).

I am happy to be wrong, and I'm happy to keep working on this, but I ask - nay, beg - of you: please be very careful what you write about this issue. I have personally spent a growing number of hours debugging it and chasing inconsistent ghosts only to find out they are expected behavior (unless I want to switch allocators which I would really rather not).

When people write stuff like in the comments above, others do a github search because they misinterpret some memory diagnostic, find these issues/PRs and after a cursory read conclude that "Zellij has a memory leak, I will not use it" - which in turn means more hours of chasing ghosts for me :) (mostly because I am a penchant for accuracy, less because I care if people use my software).

So again - I am very happy for the help, but please point to specific places in the code base if you think they leak memory, or to a consistent and significant climb in one pane (in this case) as was pointed out in the btop issue in the original thread.

As a hint to get you started, this is where the rows are Droped:

zellij/zellij-server/src/panes/grid.rs

Line 187 in c3e140c

let dropped_line_width = bounded_push(lines_above, sixel_grid, next_lines.remove(0));

When rendering, they also go into our Output buffer and are ultimately dropped when it goes out of scope at the end of this function hree:

zellij/zellij-server/src/screen.rs

Line 961 in c3e140c

let mut output = Output::new(

Thanks again, and for those tuning in: for the time being - no additional leaks have been reliably reproduced.

[imsnif]

Also, a really nice way of seeing the allocator in action is to use @tlinford 's jemalloc branch (or just to copy/paste the few relevant lines from the diff): main...jemalloc

This allocator deallocates much much faster and one can consistently see it with the yes test case: main...jemalloc

[imsnif]

Also also, regarding the original conundrum on the with_capacity causing the leak, I strongly suspect this iterator here:

zellij/zellij-server/src/output/mod.rs

Line 907 in c3e140c

let mut terminal_characters: Vec<TerminalCharacter> = row.columns.iter().copied().collect();

Maybe when doing this on a preallocated vec, some issue arises. Not sure. I'm moving on to other things, if someone wants to dive deeper into this and finds something I missed or an explanation for our riddle - I would be super happy for a ping.

[mo8it]

I am very sorry @imsnif for the too quick, wrong conclusion. I did edit my comment above.
Thank you a lot, not only for working on this exhausting issue, but of course for Zellij as a whole ❤️

[imsnif]

No worries, @mo8it ! Thanks for chiming in and keeping me on my toes :) If you do end up uncovering something I've missed I'd be very happy to know about it.

[yshui]

Sorry for barging in, but could it be related to this? https://blog.polybdenum.com/2024/01/17/identifying-the-collect-vec-memory-leak-footgun.html