Snow! Real Magic! ⛄ (NeoForge)

@luigistyle

Mod loader

Fabric

Minecraft version

1.20.1

Mod version

9.0.1

Modloader version

Fabric Loader 0.14.22 + Fabric API 0.87.0

Modpack info

No response

If bug:

Can you reproduce this issue with relevant mods only?

If bug: The latest.log file

N/A

Issue description

We have experienced major TPS lags while flying through the world with elytras. Investigating the issue with spark, we have noticed that the lag is caused by Minecraft unloading chunks & trying to save them. It turns out that, when Snow! Real Magic! is saving state/NBT data, it seems to call getBlockState. Calling getBlockState on an unloaded chunk causes will load the chunk from disk. This is blocking the server thread, causing large MSPT spikes as chunks get unloaded when flying fast.

Technical analysis

When chunks unload, Minecraft tries to save them to disk. SnowCoveredBlockEntity contains a call to WatcherSnowVariant#updateOptions when saving its state/NBT:

SnowRealMagic/src/main/java/snownee/snow/block/entity/SnowCoveredBlockEntity.java

Lines 60 to 61 in e3e9578

    
           if (options.renderBottom) { 
        
           	variant.updateOptions(getBlockState(), level, worldPosition, options); // data fix

WatcherSnowVariant#updateOptions is calling BlockGetter#getBlockState to check the block above:

SnowRealMagic/src/main/java/snownee/snow/block/WatcherSnowVariant.java

Line 17 in e3e9578

boolean ro = level.getBlockState(pos.above()).isAir();

As the BlockGetter cannot get the block state, since the chunk has been unloaded, it will load the chunk from disk again & need to block the server thread while doing so. This results in reduced TPS.

(Side note: the Hooks#canSnowSurvive call right below the previous snippet also contains a getBlockState call to check the block below)

SnowRealMagic/src/main/java/snownee/snow/Hooks.java

Line 118 in e3e9578

BlockState blockstate = worldIn.getBlockState(pos.below());

My 2 cents on how this could get fixed, even though I'm not familiar with the mod:

The data that's being saved in this case seems to only (?) relate to what properties the adjacent blocks have. Hypothetically, you would not need to store that data in the SnowCoveredBlockEntity's NBT. Instead, you can retrieve the adjacent blocks when the chunk has been loaded.
If the above is incorrect, is there any chance to "cache" this data, so the mod doesn't need to load the chunk again when saving NBT?

cc @luigistyle

somewhat similar issue causes the game to crash in single-player. when loading chunks,

Calling getBlockState on an unloaded chunk

will cause the game to crash. also causes serious frame lag.

thank you for the detailed analysis. hopefully fixed now. feel free to reopen it if the problem still occurs

	if (options.renderBottom) {
	variant.updateOptions(getBlockState(), level, worldPosition, options); // data fix

Share to