CivClassic / Citadel

A Minecraft plugin to protect your chests and builds using in-game materials. Requires an attacker to break each block a certain number of times. Built for Paper 1.16.5
BSD 3-Clause "New" or "Revised" License
1 stars 21 forks source link

InventoryListener(onInventoryMoveItemEvent) causes mass TPS loss. #37

Closed KaloHG closed 4 years ago

KaloHG commented 4 years ago

Recently this piece of code has been causing a large amount of lag on the main server.

The timings reveal that it struggles to do something with hoppers. I'm not big brained enough to easily pick out what is wrong with the code, I just know it is causing lag. (Timings: https://timings.aikar.co/?id=dcb74ed0a56c497c8939016b9f5fc97c)

I'd imagine its related to hoppers just bugging out or something, due to large cactus farm or something similar.

Maxopoly commented 4 years ago

Keeping this open until timings confirm fix

KaloHG commented 4 years ago

Fix Confirmed via new timings report! (https://timings.aikar.co/?id=1f9c28871f1c4f799cb0da292e62ffae) only serious plugin related lag remaining is ExilePearl. However, this resolved the major game breaking lag.

Thanks Ory for the fix!

Maxopoly commented 4 years ago

Reopening, latest timing still has it show up https://timings.aikar.co/?id=c1770dbfee564cc29e067398a2b470a3

Citadel::Event: v.c.m.c.l.InventoryListener (InventoryMoveItemEvent) count(72) total(82.04% 15.095s, 1,257.91% of tick) avg(209.65ms per - 628.96ms/3.00 per tick)

Each invocation took ~200ms, I'd guess that's still chunkloading.

Maxopoly commented 4 years ago

Severe TPS loss seems resolved. It's still showing up kind of high in regards to hopper carts, but that's unrelated to the original chunk loading and probably just because its happening a lot. Thanks to Ori for all the work on this.