[Bug]: Constantly increasing number of Resourceful Bee Entity instances
MaPePeR opened this issue · 17 comments
Bug Description
The number of instances and memory consumed by com.resourcefulbees.resourcefulbees.entity.passive.ResourcefulBee seems to constantly increase. I think this might be causing the garbage-collector related freezes and high memory consumption I encountered while playing All the Mods Sky 6: AllTheMods/atm6-sky/issues/486
Even after I removed all the bees from my void world the instances were still climbing.
It might not be the root cause of my problem, but its still something that shouldn't happen.
How to Reproduce?
- Let the game run for some time having the mod enabled.
- Use
jmap -J-d64 -histo:live <pid>to get a histogram of memory used. - This problem might only occur under some additional circumstances, but I wasn't able to figure out yet, what those are.
Expected Behavior
com.resourcefulbees.resourcefulbees.entity.passive.ResourcefulBeeshould not be one of the top contenders for memory used and there shouldn't be 60000 instances of it floating around.
Version
1.16.5-0.6.7.2b
Mod Loader Version
Forge 36.1.31
Logs or additional context
Shortly after start:
num #instances #bytes class name
----------------------------------------------
218: 637 519792 com.resourcefulbees.resourcefulbees.entity.passive.ResourcefulBee
After 13 minutes:
25: 33128 27032448 com.resourcefulbees.resourcefulbees.entity.passive.ResourcefulBee
After 19 minutes and the first freeze occured:
15: 46231 37724496 com.resourcefulbees.resourcefulbees.entity.passive.ResourcefulBee
After 23 minutes:
13: 55323 45143568 com.resourcefulbees.resourcefulbees.entity.passive.ResourcefulBee
Acknowledgement
- I agree that I read the terms at the top of the page.
so a little update I put a "fix" for this by only doing the updates on slot change or container update to reduce the amount it gets called. This should fix this issue so I'm gonna close it if it persists after this change which will come in the next update let me know and reopen this issue.
ok this weird because when I filled an apiary with breeders that were all running it did make new instances but it garbage collected them as soon as it reached about 5k
and about JEI yeah thats a simple fix I can just cache them with the ingredient type
Unfortunately my pc won’t be back online until Tuesday at the earliest due to moving in a new house. But I’m going to need more information in regard to this. ATM6S has no naturally spawning bees that I am aware of. As such removal of all bees from the world shouldn’t allow more “instances” of bees to be created. I would also think that since the pack has been out for some time and that we have over 3.3mil downloads and are in a number of large packs a memory leak like this would’ve already been identified. I’ve also never had issues like this in all of my testing and I’ve left worlds running for days and weeks at a time. Finally I’ve never used Jmap before so I’m not entirely sure how it works. This will need more testing and profiling to be certain there actually is a memory leak in our mod.
Please tell me, if i can provide any more info. I also haven't used jmap before, but It seemed like the right tool for the job. (I changed the java-executable to jre1.8.0_291, so I could use jmap from jdk1.8.0_291)
Looking at the other classes in the top 25 also makes me think that ResourcefulBee shouldn't be up there. Otherwise it's mostly "primitive" classes:
num #instances #bytes class name
----------------------------------------------
1: 4229963 323315776 [C
2: 3725533 299571560 [Ljava.lang.Object;
3: 1963439 295720536 [I
4: 5856826 187418432 java.util.HashMap$Node
5: 2446122 117413856 java.util.HashMap
6: 4628844 111092256 java.util.ArrayList
7: 991407 102582672 [Ljava.util.HashMap$Node;
8: 3690408 88569792 java.lang.String
9: 241378 63021752 [B
10: 1910996 61151872 net.minecraft.client.renderer.model.BakedQuad
11: 1471578 58863120 java.util.LinkedHashMap$Entry
12: 731029 52634088 net.minecraft.item.ItemStack
13: 55323 45143568 com.resourcefulbees.resourcefulbees.entity.passive.ResourcefulBee
14: 1383445 44270240 java.util.RegularEnumSet
15: 318289 43949112 [J
16: 442862 38971856 net.minecraft.block.BlockState
17: 111128 37330656 [Lnet.minecraft.pathfinding.PathPoint;
18: 528388 33816832 it.unimi.dsi.fastutil.chars.Char2ObjectOpenHashMap
19: 1111938 26686512 net.minecraft.network.datasync.EntityDataManager$DataEntry
20: 770627 24660064 net.minecraftforge.common.capabilities.CapabilityDispatcher
21: 279483 23421424 [Ljava.util.WeakHashMap$Entry;
22: 932845 22388280 net.minecraft.client.renderer.model.ModelResourceLocation
23: 491717 21643560 [Z
24: 1271419 20342704 java.lang.Integer
25: 770627 19882296 [Lnet.minecraftforge.common.capabilities.ICapabilityProvider;
I have put out some requests to other devs to look into this further until I am able to use my pc again. But it definitely needs more testing and profiling. You can try attaching VisualVM profiler to the JVM and see if it produces similar results.
The "Sampler" function of VisualVM seems to be consistent with the output of jmap. I think I also found some culprit: I had bee jars sitting in an apiary breeder but not enough food for them to breed. I think after removing the jars from the breeder the number of instances didn't climb any further. Except for when looking at bees in JEI, but the JEI-instances were freed by garbage collection again after not looking at them.
Edit: I assume there is an entity leaked everytime BeeJar.getEntityFromStack is called?
Ok some questions:
- Were you currently looking at bees in JEI?
- Were you using the apiary breeder?
I tried multiple things.
I noticed that having filled bee jars in the apiary breeder will constantly increase the number of ResourcefulBee instances that will not be freed by garbage collection. Even when not looking at Bees in JEI.
Looking at bees in JEI will also drastically increase the numbers of ResourcefulBee instances, but when not looking at them they will be freed after some time and the instance count will go down to the number of bees created by the breeder. (Creating entitys in a render-method should probably be avoided, though?)
Funfact: After ~23 minutes I had 55323 instances. The breeder creates 2 instances per tick, 20 ticks per second => 55323 / 2 / 20 / 60 ≈ 23 minutes?
Well yeah. Bc that change wouldn’t fix the issue.
I was misled by the "removes unnecessary local field creation" in the commit message and hoping I'm not knowing some weird JVM quirk, that would make this change fix it. 😅
My garbage collector IS doing its job. It's just not cleaning the ResourcefulBee Instances.
I now used VisualVM and created a heap dump to look at the references to the ResourcefulBee instances (Didn't know one could do that before). It even has an option to calculate the "closest GC root":
It turns out, that the garbage collection of the entitys is prevented by Alex-the-666/Citadel and Alex-the-666/Ice_and_Fire, because it keeps hard references to all the Bee entitys. (Don't be fooled by the WeakIdentityHashMap: It's only WeakReference in the key, but there is a Hard-Reference in the value of the map as well)
To avoid issues like that in the future it would probably still be wise to change the apiary breeding code to use EntityType and only create Entitys, when there is a real Entity that is supposed to live in the world, though.
I need to obtain data that can only be obtained by using creating an entity. Unfortunately forge/Mojang doesn’t cache the class for the entity type in the entity type class. That makes it extremely difficult with generics to figure out which class im working working with without the need to create the entity. But at least we’ve been able to figure out several issues from this scenario.
Just to further clarify the class cache issue. Basically because bee jars work with all bee entities not just ours and our class hierarchy is setup such that we can have multiple variations down the line, I have to create the entity in order to be able to check that the entity created is an instanceof ResourcefulBee. If instead the entity type class cached the class of the entity then this would not be an issue ever and would also likely reduce a ton of overhead in a lot of instances across mods. But forge is really annoying for doing PRs so I refuse to work with them.
I applied the changes from af46b47 on top of the 1.16.5 branch and it did not fix the memory leak caused by the breeder.
Well yeah. Bc that change wouldn’t fix the issue. It basically just wrapped the code into a static method. The entities need to be statically cached which they are not. So basically 4 entities are created every tick. But tbh as much as it is an issue with the code it’s also an issue with the garbage collection not doing its job properly. Even with the breeder I’ve never had increasing instances that weren’t garbage collected properly. In any case this issue won’t be resolved by me personally for a little while and at that point it will be completely rewritten
As you can see in this video https://streamable.com/ttr2fb the garbage collector works fine on my system, there is still a problem with us making too many instances but.... clearly it's a problem with the garbage collector. If you can think of any more information about your system that may break the garbage collector like this can you tell us?