search

lem-project / lem

Common Lisp editor/IDE with high expansibility
http://lem-project.github.io/
MIT License
2.4k stars 180 forks source link

Memory leak in SDL2 build #1364

Open sbenitezb opened 6 months ago

sbenitezb commented 6 months ago

I compiled Lem from source on macOS/ARM and there appears to be a memory leak, as the running image grows continuously above 1GB.

BierLiebHaber commented 6 months ago

Does it grow on it's own? I just tried it and for me it only grows when I scroll in a text buffer. According to (room) lisp doesn't know about the growth. I'm guessing some of the textures aren't being freed properly.

sbenitezb commented 6 months ago

I left it over night and didn't grow a lot, to 1GB. But if I scroll, yeah it jumps by around 100MB. Most of it seems to be released often, but not all, and so it will start accumulating over time.

BierLiebHaber commented 6 months ago

It might not be a leak after all. 🤔 Can you try (lem-sdl2/text-surface-cache:clear-text-surface-cache) when the memory usage is high? For me it reclaimed most of the memory. Edit: I tried again after a while and it doesn't seem to reclaim all of it

BierLiebHaber commented 6 months ago

I managed to get memory usage up to 5GB by opening micros.lisp then pressing and holding the right arrow key until I reached the end of the file.

BierLiebHaber commented 6 months ago

After disabling the text-surface-cache I retried moving through micros.lisp and the memory footprint stayed below 600MB. I disabled the cache by adding #+() to https://github.com/lem-project/lem/blob/60a8d57a676b487699ef6147cf0cff6e67800863/frontends/sdl2/text-surface-cache.lisp#L25-L29

What's the reason for the surface-cache anyways? Rendering doesn't seem slower without it and it causes problems when it gets really big (backspace with a big cache feels unresponsive). At the very least there should be something cleaning up old entries so it doesn't grow to a ridiculous size.

Edit: interestingly clearing the cache while going though the file doesn't seem to have the same effect

cxxxr commented 6 months ago

Thank you for your research. It is true that this cache is not very useful. I remember that when I was drawing characters one by one, the process of creating the surface was very slow and I implemented this cache. But, the current situation is that the surface is created in larger units, so the effect may be lost.

BierLiebHaber commented 6 months ago

according to #1255 we are still creating 1 surface per character if line-wrapping is disabled, correct? If that's the case, then cache is probably still useful.

A compromise might be to either only enable the cache if line-wrapping is disabled, or only enable the cache for small strings.

cxxxr commented 4 months ago

realted issue: https://github.com/lem-project/lem/issues/1332#issue-2177441613