Spread out slot transfer actions & caching

[aria1th]

!! It requires reviews
I was using this for months so it should be okay tho...

Changes :
Transfer Behavior : its transfer speed is actually equal, but it does not instantly transfers items.
-Now it inputs items then outputs items in separate tick, so it efficiently spreads out mspt.

Caches InventoryStorage.of
    -Should not affect anything and just 2% performance boost
Prevent SyncWithAll being spammed
    -reduces packets... but seems there's more packet spam issues, it generates thousands of packets...

Performance boosts:
Tested with 670 storage units working constantly, reduced 4.6mspt -> 2.67mspt

[aria1th]

Small idea, but can we spread out packet sync timing by using lastTickedSync = world.random.nextInt(20); ?

[modmuss50]

What is casuing this to sync so much?

If its beign called everytick or something that should be sorted at the root of the issue imo.

[aria1th]

Its part of packet issue, mostly storage units, are being updated once altogether and causing packet bottleneck for certain players, generating packet 'spikes'... so this is not really the solution.

I found that block entities are having same interval and same starting point, which was not spreading out packets over time(also mspt too).

[Technici4n]

-Now it inputs items then outputs items in separate tick, so it efficiently spreads out mspt.

"Spreading out" doesn't make sense imo, it still uses the same amount of time in total.

Performance boosts:
Tested with 670 storage units working constantly, reduced 4.6mspt -> 2.67mspt

Why? Is it just caching the storages? I see 3 somewhat unrelated changes, you should only keep the one(s) that actually make(s) a difference.

[aria1th]

Actual difference came from spreading out its working timing, which was not ready for review. Storages or machines being loaded at once shared its working timing and intervals, it had bad performance and packet issue. Especially industrial chunk loader-loaded machines will share its timing, on world load....

I randomized its starting point and actual difference was made. Caching itself was very important too, however.

Sync timing limit was not helpful at all, since storages were not spamming unlike machines, so I should revert it.

[Technici4n]

I really don't see why spreading out work helps? It reduces tps variance, but shouldn't change mean tps?

[aria1th]

hmm yeah, I inspected more and found that 'instant input-output' is now disabled, not sure if its intended. Storage or general machines are having 2 ticks to handle items, instead of outputting instantly when its auto input + auto output.
I tested with very intense bottleneck condition with constant flow of 30+ items IO is happening, and its difference is mostly from less bottleneck and more item merging.

[Ayutac]

so are you saying we don't need this PR after all?

[aria1th]

Not really, this partially solves some weird issues with highest mspt being observed as 200mspt, and internet connection issues with packet limit conditions when 400+ storage units are doing constant I/O (3200+ packets per seconds per player condition).
Test case is very extreme, so I'd say it helped mspt more from having more chance to merge items rather than others :

[Ayutac]

I... would rather have these extreme cases deal with that themselves before we accidentically introduce bugs that affect 95% of the playerbase instead of the 5% it might be actually be made for. But that is just my POV

[aria1th]

I couldn't see any bugs or serious effects with this, as I'm already using it in my server... even before this, storage units were not instant dropper and its update order was not promising. So it shouldn't really affect something, except for some really extreme cases :

Case A - someone was using 4-overclocked electric furnace as 'pipe', so they used input slot for Auto Input + Auto Output
General Case : someone was using 1-2 tick recipe crafter as 'pipe', then items will be processed before being outputted

but still it requires review, to prevent any unknown bugs or issues.. should I separate PR, one with separate actions / one with caching inventories?