Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Client-side memory leaks in 1.12 #253

Closed
4 of 7 tasks
jchung01 opened this issue Aug 5, 2023 · 6 comments
Closed
4 of 7 tasks

Client-side memory leaks in 1.12 #253

jchung01 opened this issue Aug 5, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@jchung01
Copy link
Contributor

jchung01 commented Aug 5, 2023

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

@ACGaming ACGaming added the enhancement New feature or request label Aug 5, 2023
@jchung01
Copy link
Contributor Author

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:

ACGaming added a commit to ACGaming/MrTJPCore that referenced this issue Aug 16, 2023
ACGaming referenced this issue in ACGaming/CBMultipart Aug 16, 2023
@ACGaming
Copy link
Owner

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

@jchung01
Copy link
Contributor Author

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

@ACGaming
Copy link
Owner

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

@jchung01
Copy link
Contributor Author

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.

@ACGaming
Copy link
Owner

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

Repository owner locked and limited conversation to collaborators Dec 28, 2023
@ACGaming ACGaming converted this issue into discussion #324 Dec 28, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants