-
-
Notifications
You must be signed in to change notification settings - Fork 167
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
[Bug Report][GNU/Linux] Memory usage not freeing with dispose #68
Comments
You can see the attached video in README showing test application running on Windows. Flutter shell or Dart VM for GNU/Linux is underperforming. There are many other issues as well. For instance: #64. |
Not able to figure this one out. Memory leak still happening. What I tried :-
|
I have experimented with it a lot before. Think yourself, if same code is working correctly on Windows & macOS (memory gets free'd as intended). Certainly, something is wrong within the Flutter embedder or Dart VM for GNU/Linux. At least how it treats memory or garbage collects it. There are no memory leaks in my code. IIRC even if you don't create any |
It's happening in Windows too unfortunately. Here's a demo that loops on a test video - but initializes & destroys player every time - demo It surely doesn't crash as fast as in linux. Takes a bit longer to pile up than linux. For me, win 10 - 2gb gpu, it took about 7 mins to go from 120 mb to 500 mb. |
Let me correct it:
memory.mp4It's more as to how Dart VM treats the memory, how OS schedules things etc. You can see that memory is freed automatically sometime afterwards, it can be: Dart VM itself, libmpv internals e.g. demuxer cache, FFmpeg internals, Windows scheduler etc. If you leave it suspended for more time, the memory drops further down. Note that mpv actually occupies space for cache in memory which is necessary for smooth playback of any kind. You can adjust it with But yeah... GNU/Linux situation is serious. The pool based solution should still be helpful. Who knows... what if Google Chrome also recycles same instances once created. Realistically, you are never going to have more than 5 at once. |
I have a good news & a bad news. Good: The memory clean-up has been further improved for all platforms. |
I have spent about 20-25 hours on this in the past months... and am still clueless. Tried so many combinations of flags. Not sure if it's mpv core or if it's gnu or if something else. Definitely nothing to do with media_kit code... i think, not sure if something weird happens over texture / angle. Will look into it again when I get the time. Leaving videos running in the background for long durations is critical for my case (digital signage player). |
Unfortunately this happened in IOS also, here is the log i got when app crashed |
How sure are you that's not a bug in your code? Can't really help against vaguely said statements. We require clean minimal code that can be used to reproduce the issue. In either case, using existing |
Im disposing the player properly before assing the new player instance. |
Hello. For people experiencing this on Linux (@femalemonkeyman , @srix55 ) This is fixed for next release and for 2.37 but most distributions does not ship this patch yet. Using glibc 2.37, I have a huge memory leak when lauching the multiple reader test case. I go to 30G in about 2minutes. This memory is never reclaimed when going back to menu and waiting. Tried the test example with mimalloc and didn't experience the leak. Memory raise a bit while playing videos but reduce when going back to the menu. |
Hi @ndusart, Your comment & the bug-report you linked is quite interesting. The memory issue is solely present on GNU/Linux & seemingly memory never gets freed. You can see the video I attached above (Windows), where memory gets freed as intended. It has been further improved after the latest updates, it's been 5 months since then. The project will be greatly helped with any workaround or fixes. We currently bundle I'm really curious to know how you tested our example application with mimalloc (updating native video rendering code? updating package:ffi I have no hate for GNU/Linux, but there are many issues, it's like nothing is stable & the quality is also not optimal. Few issues other than this still remain:
|
@ndusart Arch glibc is at 2.37-3 which is where I am |
I just tried out |
You figured that out already it seemed. libc memory allocator can be overriden thanks to @femalemonkeyman Yes I am on arch too and the patch has not been landed yet. This can explain part of the issue. @alexmercerind I hope this is really the issue here, I thought of testing it because I had similar issues in some of projects at work. But @srix55 seem to have some memory allocation issue on older glibc too. Maybe the glibc 2.37 behavior also added another memory issue on top of another one. But still, the stress test is raising indefinitely in memory for whatever memory allocator I use. I do not know if it is normal (that is still a stress test after all ^^). I tried to take a look at the code of |
Hi @ndusart,
Yeah! I used it later (made comment early in excitement ^^).
No, it's not normal. It doesn't happen on other platforms (Windows, Android, macOS & iOS). For the note, I'm still going to blame glibc & it is only it's fault. Same implementation works 100% correctly & does not leak on other platforms. I had this memory issue on GNU/Linux long before I implemented video rendering (FYI: package:media_kit was created to power Harmonoid ~ a music player; no video rendering. You can still see me getting bashed in the issues for performance on GNU/Linux). The memory used to leak even when there was no video embedding. I honestly didn't know that it's this easy to switch allocators ^^ (& if it was even possible). |
Now, for the good-news:
So, actual leak was fixed by switching to mimalloc. First of all, I don't have experience in GTK/GObject etc. I've mostly worked with Win32 & standard C++. In current implementation, I followed the recommended "GObject practices" as good as I could. Still... I'm not very familiar with it. It turns out I was leaking a new instance of I fixed this in: #319 🎉 Now a good question may be, why was memory growing in stress-test if it's only in software rendering, aren't we using hardware rendering by default? Like I said in the comment above:
When multiple |
Now it's not leaking at all. |
Screencast.from.04-08-23.08.50.18.PM.IST.webmYou are right. This seems to fix the leak :) Thanks @alexmercerind & @ndusart . I was testing it the whole day. (Although my application still leaks - i think it's because of the pre-fix myriad optimizations that are getting in the way). I did an isolated test with 6 concurrent players disposing & playing short videos at random. Doesn't seem to leak memory. 🎈 🥳 🎈 |
@srix55 the CPU usage should be 0.0% ideally. #329 will reduce the CPU usage by large margin (like in the earlier versions). Now multiple videos use hardware decoding correctly & don't fallback to software decoding. That 0.0% only works on NVIDIA devices only for now (through NVDEC). VAAPI unfortunately will only work with -copy on GTK 3.0. On my entry-level Ryzen 3 2200 U with integrated Radeon Vega 3, I'm able to play around 20 HD videos at once without any lag. Multiple 4K/8K 60 FPS also work as intended. The performance should be higher on capable devices. |
A final note for the future visitors: Memory leaks on GNU/Linux have been resolved & it is recommended to use mimalloc in your project: Refer to "Utilize mimalloc" section in the README. You just need to add one line in your target_link_libraries(${BINARY_NAME} PRIVATE ${MIMALLOC_LIB}) I tried to eliminate this requirement of manual modification from developer's end, but it's not possible. |
It'd be good idea to let glibc's developers know what's wrong. |
I' just noticing that the memory taken up by both my own and the test application on linux (I have not tested other platforms) is never clearing. It is fine to begin with but eventually does get out of hand. I looked a bit and didn't really see an issue with flutter itself pertaining to this so i was just wondering if there was any insight.
The text was updated successfully, but these errors were encountered: