Universal Tweaks

Universal Tweaks

871k Downloads

Client-side memory leaks in 1.12

jchung01 opened this issue · 6 comments

commented

EDIT: I'll update this list as I find more.

I've been doing some memory profiling on my client on a large modpack, and I figured I would list some mods in 1.12 that seem to be causing memory leaks by holding onto WorldClient and/or EntityPlayerSP, noticeable when changing dimensions or reconnecting (as that creates a new client/player):

  • CB Multipart: known leak, documented here.
  • MrTJPCore: pretty much the exact same leak as CB Multipart, documented here
  • New Tardis Mod: leak in ClientProxy due to adding players to a map that is unused (because this mod was in alpha for 1.12)
  • Cosmetic Armor Reworked: possible leak in PlayerRenderHandler. It uses a timed eviction cache which could be the issue (more recent version use weak keys)
  • JourneyMap (ARR): Possible leak in journeymap.client.task.multi.RenderSpec, I don't have much to back this up though besides what JProfiler says
  • Bibliocraft (ARR): Not really a memory leak in the strict sense, but does hold the first WorldClient (for whatever reason) of the session on world load/connect due to VersionCheck

EDIT:

  • Simply Jetpacks: Leak in SyncHandler maps client side - Server side, these maps are cleaned up properly here and here, but client side also updates these maps and those events are never called client-side, so the client-side maps are never cleaned up.

The first 3 (bolded) were the most consistent in holding the WorldClient, and although Tardis should be easy enough to fix (by just disabling adding to the map), I have tried a few ways to fix CB Multipart and MrTJPCore, but they are in Scala, so even if a simple fix is to use a WeakHashMap, I don't know how to apply it externally.
Some screenshots from a heap dump:
heapdump
heapdump2

commented

Other memory leaks that recently got fixes by RLMixins that can be added to UT as well as I'm still not sure if it's compatible with MixinBooter 8:

commented

Highly doubt that mixins for Scala imports would be easy, so I included them in separate forks.

commented

I think there may be a way to properly cleanup the maps using packets, so I'll look into it while looking into SimplyJetpacks.

commented

Other approaches might be possible, yes. But don‘t want to mess with Scala more than needed!

commented

Maybe we could add this to the list as well?
xJon/Tekkit-2#43

commented

Added fix for TARDIS and Simply Jetpacks in above PR. It would indeed require messing with a lot more files in Scala with MrTJPCore and CBMultipart and their dependents, so looks like the forks are the way to go.