TransmitterNetwork lag
sfPlayer1 opened this issue · 26 comments
Mekanism cause lag spikes, e.g. as seen in http://creepgaming.com/nps/spike4.nps (VisualVM compatible), where it spends 3.3 seconds within a single tick building some network as a followup on ae2 loading a chunk.
The connectivity lookup into unloaded chunks problem I mentioned at SleepyTrousers/EnderIO-1.5-1.12#1959 seems to apply to Mekanism as well.
I am also experiencing this exaclty same issue. It's really annoying and almost makes the game unplayable.
Here is a VisualVM profiling of my server: http://halvors.org/downloads/Applied%20Energistics%202.nps (It's named AE2, but that was because i first thought it was it).
@aidancbrady may you prioritize this issue? Ir really makes the game unplayable :-(
Another nps, 12s spike, showing it more clearly and originating from Mekanism's own tick:
http://creepgaming.com/nps/spike-12067-50323413.nps
Just found what I think to be the source of the issue - each of those chunk load calls originated from a transmitter checking for an adjacent, active redstone connection with "isBlockIndirectlyGettingPowered()." It looks like that method force loads chunks without checking if they exist, and then that chunk load triggers other transmitters to do the same check, resulting in the halt.
I just developed my own redstone check, after I push the build this should no longer occur.
Hmm. Still getting the lag. It seems to be a little bit better, but it's still there :-(
It's still loading chunks, see http://creepgaming.com/nps/spike-2074-189208754.nps
Afaik the method you are calling reaches 2 blocks far, but you only check 1.
@aidancbrady May you reopen this issue?
Actually I think you didn't since it can go like this, where S = start, N = neighbor, Q = blocks also queried:
Q
QNQ
QNSNQ
QNQ
Q
The diagonals aren't being checked.
A proper check would require to check all neighbors and their neighbors, or better and faster just check World.doChunksNearChunkExist(te.xCoord, te.yCoord, te.zCoord, 2).
Hopefully I've just fixed it fully, please test 176 when it's finished compiling.
@sfPlayer1, check out my fix in the previous commit - I'm almost positive it fixes your issue as mentioned, but I'd appreciate a second pair of eyes.
It looks good in terms of not loading extra chunks from what I can tell.
I'm not sure if you want to check weak power from the direct neighbors as well and whether the implementation can cause cases of being unable to turn the rs signal off due to the indirect queries checking the original block as well. The 2nd case of course only applies if the block interacts with redstone in a bidirectional fashion.
It doesn't, but for future reference, what would be the best way around that?
-aidancbrady
On Mar 4, 2015, at 1:11 AM, sfPlayer1 [email protected] wrote:
It looks good in terms of not loading extra chunks from what I can tell.
I'm not sure if you want to check weak power from the direct neighbors as well and whether the implementation can cause cases of being unable to turn the rs signal off due to the indirect queries checking the original block as well. The 2nd case of course only applies if the block interacts with redstone in a bidirectional fashion.
—
Reply to this email directly or view it on GitHub.
@sfPlayer1 Can you confirm if this works now with the latest build? Still experiencing some lag on my server, but it may not be Mekanism related.
I can't test it myself, some sampler profiling info from elsewhere is needed (presumably spike triggered).
I've just noticed a somewhat related problem, this time with PartSidedPipe(?), which is likely not fixed yet. It happened with an older build though, but the redstone fix was very specific.
A nps file showing the problem can be found at http://files.player.to/tmp/spike-2918-1551299993_out.nps
What fun. Any idea what on earth could be causing that? Apart from just a very large network of transmitters across many chunks?
I may be missing something, but the nps file indicates an issue relating to AE2.
Keep digging. It does look like AE2 has a problem too, but it's not as bad as ours.
I think notifyTileChange has similar issues to the redstone method that was causing issues before.
The call in this line is loading all adjacent chunks to each chunk we load, worst case. I guess we write a new non-loading implementation if possible.
Yes AE2 only triggered the 1st chunk load, but Mekanism did others.
Besides that there seems to be a more general issue with the network code being rather inefficient at adding something to a grid, esp when adding a larger quantity at once. It looks a bit like it's throwing everything away and starting over.
It shouldn't be, but that does sound like the kind of thing my code would do when it wasn't supposed to be. I've (hopefully) fixed notifyTileChange() to not load chunks, which should improve the situation somewhat. At some point I'll see if I can audit the code a bit to see if there's anything I can do to improve it.