Memory leak in 4.2.11

[hippie68]

Platform
Linux, Debian 11, x64

Version
4.2.11 .jar file

Describe the bug
While playing turns for just a few minutes, memory usage steadily grows to more than 4 GB until the RAM (6 GB) is full and the OS freezes.

To Reproduce
Steps to reproduce the behavior:
Play a game with the 4.2.11 Java file (openjdk 17.0.4 2022-07-19).

Expected behavior
The memory usage does not grow until 6 GB of RAM are full.

Screenshots

[touhidurrr]

doesn't happen in previous versions?

[hippie68]

Now I have tried 4.2.8, which has the bug, too. The memory leaks after ending a turn.

[yairm210]

Okay, this is really freaky
I can see the memory usage go up on the performance monitor, even when I set -xmx100m so it shouldn't consume above 100mb of memory
But, research lead to [this'(https://plumbr.io/blog/memory-leaks/why-does-my-java-process-consume-more-memory-than-xmx), and so I tried setting -XX:MaxPermSize=100m and that did crash.

Following @Azzurite's excellent advice at #7491 I took a dump - generated a heap dump file from the game and it shows something interesting:

The important parts here are that there are 5 WorldMapHolder and WorldMapScreen in memory, when we should be disposing of the old ones as we generate new ones.
It's very hard to understand who's holding a reference to it, but it's definitely still there,

Thing is, screens are disposed, and most of their data is gone, but still somehow something is still referencing them - so what is it?

After a WHOLE BUNCH of digging, it looks like this is the suspect:

addZoomListener doesn't actually use an existing class. Instead, it creates an object based off of an existing class, with some changes.

Since class objects are never disposed of, and remain in the permanent storage (remember MaxPermSize?), if we create a class with a reference to the zoom pane, which references the world map holder, which references the world map, none of them will be disposed!

So that's my guess for now, going to try and turn all those 'dynamic classes' into real honest-to-goodness classes and see it if makes a difference

[yairm210]

Well that wasn't it.
I have no idea what's going on, I can't seem to simulate the OOM error except on extremely small max memory sizes

[yairm210]

But the used mem is still showing up as like 600mb

[touhidurrr]

The important parts here are that there are 5 WorldMapHolder and WorldMapScreen in memory, when we should be disposing of the old ones as we generate new ones.
It's very hard to understand who's holding a reference to it, but it's definitely still there,

Well it seems like my random guess was plausible. I actually thought that this might be an issue. That's why I talked about Garbage Collector in Discord. Such a thing in Unciv is nothing new as it has been reported before that Unciv takes more and more memory as the game progresses.
I thought it might be because the game isn't dumping the old game objects properly and Garbage Collector is not doing good enough job to dump useless parts of memory and thus it keeps piling up each time the client refreshes. I thought it might be interesting to see if using newer JRE's results in better use of memory.
But then I saw that the person who opened this issue was actually using a newer version of JRE. So that means we need to resolve this issue manually.

As objects are basically references, deleting objects often results to nothing.
The only solution I can think of is manually deleting all variable and objects and lists recursively and building new object.
That might work, who knows.

[touhidurrr]

Another solution is dynamic object building. Instead of dumping the whole game object, we can try to make minimal changes to the object to represent the updated version of the game. Also never done anything like this myself. Not sure if this would solve the issue but should improve performance if ever implemented.

[hippie68]

Seeing you mentioned the World Map, perhaps it may help to know I played in 4K native resolution and set the game's internal resolution to 1500x1000. I also changed the game's default settings to show more icons on the map. Perhaps this all contributes to more memory leaking than it would on smaller screens.

[yairm210]

That makes very little difference. The big difference is in map size - the larger the map, the higher the costs.

@touhidurrr that's not a bad idea, and it's only possible now thanks to @Azzurite ensuring we have a static image of the map for swapping out :).
Will be quite a bit of work though

[SomeTroglodyte]

As I understood, object : superclass { in expressions does one instance of an anonymous subclass, just a little different to what the keyword does on the declarative level (where it does some singleton management stuff). That might explain "Well that wasn't it".

[yairm210]

After 'general' memory profiling on Android for about 20 turns, I discovered the following:

Start:

After some turns:

After move turns:

After some time doing nothing:

Conclusion

The reason the Java memory limiters didn't cause OOM exceptions is because the leaking memory isn't from Java objects. It seems to be from Graphics, which rise about 10MB every turn, so after 100 turns you'll have an entire wasted GB of them.
I these 'Graphics' are OpenGL textures or somesuch, but I'll try to research this and mem dump the Android after we have a lot of that so we can see where it's actually going.

[yairm210]

O SHIT SON