justinwon777 / HumanCompanions

Human Companions Mod
GNU General Public License v3.0
3 stars 15 forks source link

Excessive CPU Usage on Server #31

Closed jkwphysics closed 1 year ago

jkwphysics commented 1 year ago

Describe the bug Following an update from v1.6.0 to v1.7.0, which added the health leveling system, my server is reporting excessive ticking from the chunks containing companions - to the point of crashing. I rolled back to 1.6.0 and no longer have this issue. I don't have something more quantitative, but usage jumps from 20-50% on the previous version to 90-120% with only 1 player logged in (using a hosted service that allows temporary overuse).

To Reproduce 1.19.2 MC Server with Forge 43.2.0 running Human Companions 1.7.0 will show a noticeable increase in CPU usage on server compared to previous versions of the mod.

Versions (Please fill this out) Minecraft version: 1.19.2 Human Companions version: 1.7.0

humancompanionstick

justinwon777 commented 1 year ago

Not sure how to reproduce this. Do you know how many companions were in the chunks with issues?

jkwphysics commented 1 year ago

Not sure how to reproduce this. Do you know how many companions were in the chunks with issues?

Just one. The spark profiler shows the entities in the world for that chunk, and it's the same chunk that keeps generating a message "Saving oversized chunk [15, 149] (1911090 bytes} to external file ./aldinn/entities/c.15.149.mcc" in the console.

justinwon777 commented 1 year ago

I'm assuming that companion was spawned before you got version 1.7.0? I think 1.7.0 may not be compatible with previous versions because the health calculation depends on a value set when the companion first spawns. Can you try spawning a new companion in a different chunk and see if that chunk has issues, too?

justinwon777 commented 1 year ago

Is the picture of the thread what it looks like every tick? Asking cause the function modifyMaxHealth() shouldn't run every tick and that seems to be the issue with 28.2% of the usage.

jkwphysics commented 1 year ago

Is the picture of the thread what it looks like every tick? Asking cause the function modifyMaxHealth() shouldn't run every tick and that seems to be the issue with 28.2% of the usage.

Sorry, I am not the most familiar with the tool. I assume it is each tick because the percentages are of the total 100% of the thread that runs the server. I will try to spawn a new companion as soon as I can. I just started trying to get into the world, which looked fine from the server panel, but my character won't load because I happen to be in that chunk I think. This is after rolling back so not sure what's damaged yet.

jkwphysics commented 1 year ago

I'm assuming that companion was spawned before you got version 1.7.0? I think 1.7.0 may not be compatible with previous versions because the health calculation depends on a value set when the companion first spawns. Can you try spawning a new companion in a different chunk and see if that chunk has issues, too?

I was able to regen some corrupted chunks and do the upgrade to 1.7.0. Spawning a new companion doesn't seem to make the server go crazy, so perhaps there is something to that.

MarkusBordihn commented 1 year ago

I was able to reproduce the same kind of issue, but on the client side with my existing human companions on a dedicated server. As you already mentioned it seems that the new version is not compatible with older human companions and is causing some kind of loop which freezes the client in the end.

In my case the only solution was to downgrade to the former version or to deactivate the mod to be able to join the server again.

justinwon777 commented 1 year ago

I haven't been able to reproduce. I started a server in 1.5.1, spawned a companion, updated the mod to 1.6.0 and then 1.7.0, and saw no issues playing on the server.

justinwon777 commented 1 year ago

Ok, I'm pretty sure I fixed it. Try it out guys. https://www.curseforge.com/minecraft/mc-mods/human-companions/files/4275514

MarkusBordihn commented 1 year ago

I could easily reproduce the issue with a world copy even with a new profile and the version 1.7.0

CurseForge Profile: image

Client Freeze: image

Java Sampler: image

According the sampler it seems there is a loop with older "Human Companions".

Unfortunately the world is 800MB in size and it seems your source-code in GitHub is not up to date. But let me know if I can help you with isolate the issue more in detail.

MarkusBordihn commented 1 year ago

Just saw your last update, unfortunately version 1.7.1 shows the same issue in the sampler: image

justinwon777 commented 1 year ago

Looking into it, but not sure what exactly the issue is from looking at that.

justinwon777 commented 1 year ago

I think it might be a different issue from the other person? The thread doesn't show the mod anywhere, so hard to see what part of my code is causing this, but something with how I use AttributeInstance probably. The github code is up to date btw.

MarkusBordihn commented 1 year ago

Thanks for updating the code. Was able to reproduce the error in the dev env and it's related to the modifyMaxHealth part which is requested several time per second.

image

justinwon777 commented 1 year ago

Is that with the latest change? The last change I made fixes that. At least for me it did. Did you make a new world or open your world with the issue?

MarkusBordihn commented 1 year ago

It's the lastest version from GitHub with my existing world.

Seems related to the checkStats() function, if I uncomment the function in the tick the world is loading as expected, e.g.:

    public void tick() {
        if (!this.level.isClientSide()) {
            checkArmor();
            //checkStats();
        }
        super.tick();
    }

According some additional test it seems that this.getMaxHealth() return 1 for the first few seconds, which triggers the attribute updates several times.

image

justinwon777 commented 1 year ago

What do getBaseHealth() and getExpLvl() return inside checkStats()? Oh I see, so baseHealth is returning 20 and level is 0 I'm guessing, hence 1 != 20 = true So it seems like max health isn't changing after modifyMaxHealth()?

justinwon777 commented 1 year ago

I don't know why modifyMaxHealth() keeps being called. The issue I fixed was old companions don't have a baseHealth, which is set at spawn, so it'll be 0 when getBaseHealth() is called, so modifyMaxHealth() was trying to change the health to <1, but that's not possible cause the minimum health is 1, so the max health is set to 1 instead and the if statement in checkStats() is true every tick.

MarkusBordihn commented 1 year ago

You could maybe add a check if the same modifier already exists and only update the modifier if it is different from the value before. Additional you could also add a counter which triggers these checks maybe every 10-20 ticks.

Seems the health is set after 5-10 min (on disk), but because of the attribute spam in the beginning the client is blocked the whole time and is unresponsive even after the 5-10 min.

justinwon777 commented 1 year ago

I see, thanks. I didn't fully understand how attribute instance and modifiers worked.

MarkusBordihn commented 1 year ago

No worries, it took me some time myself to understand them. For my mods I using a different method for a level system like:

As a quick-fix you could maybe change the update to every 60 seconds, which helped to fix the issue in my dev environment with my existing world. For my own mod case no user has complaint so far that the only see a level up, after 1 minute and not immediately. ;)

justinwon777 commented 1 year ago

Thanks, I just pushed a change with the attribute fix. Could you try it on your world?

Also hi, didn't know you were the player companions dev.

MarkusBordihn commented 1 year ago

Thanks a lot for the fix, it worked perfect on my world and fixed the issue. Will push it to my server today and will let you know in the case if there are any issues with several players.

Really like your mod, so keep up the good work. 😄