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

"DOOM II RPG" and "Orcs and Elves II" cause memory leaks by not deallocating MIDI streams #198

Open
AShiningRay opened this issue Oct 21, 2023 · 6 comments · May be fixed by #199
Open

"DOOM II RPG" and "Orcs and Elves II" cause memory leaks by not deallocating MIDI streams #198

AShiningRay opened this issue Oct 21, 2023 · 6 comments · May be fixed by #199

Comments

@AShiningRay
Copy link
Contributor

I studied this one mostly on DOOM II RPG, where RAM usage skyrockets right at the start if you interact with things and load multiple different MIDI streams. There's just a few different samples, but whenever a different sample from the previously played one is requested, the game allocates an entirely new player for it without de-allocating any of the others, and that adds up quickly.

Simply put, you can easily end the first stage with FreeJ2ME using upwards of 3GB of RAM if you are thorough on the map, whereas it will barely go over 300MB if audio is turned off.
image

Given that this has the side effect of making Garbage Collection much slower as the game goes on to the point of locking up if you play for long enough or interact too much, i think this and issue #90 are related in some way, since ID Software games tend to rely on MIDI for pretty much everything, and use similar engines.

Now, what i'm proposing here is more of a hacky workaround than anything, but it works and i couldn't find any negative side-effects in other games so far: We can limit the amount of "MIDI channels" that a J2ME app can allocate on FreeJ2ME, and attempts to allocate more than that will forcefully make one of the channels to deallocate in order to give room for the new sample to be played.

I went the safer route and allowed for up to 36 samples on one of the incoming commits in a pull request, which i think is far more than enough for J2ME apps unless they have some kind of bug allocating or freeing MIDI resources.

@AShiningRay
Copy link
Contributor Author

With PR #199, this is about where RAM usage maxes out with OpenJDK 8 on my system, never goes above that no matter how long i play DOOM II RPG since MIDI "channels" will be forcefully de-allocated to keep mem usage in check:
image

Still rather high just to run some J2ME app, but it's better than leaking memory till it causes an overflow and triggers the VM's OOM. Performance on that game is far more stable and less stuttery than before as well.

@recompileorg
Copy link
Collaborator

This makes me feel old... What you're calling a 'hacky workaround' is just what we used to do as a matter of routine. In the ancient past, memory was rare and precious so we learned to let go. We'd often have a fixed amount of memory allocated for whatever and either overwrite something old or fail to create a new thing. You can sometimes see the effects of this in old games. Not just with memory, but other limited resources as well.

I'm guessing that the designers of Doom II RPG were counting on J2ME implementations on the resource-constrained phones of the time to be a bit more aggressive. I can't imagine an audio system from the time even attempting to play 36 MIDI streams at once! I wouldn't be surprised at all if limiting the number to just two or three would be adequate. Maybe we could make this a configurable option?

@AShiningRay
Copy link
Contributor Author

AShiningRay commented Oct 21, 2023

This makes me feel old... What you're calling a 'hacky workaround' is just what we used to do as a matter of routine. In the ancient past, memory was rare and precious so we learned to let go. We'd often have a fixed amount of memory allocated for whatever and either overwrite something old or fail to create a new thing. You can sometimes see the effects of this in old games. Not just with memory, but other limited resources as well.

Yeah, i call it a hacky workaround here because that's the kind of thing that might fix a game but potentially break others. J2ME apps are all over the place as far as coding goes, Ferrari GT 3 for example won't play some MIDI tracks if the amount of available streams is less than 36 (even 32 makes it unable to play the jingle at the end of a race)... dunno, i just don't like those "per-game" fixes, though that is to be expected on such a vast platform compared to consoles like the NES or whatever where there's only one hardware and emulating it shouldn't depend on things like these.

I wouldn't be surprised at all if limiting the number to just two or three would be adequate. Maybe we could make this a configurable option?

We definitely can. DOOM II RPG and other ID games will work with as little as 2 streams just so they can swap things around, though others like Asphalt 4 and Ferrari GT 3 do need a bit more slots to play audio as intended.

I'll see to it soon, currently looking at a few other bugs on audio playback.

@woesss
Copy link
Contributor

woesss commented Oct 21, 2023

When you reach the end of the array, you go back to the beginning and call deallocate() on the player that may be currently playing. The first instance created will not always be the first unneeded one.
Wouldn't it be better to use a finalizer?

@AShiningRay
Copy link
Contributor Author

AShiningRay commented Oct 21, 2023

When you reach the end of the array, you go back to the beginning and call deallocate() on the player that may be currently playing. The first instance created will not always be the first unneeded one. Wouldn't it be better to use a finalizer?

Good catch. While i don't think finalizing the stream by normal means will work (In ID games' case, those allocations will just keep piling up there, never being cleaned by GC), i think checking for streams that aren't currently playing would be a good improvement to the force-cleanup logic.

MIDI Player's deallocate already stops the stream before cleaning everything up, but that does open up the possibility that it might end up cleaning a stream that is currently playing as you pointed out.

@AShiningRay
Copy link
Contributor Author

Okay, and now the max amount of MIDI players is a configurable option on both AWT and libretro builds.

image
image
image

Going beyond 64 is completely ludicrous, seeing as the new deallocation logic results in better memory efficiency by reducing how many slots are needed, really i'd only use those for debugging. Ferrari GT 3 now appears to be perfectly fine running with 16 slots as opposed to 36 like before, although i didn't play it as much to see if it still failed to allocate something down the line... still, should be anywhere between 16 and 32.

ID Software games also get by with a single slot and are FAR lighter to run thanks to 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

Successfully merging a pull request may close this issue.

3 participants