Skip to content
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

[Bug] Memory Leak of Client Player #556

Open
pietro-lopes opened this issue Aug 24, 2024 · 8 comments
Open

[Bug] Memory Leak of Client Player #556

pietro-lopes opened this issue Aug 24, 2024 · 8 comments
Labels
triage Newly submitted bug that needs verification

Comments

@pietro-lopes
Copy link

Observed behaviour

Continuation of #549

Now it is holding on ClientMagicData on 3.4.5

image

Expected behaviour

No leak

Steps to reproduce

Same as old issue.

Server Type

Single Player

Crashlog

No response

Iron's Spells N Spellbooks version

3.4.5

Forge version

1.21.1 - 21.1.22

Other mods

No response

@pietro-lopes pietro-lopes added the triage Newly submitted bug that needs verification label Aug 24, 2024
@iron431
Copy link
Owner

iron431 commented Aug 24, 2024

The SpellSelectionManager and the SpellBarOverlay do not store a reference to ClientMagicData, nor does any class

@pietro-lopes
Copy link
Author

ClientMagicData stores spellSelectionManager which stores player

@iron431
Copy link
Owner

iron431 commented Aug 25, 2024

@pietro-lopes
Copy link
Author

I see, but that does not include dimension change or death. Both triggers player creation again.
We don't have something similar to PlayerEvent.StopTracking on client side, so I don't know which event should be used for both cases (player dim change and death)

I would just not store player and replace player with Minecraft.getInstance().player, this is not an expensive call, unless there is something I'm missing that you shouldn't use.

@pietro-lopes
Copy link
Author

OH, I forgot about ClientPlayerNetworkEvent.Clone, you can listen to that to replace/clear player.

@Shadowfire86
Copy link

Shadowfire86 commented Jan 5, 2025

Not 100% but Issue still seems to exist. I've been messing around exculsivley with Iron's Spells and Spellbooks and the symptoms started up. I know it's not very helpful, but figured you'd want to know. If it helps, I notice it most often when swapping spellbooks or washing scrolls for ink. Will test a bit more tomorrow. Seems likely related to either the Cauldron or the Spell UI (Only 2 consistent things present that I can see so far). Yeah, about 75% sure it's related to the cauldron. Rapidly increases memory allocation to 100% even when no armor or spellbooks equipped. Even at 100% (before final stage of lag spikes) I was able to equip and run around casting for quite a while without locking up.

@iron431
Copy link
Owner

iron431 commented Jan 5, 2025

it is not possible for the spell selection manager to affect the memory usage of the cauldron

@Shadowfire86
Copy link

I'll admit I know nothing about how the mod is coded. Just reporting what I saw. Basically if I'm doing either the scroll to ink or ink upgrade process, the memory leak happens rather quickly (admittedly working in batches of a few hundred scrolls/inks). That was the repeatable result of my testing. If I'm wrong, fair enough. I just am enjoying the mod other than the leak, so I wanted to help you narrow down possibilities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Newly submitted bug that needs verification
Projects
None yet
Development

No branches or pull requests

3 participants