Simple Storage Network

Simple Storage Network

56M Downloads

Lag from chunk reloading

LemADEC opened this issue ยท 4 comments

commented

As of 1.12.2-1.3.0, profiler reports lag here:
image
It appears the network is refreshing and actively reload chunks in the unloading event.
There's actually 3 issues in network refresh logic here:
1- it is using recursion which is prone to cause a stack overflow.
2- it's loading new chunks while scanning.
3- it's called during chunk unloading.

commented

There's already a ReloadNetworkWhenUnloadChunk configuration option with the following warning:
"If this is true, reload network when a chunk unloads, this keeps your network always up to date. It has been reported that this cause lag and chunk load issues on servers, so disable if you have any problems. "

Following a quick code review, in TileMaster.addConnectables():
1- code already tries to avoid loading chunks
2- getTileEntity() is called 5 times for each position
3- instanceof will fails if object is null, as such there's no need to test against null. In other words, object != null && object instanceof MyClass is the same as object instanceof MyClass.

In TileConnectable.onChunkUnload() there's the actual issue:
1- master tile entity is always accessed even when the chunk is not loaded
2- for every connectable in a chunk, there'll be a network refresh. Instead, we could just tag the master for refresh to aggregate in a single refresh on next tick.

commented

1.4.0 has been in the works for a few weeks now. Addresses some of these issues, as well as two big performance adds, and integration with FastWorkbench . I addressed the three points - i think - from addConnectables in latest develop branch

I dont want to touch 'onChunkUnload' right now since it seems like a key part that could have risk involved.

commented

What risk is involved in your mind for onChunkUnload?

commented

New release is out here , lots of new changes. Willing to talk more on discord or in a new issue for anything. Code review is also always appreciated,

for onChunkUnload i just literally didnt have time to look at or test that right now, i wanted to get all the other changes released first before jumping on something new.

https://minecraft.curseforge.com/projects/simple-storage-network/files/2617829