Darkhax-Minecraft / ItemStages

Allows items to be put into stages.
GNU Lesser General Public License v2.1
7 stars 18 forks source link

Lag on the server due to restriction #63

Closed Diversion98 closed 3 years ago

Diversion98 commented 3 years ago

Hey.

When 1 player joins the server, its kinda ok but soon as a second player joins, the tps goes down to 4 tps overtime.

I did a spark profile of ticks that takes longer than 45ms with 2 players online. This is the result: https://spark.lucko.me/muyuuMCUha

my script: https://pastebin.com/cwhPLcVN

Is it my script that is inefficient? or is it something in the mod?

I edited that script on the server for now and the TPS is fine now. Script running right now (basically the same script but commented itemstages): https://pastebin.com/QeUCQm2k tick only takes 22ms with 2 players online.

Thanks in advance.

Cheers!

Darkhax commented 3 years ago

Hello, I have released a new preview build that will reduce the overhead of ItemStages by a fair bit. There will likely be some significant overhead still though, this is a combination of your scripts and ItemStages needing further optimizations. Currently ItemStages will scan the inventory of a player every tick and test each item for an unmet restriction. This is done to account for restricted items being obtained without picking them up, like dispensers or villagers. This is not needed for every item and can be disabled using preventInventory(false).

Diversion98 commented 3 years ago

sweet, i'll test it on the server and give some feedback :) preventInventory(false) was allready in my script for every mod.

script that goes over the mod:

for modid in HideFuturistic { mods.recipestages.Recipes.setRecipeStageByMod("industrial_age", modid); var Futuristic = ItemStages.createModRestriction(modid, "industrial_age"); Futuristic.preventInventory(false); // Allows item to be kept in inventories. Futuristic.preventPickup(false); // Allows item to be picked up. Futuristic.hiddenName("Unknown Item from the Futuristic Age"); }

Darkhax commented 3 years ago

@Diversion98 Thanks for letting me know. It's still testing those restrictions even though they've been disabled for this task so that's a bug.

I have a few more optimizations planned that should drastically lower the impact even further, although I would really appreciate it if you could submit another profile with the previous optimizations in place.

Diversion98 commented 3 years ago

no problem, happy to help :)

I commented the code again (after the new spark profile) because I gated mods but it also gates ores, and i have no way to ungate it. I need to rewrite the script to make it work on the server but yeah thats a lot of code to do xd

new spark profiler with the new update of itemstages and no commented code: https://spark.lucko.me/yzqtmqlB6d

Darkhax commented 3 years ago

Thanks for profiling again. It looks like the first round of fixes reduced a good amount of the performance overhead but it is still unreasonably high. I've released another update that fixes preventInventory(false) restrictions still being checked. I've also fixed a separate bug that was causing a significant number of redundant stage and restriction checks.

With both of these fixes the overhead should be almost entirely eliminated. As a general note it is somewhat costly to create a new restriction which is why I recommend grouping as many of them together as you can. I've added a new ItemStages.createModRestriction (String[] modids, String... requiredStages) method that should allow you to group most of your restrictions together using the modid arrays you already have. While not necessary it may be worth looking into after confirming the first two fixes are working for you.

Diversion98 commented 3 years ago

aight ill do the changes tomorrow to use the new method and make a new profiler to show u :)

question: is their gonna be a method to ungate an item later on like with recipestages? or is it not planned? Just to know if i need to make any changes.

Darkhax commented 3 years ago

No it will not be possible to unstage an item after the fact. This is because the restriction does not keep track of the individual items, it creates logic that can test the items as needed.

Diversion98 commented 3 years ago

i did look at your method and at my code but I was allready using that method.

my script: https://pastebin.com/Z7yVZdd6

Darkhax commented 3 years ago

This is a different method with the same name. Instead of using a single modid you can use all of them at once.

For example

var modsToHide as string[] = [ "moda", "modb", "modc" ]
var one = ItemStages.createModRestriction(modsToHide, "stage_one");
Darkhax commented 3 years ago

Have you had time to test out the new version?

Diversion98 commented 3 years ago

hey!

Sorry for the late response, been a busy week at work. I tested the new version and no more lag thanks to your mod.

85% unsafe.park(), i think that means that the server is waiting to do something? this is with 0 players online (everyone sleeping), ill put another profiler up later today when somebody is online as this profiler won't give you any information I suppose

profiler: https://spark.lucko.me/4nVc2H8ITQ

Diversion98 commented 3 years ago

profiler with 2 people on: https://spark.lucko.me/04er7Gcr9J

Darkhax commented 3 years ago

Thanks for the additional profiling. My interpretation is that everything is running as expected and there are no performance issues now?

Diversion98 commented 3 years ago

thats fully correct. We hitted 10 players on that test server yesterday running at 19.8 tps :D This issue can be closed in my opinion :)

Darkhax commented 3 years ago

Okay, thanks for helping me debug this and improve the mod. Your JEI issues are still being looked into however I haven't been able to make any headway and have been working with the JEI dev about fixing it.