Possible memory leak from WorldEventHandler
comp500 opened this issue ยท 6 comments
Versions
Betweenlands version:
3.3.11
Forge version:
14.23.4.2750
Singleplayer or Multiplayer:
Tested in Singleplayer
Installed mods:
Link to full crash log
Log uploaded here, however the game does not crash.
Description of the problem
How to reproduce:
I was testing a modpack that I made and generated many chunks by flying around to find memory leaks in the pack. I did find that there were significant GC lagspikes after the game had been running for about 15 minutes, where the game had plenty of free ram beforehand.
I ran Eclipse Memory Analyzer to find sources of leaks. The Leak Suspects report (attached below) that it generated suggested that thebetweenlands.common.handler.WorldEventHandler was holding a large amount of the heap (about 1GB) from being GC'd. It seems that the UNLOAD_QUEUE HashMap held 13,147 previously loaded chunks in memory.
I think you may be able to rewrite the WorldEventHandler to use WeakReferences to chunks instead of normal references which prevent the chunks from being garbage collected.
There might be another mod that is holding these chunks in memory (or cancelling an event that removes chunks from the UNLOAD_QUEUE?), however when I removed The Betweenlands the game used significantly less ram and very few GC lagspikes occurred.
Edit: I've read the code a bit more and I think it may be that the entry is not removed when unloadChunk is called in line 114. It may also be due to a static value being referenced from both server and client sides (as this was observed in singleplayer).
Pictures:
Eclipse Memory Analyzer leak suspects html report (zipped)
I can also send the memory dump (compressed to about 400MB) if it would be helpful.
I had quite a serious memory leak issue while in the Betweenlands in the current version of SevTech with javaw.exe taking over 12GB when I had only allocated about 6GB.
No idea if it was the fault of Betweenlands, but I'm making this remark just in the case someone else gets the same problem and is searching the net for a place to report it.
Excellent, it does seem to be completely fixed. I tested the new build and the previous release again and the new build has no entries in the UNLOAD_QUEUE, while the old build has over 1000.
I have uploaded the log to https://paste.ee/p/62dva.
Interestingly, I found this in the log for both builds:
java.lang.NullPointerException: null
at journeymap.client.forge.event.WorldEventHandler.onUnload(WorldEventHandler.java:33) [WorldEventHandler.class:1.12.2-5.5.2]
at net.minecraftforge.fml.common.eventhandler.ASMEventHandler_1963_WorldEventHandler_onUnload_Unload.invoke(.dynamic) [?:?]
at net.minecraftforge.fml.common.eventhandler.ASMEventHandler.invoke(ASMEventHandler.java:90) [ASMEventHandler.class:?]
at net.minecraftforge.fml.common.eventhandler.EventBus.post(EventBus.java:182) [EventBus.class:?]
at net.minecraftforge.common.DimensionManager.unloadWorlds(DimensionManager.java:403) [DimensionManager.class:?]
at net.minecraft.server.MinecraftServer.func_71190_q(MinecraftServer.java:788) [MinecraftServer.class:?]
at net.minecraft.server.MinecraftServer.func_71217_p(MinecraftServer.java:668) [MinecraftServer.class:?]
at net.minecraft.server.integrated.IntegratedServer.func_71217_p(IntegratedServer.java:185) [chd.class:?]
at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:526) [MinecraftServer.class:?]
at java.lang.Thread.run(Unknown Source) [?:1.8.0_171]
I'll try and pinpoint the root cause of the issue (probably in another mod), it might also be causing memory leaks in JourneyMap.
Edit: Nevermind, this error happens with just journeymap, so it probably won't affect other mods.
Huh, how did I even miss that. I'm almost certain I had tested that the map was properly cleared, but alas I apparently did not. Thanks for reporting this!
Actually, this does not seem to happen with only the Betweenlands installed. The chunk save event is usually always called right after a chunk unloads on the server side. I suspect another another mod might be slightly changing how chunks are unloaded and saved. Either way line 114, like you pointed out, was intended to clear the map in such a case and I've now fixed that. However, the fact that this is happening in the first place might mean that some data isn't being saved properly (such as for Wight Fortresses, Giant Trees and smaller Dungeons).
Please install this dev build https://github.com/Angry-Pixel/The-Betweenlands-Development-Builds/releases/tag/dev-1.12-dev-896-03.09.2018, then enable "Debug mode" in the Betweenlands config and after that fly around to load/unload some chunks (just like you did when you found this problem) and then send us the log.