Memory bloats when selecting a map.
Opened this issue · 11 comments
Steps to reproduce:
- Have a program that tracks memory usage open. (ex. Task Manager)
- Open the game and go to Custom Game.
- Select a map (usually larger maps to see the bigger effect)
- Click on it multiple times. (do not start the game)
- Check memory usage.
- Repeat Step 4.
Not sure if it's intended to be cleared away by garbage collector, but it doesn't seem so after leaving the program running for quite a while. Probably a concern for those with limited memory.
Were you able to check this with a Java profiler app like JProfiler? I do not currently have a license to that, but I have seen that it can be quite powerful.
I would find it to be quite possible that the Java Virtual Machine might wait to deallocate memory until a GC event, so it is difficult for me to know what we would fix without a memory profiler analysis of particular classes or functions known to be leaking data into an ever-present map or static state that isn't cleared upon loading a new map.
The first thing that comes to mind for me might be the excessive use of anonymous inner classes in Jass2.java because a Java wizard long ago told me that anonymous inner classes (and therefore also lambda functions?) in Java leak memory badly and it is best to avoid them, as a language syntax, entirely. However, converting the Jass2 file to define each of these natives individually as a Java class would seem to be a software tragedy of some form because it would dramatically bloat the amount of code.
An internet search suggests that Java 8 and beyond's lambda functions might be better about this issue in certain cases than their anonymous inner class counterparts, so maybe the memory would get slightly better if we changed every Jass2 definition in that file from an anonymous inner into a lambda instead. I doubt that would totally solve whatever you're seeing, though, as some of the lambdas would still reference contextual variables because almost all of Warsmash was designed to avoid static state.
I don't have JProfiler since I never used any Java profilers nor I can't afford its license. I checked for open-source alternative and chose VisualVM to profile the game. This is the profile result I got over its memory. Not sure if it's accurate but these are the classes that ranked up occupying the most memory when I click the map multiple times.
Not too knowledgeable with how profiling works so I apologize for it.
Hmm. I guess one part of this that seems weird to me is when it says it only has 5,889,312 bytes of SetPoint instances. In theory that should be maybe 5MB.
I tried spam clicking maps with my mouse like you suggested and indeed was able to see the JVM on my windows computer reach about 33 GB of used RAM according to the Windows Task Manager, although shortly after that it jumped down to about 17 GB on its own even when I was still clicking a bunch. So it may be that the system is able to free this memory after using it at least partially.
I found that even when I limited the JVM with Xmx and Xms to a lower memory value such as 1GB heap and 1GB stack, spamming the mouse on the map list was still able to reach 7 GB of used memory according to Windows Task Manager pretty quickly. I have started to wonder if the issue might be due to non-Java texture allocations that wouldn't appear in a profiler like what we at first suggested. For example, when loading the map file the minimap is displayed by allocating a com.badlogic.gdx.graphics.Texture instance. But I am not calling .dispose() on these instances. That might be allocating memory using native code from the C++ JNI calls that LibGDX uses to talk to OpenGL and the graphics world. That seems like another place we could look.
I probably dig up something interesting. Turns out that LibGDX's texture isn't causing the bloat that I completely disabled the setMap function (since it involves creating and swapping texture) yet the problem persists, there's a problem within the listener.
As I narrow down (i.e commenting section of the codes) the lines of code within the MapListContainer's selection listener, teamSetupPane.setMap(MenuUI.this.rootFrame, MenuUI.this.uiViewport, war3MapConfig, mapInfo.getPlayers().size(), mapInfo, new PlayerSlotPaneListener() seems to be the culprit in the bloat. When I comment its section of code out and retry the steps, the program no longer hogging the memory above 1 GB.
There's something faulty within the codebase of teamSetupPane.
I appear to be seeing similar results on my machine. It looks like the issue may be related to too many FreeTypeFontGenerator.generateFont() calls. I wanted to conceptually support each string frame specifying its own font size, but as a result the allocation of a Font object for each string UI component appears to be exceedingly wasteful, and there are string UI components being created but not explicitly/carefully destroyed frequently when populating some UI components in some cases.
We would try to make a mapping from font size to Font to perhaps dramatically reduce the number of Font objects generated.
Edit: After an initial attempt to address this issue, the base memory used by Warsmash when attempting to launch to the menu was 300 MB on my machin instead of 1.6 GB, suggesting that apparently the vast majority of memory consumed was used on redundant font objects.
Great work! Is it doable to put the minimap textures into a hashtable to get the map's texture without creating instance of it every time? I'm planning to do that and see if there's any improvement to it.
EDIT:
Nope, it isn't and your implementation works better. 😛
While doing some bugfixing, I noticed that Jass2.loadConfig causes a heavy memory usage when trying to load a custom map (Blizzard TD, Skibi's CastleTD, etc.) for MapInfoPane.
Few clicks with Skibi's CastleTD is enough to reach 2GB. The textures (mapPreview & mapMap) does little to no effect to the major memory consumption and I assume this could be true to melee maps.
I couldn't able to get much indepth inspect with jassProgramVisitor.visit(parser.program()); because my editor is unable to find these missing Java files. (JassLexer, JassParser, JassBaseVisitor)
Yet Gradle seems able to recognized these files, is there any way to import/download these missing files into the source code?
Sure. These files are not a download but rather are generated code created by ANTLR. So if you wanted to view them, the key would be to look up how to manually run the ANTLR code generator. There is probably a Gradle target like "generateCode" or maybe "generateJava" but I am away from my PC at this moment. The file in the repo used to generate this is "jassparser/antlr-src/Jass.g4".
What you will probably find if you look into that is the limitation on the design of figuring out a map's available player slots. Conceptually, each map has one giant code file with all the custom user code, located inside the map archive usually under the name "war3map.j" which is a Jass2 syntax code file. Inside this file there is a function "main" to be called when the map begins as the loadbar finishes for play, but there is a separate function called "config" to be called on the lobby to configure the map lobby. This is also how some maps were able to customize their lobby music, although I have not tried to support that yet on Warsmash.
So, if you can imagine, this generated parser for an old Warcraft 3 specific language is parsing out the entire map script including all custom map behaviors for the custom game, and then throwing 99% of it away in order to call the "config" function. If you were going to somehow optimize this, one key would be to somehow "trim" the map script and I am not sure how you would conveniently and reliably do so.
It might be easier to reuse the parsed code that is loaded and combine "loadCommon" and "loadConfig" functions on the Jass2 class, but even that would be a headache, because right now I attempt to parse everything each time with different meanings for native functions bound so that the map lobby can't call "CreateUnit" for example because it wouldn't make sense without the context of a game environment.
Other than merging the two contexts and parsing the map script only once and then just swapping out the defined meaning of native functions if the player clicks Start Game, the only other optimization that quickly comes to mind for me would be trying to see if each definition of a "native" function could take less memory because at the moment they were anonymous inner classes. Maybe you could try converting them to lambdas and seeing if that reduces their memory footprint?
Edit: As a note to help understand this, rumors I have heard about the original Warcraft 3 game claimed that the processing for its custom syntax language used its own compiler, but currently in Warsmash when simulating an interpretation of the code I only have an interpreter and not a true compiler. If you dive into it at some point, knowing that nomenclature difference might help when looking things up. For example, when Warcraft 3 modders talk in online discussions about "jass bytecode," they are referring to something Warsmash does not have.
Thanks, I tried converting some of the native functions into lambda and so far the memory bloat hasn't be curbed yet. The jassProgramVisitor.visit(parser.program()); is the cause of the bloat and I'm not sure if there's a way to flush the parser after use.
In a meanwhile, I'll try converting most of the native functions into lambda since they look syntactically pleasing.
Thanks, I tried converting some of the native functions into lambda and so far the memory bloat hasn't be curbed yet. The
jassProgramVisitor.visit(parser.program());is the cause of the bloat and I'm not sure if there's a way to flush the parser after use.In a meanwhile, I'll try converting most of the native functions into lambda since they look syntactically pleasing.
One method I tried to mitigate this issue is by explicitly calling System.gc(); after loading Config. It does manage to hover between 300-400 MB without reaching 1 GB but I feel like calling the garbage collector explicitly isn't an optimal practice and I hope there's another method to resolve this. I wonder if the high memory usage was due to how Java handles memory as it'll only deallocates memory of discarded variables if it's needed.