SoRadGaming / Simple-HUD-Enhanced

https://discord.gg/2NY3a3AJ8e Minecraft mod that enhances the game's Heads-Up Display (HUD) by introducing customizable elements and features to display information.
MIT License
4 stars 1 forks source link

Increasing memory usage when using the Equipment Status HUD #11

Closed alerouzic closed 8 months ago

alerouzic commented 9 months ago

Hello there !

I've been recently using your mod (which is great!) and found out that I started to have issues with my memory and CPU usage.

I did notice that after some time (1/2 hours) my game was running out of memory, fluctuating a lot at the highest limit (2GB here). I also suspected an abnormal amount of GC, which was confirmed when I started to see my CPU ramping a lot in this situation. After some testing, It seems to be related to the Equipment Status HUD feature and on how object' lifecycle are handled (EquipmentInfoStack class).

Setup

Testing With jvisualvm I've started to monitor my game and see if I could find a suspicious class using a lot of memory.

First test, with the option activated, we can see an increase in my memory usage, and a lot of live object for the EquipmentInfoStack. context: was playing as usual on an SMP server for 1+ hour ![git_1_1](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/cfbbbfe9-9efd-4ace-9440-5d0a044a2130) ![git_1_2](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/ec8c07b2-d67d-4ff7-9111-28c532e4a2a0) ![git_1_3](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/15e626b5-28cd-4c0d-b934-c92188c2ad11)
Second test, with the EquipmentStatus HUD option deactivated (didn't changed anything else on my setup). context: was afk on the same SMP server for 1+ hour ![git_2_0](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/6d82ac37-a71e-41fa-9e7f-40b2e406e28d) ![git_2_1](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/0f940561-458f-409f-92c0-7f701f021c7b) ![git_2_2](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/c8cd1490-4984-4e7b-bd36-8ec1749318fa)

Expected behaviour Using the Equipment Status HUD option should not cause indefinite increase in memory usage.

Additional context Feel free to ask me anything, I hope I didn't forget anything :) Regards,

SoRadGaming commented 9 months ago

Thanks for this amazing detailed bug report. I observed this, too, ~and just found the issue: my code constantly adds armour or items the payer is holding to a list controlled by EquipmentInfoStack Class. I just keep adding to it, and it never gets unallocated. In theory, if you keep cycling items, this should speed up the process of data allocation.~

But the only thing that makes me doubt it is that this class should not run if you disable the option in settings. (Yeah not the Issue)

// Draw Equipment Status
        if (config.toggleEquipmentStatus) {
            Equipment equipment = new Equipment(context, config);
            equipment.init();
        }
public void init() {
        // Get Armour and Hand Item from Inventory and Offhand
        equipmentInfo = new ArrayList<>(
                Arrays.asList(
                        new EquipmentInfoStack(this.player.getInventory().getArmorStack(3)),
                        new EquipmentInfoStack(this.player.getInventory().getArmorStack(2)),
                        new EquipmentInfoStack(this.player.getInventory().getArmorStack(1)),
                        new EquipmentInfoStack(this.player.getInventory().getArmorStack(0)),
                        new EquipmentInfoStack(this.player.getOffHandStack()),
                        new EquipmentInfoStack(this.player.getMainHandStack())
                )
        );

        // Remove Air Blocks from the list
        equipmentInfo.removeIf(equipment -> equipment.getItem().getItem().equals(Blocks.AIR.asItem()));

        // Check showNonTools
        if (!config.equipmentStatus.showNonTools) {
            equipmentInfo.removeIf(equipment -> equipment.getItem().getItem().getMaxDamage() == 0);
        }

        if (config.equipmentStatus.showDurability) {
            // Draw Durability
            getDurability();
        }

        // Draw Items
        draw();
    }

Note: I am working on text scaling, I am going to finish the new scaling text code and then create a potential fix for this issue.

alerouzic commented 9 months ago

Hey !

I've been reading some of the code, and I think the problem comes from the parent object named Equipment, we can confirm this with my previous screens where we can see the number of instances increasing for Equipment (test 1 with the option active).

Just an hypothesis here, but could the non-deletion of these instances be linked to the context parameter in the constructor of Equipment ? Can it, somehow, avoid the deletion of Equipment instances ?

Looking forward to hearing more about your findings when you will work on this issue 👀

SoRadGaming commented 9 months ago

It seems my v4.0.0 Update has worked without much issue. I will now spend all the time that I allocate to open source on this. Will start testing in the coming days. Ill write when I have an update.

SoRadGaming commented 8 months ago

Please Test this new update to check if the issue is fixed. (jar is in the PR Page)

alerouzic commented 8 months ago

Hello there,

Sorry I wasn't able to get back to you sooner, I was on holidays.

I just tested the new version (4.1.0), but it seems that the problem persists 👀 Please find my new tests enclosed. On my side I will not be able to test future binaries since I'm very busy :(

Regards

FIRST TEST: 1GB memory, Equipment HUD ENABLED, 1 hour AFK ![0](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/9159276d-117f-4a42-8b7d-1b2c995b045c) ![1](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/ab7f808c-c8a8-49db-9eec-bfeb4fafc124) ![2](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/163fa739-4a74-48b7-b96c-95d35c8c37c3)
SECOND TEST: 1GB memory, Equipment HUD DISABLED, 1 hour AFK ![disable-0](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/15fad2ca-3656-4d79-a10b-14d7c5cd9406) ![disable-1](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/cd6fa8d0-dd67-4f4f-82d8-419ee290ca15) ![disable-2](https://github.com/SoRadGaming/Simple-HUD-Enhanced/assets/70151737/4392287e-a891-42e6-8af9-2d85ef812cce)
SoRadGaming commented 8 months ago

I finally found the issue; it seems v4.1.0 has improved but has not been fixed. The Equipment class extends the Hud class which causes its instances never to be garbage collected.

Here is the jar With the Fix; in my testing, it seems to have fixed the problem. simple-hud-enhanced+1.20-1.20.2-v4.2.0.zip

alerouzic commented 8 months ago

Yup, the memory leak seems to be fixed, I now have an expected memory curve with my game !

Thanks for the fix <3

Capture d'écran 2023-10-30 143008

SoRadGaming commented 8 months ago

I will release the update and some improved async code (still in progress); thanks for all the help. What do you prefer as a contributor name, Antonio or alerouzic?

alerouzic commented 8 months ago

alerouzic is fine 👍