progwml6 / ironchest

Iron Chest minecraft mod by CPW
https://minecraft.curseforge.com/projects/iron-chests
GNU General Public License v3.0
137 stars 88 forks source link

1.12.2-7.0.59.842 | Hopper & Diamonds chests drops TPS #179

Open ghost opened 5 years ago

ghost commented 5 years ago

So, Minecraft 1.12.2, latest Sponge API, latest 1.12.2 version of ur mod (1.12.2-7.0.59.842) and when players connect hopper to a diamond chest, there is big laggs (20.00TPS > 6.50 TPS).

I tried with only this mod and nothing more (no plugins etc) and the problem is still here. The server does not crash, but it's very very laggy.

Thank's in advance !

progwml6 commented 5 years ago

Does this occur without sponge as well?

ghost commented 5 years ago

Mmmmh I think not, difficult to say but I tested that and I didn't had some laggs so I think nop.

ghost commented 5 years ago

Edit: same with diamond shulker box :/

ghost commented 5 years ago

And with golden chests too...

alexbegt commented 5 years ago

Is this still happening with the latest release of IronChests?

ghost commented 5 years ago

'will try it

ghost commented 5 years ago

Seems fixed...

9chu commented 5 years ago

Hi, I've met the same issue that the IronChest cost high cpu usage on my server. Upgrading IronChest from 1.12.2-7.0.59.842 to 1.12.2-7.0.67.844 won't fix the issue. It seems when the chest is full and using hopper to transmit block in it will cause the issue.

QQ20190618-230725@2x QQ20190618-230750@2x
ghost commented 5 years ago

"Up" as we say

9chu commented 5 years ago

"Up" as we say

Would you please tell me which one is the latest version? I have downloaded the version 1.12.2-7.0.67.844 from curseforge.com, which didn't work.

ghost commented 5 years ago

same here boy, but as always no update

ghost commented 5 years ago

RIP I think

noobanidus commented 5 years ago

You might also want to consider the fact that the Iron Chests inventory handler copies every item stack being inserted into it. When there's a large number of capabilities/capability events, this can be quite painful. Considering the hopper in vanilla at least (I think) tries to insert into every slot, this means it's constantly copying itemstacks. I don't know if this is relevant but I just spotted this issue; I recently switched to a different mod after noticing while profiling (using YourKit) how dreadful the diamond chest's inventory handler was.

Specifically referring to this line in ICChestInventoryHandler.java. Honestly, I'm not sure what the design choice behind not switching to ItemStackHandler is at this point -- I presume this is just legacy code?

alexbegt commented 5 years ago

Please try ironchest-1.12.2-7.0.71.846 when it goes live on curseforge to see if the TPS issue is fixed.

HenryLoenwind commented 5 years ago

Careful with the new version. Unless I misread that code, it now has major potential for voiding items.

noobanidus commented 5 years ago

Yes, I agree with HenryLeonwind. I didn't realise the extent of modification as I was mainly focused on the overhead caused by the ItemStack copying. The entire item handler should be rewritten either to use ItemStackHandler as its base (instead of a pseudo-IInventory), or replaced with InvWrapper, or use InvWrapper as an example on how to perform operations on the pseudo-IInventory.

HenryLoenwind commented 5 years ago

May I just remark that the real root cause of this is that ItemStack.copy() has to fire that event instead of being able to clone the capabilities (like it does for item, meta and nbt). Intermediary cause is that some mod(s) subscribe to that event and waste CPU time. ItemStacks are copied thousands of time each tick, that has to be a fast operation.

noobanidus commented 5 years ago

Oh, I agree, the issue is definitely some sort of cross-mod interaction with slow capability events. My point was more that none of the vanilla Forge implementations (ItemStackHandler, InvWrapper) copy straight off the bat, so they're less likely to suffer from the consequences of others' bad programming.

A further way of debugging this would definitely be to determine what mods add additional capabilities to ItemStacks in your pack and determine which ones are expensive and then pester those authors to update/improve their code.

ghost commented 5 years ago

So RIP as we said.

alexbegt commented 5 years ago

Try the newest version that I released on curseforge. It shouldn’t delete items as it uses vanillas