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

Fix some weird memory leak from changing dimensions #581

Closed
wants to merge 3 commits into from
Closed

Conversation

kuba6000
Copy link
Member

Changed to ItemStackMap
before:
image

Copy link

Warning: 2 uncommitted changes
#582

Co-authored-by: GitHub GTNH Actions <>
@kuba6000 kuba6000 requested a review from a team December 31, 2024 05:05
@Glease
Copy link

Glease commented Dec 31, 2024

isn't the map supposed to remove the oldest entry since 20 elements? is that broken?

@kuba6000
Copy link
Member Author

isn't the map supposed to remove the oldest entry since 20 elements? is that broken?

probably, as you can see in the screenshot of memory dump

@Glease
Copy link

Glease commented Dec 31, 2024

what java version are you using? in gt5 dev env it definitely don't do it

@kuba6000
Copy link
Member Author

what java version are you using? in gt5 dev env it definitely don't do it

jdk-22

@slprime
Copy link
Member

slprime commented Dec 31, 2024

Won't this create a memory problem since this method is called every time you hover over an object in world?

@slprime
Copy link
Member

slprime commented Dec 31, 2024

and this can't solve problem, only hide it

@slprime
Copy link
Member

slprime commented Dec 31, 2024

#583 - this PR solved the problem better

@Glease
Copy link

Glease commented Dec 31, 2024

This map has some race condition problem. I bet this is just the fallout of that and neither PR would really work. I suggest to wrap it around synchronizedMap instead.

GT5 dev env doesn't have this issue because there is simply no contention there. The top offender lives in GC where it's calling getFluid as part of its tooltip event handler.

@slprime slprime closed this Dec 31, 2024
@Dream-Master Dream-Master deleted the fix branch December 31, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants