MC-238249Redundant checks in explosion code
When an explosion is created, it casts a lot of rays (1352, to be precise). The issue is, the block and liquid blast resistance at the origin point of the explosion is checked for every single ray, even though for each of them, the result will be exactly the same, so only checking once would be needed. Calls to getBlockState and getFluidState are quite expensive, and having a lot of these to be unnecessary can really hurt the performance.
Code Analysis of current latest version (1.17.1) (using mojang mappings and CFR decompiler) :
if (i != 0 && i != 15 && n2 != 0 && n2 != 15 && n != 0 && n != 15) continue; double d = (float)i / 15.0f * 2.0f - 1.0f; double d2 = (float)n2 / 15.0f * 2.0f - 1.0f; double d3 = (float)n / 15.0f * 2.0f - 1.0f; double d4 = Math.sqrt(d * d + d2 * d2 + d3 * d3); d /= d4; d2 /= d4; d3 /= d4; // this sets the starting position of the ray at the explosion point double d5 = this.x; double d6 = this.y; double d7 = this.z; float f = 0.3f; for (float f2 = this.radius * (0.7f + this.level.random.nextFloat() * 0.6f); f2 > 0.0f; f2 -= 0.22500001f) { object = new BlockPos(d5, d6, d7); // however as you can see, it checks for the blockstate and liquid state at that position, every time a new ray is made BlockState blockState = this.level.getBlockState((BlockPos)object); object2 = this.level.getFluidState((BlockPos)object); if (!this.level.isInWorldBounds((BlockPos)object)) continue block2; Optional<Float> optional = this.damageCalculator.getBlockExplosionResistance(this, this.level, (BlockPos)object, blockState, (FluidState)object2); if (optional.isPresent()) { f2 -= (optional.get().floatValue() + 0.3f) * 0.3f; }
How to fix :
a good way to fix would be to have a boolean check if the blast resistance have already been checked at the origin position, keep the origin position as well (it's basically free skipping some calls to the world if the same ray calls multiple time the origin pos, which can happen, as jumps along the ray are only of 0.3F), and keep the blast resistance in a float.
Fixing this bug would allow explosions in general to perform better, especially in case they would happen inside of a liquid or a blast proof block, since instead of 1352 * 2 calls to the world, there would only be two.
Normal
Created Issue:
redundant checks in explosion code
When an explosion is created, it casts a lot of rays (1352, to be precise). The issue is, the block and liquid blast resistance at the origin point of the explosion is checked for every single ray, even though for each of them, the result will be exactly the same, so only checking once would be needed. Calls to getBlockState and getFluidState are quite expensive, and having a lot of these to be unnecessary can really hurt the performance.
Code Analysis of current latest version (1.17.1) (using mojang mappings and CFR decompiler) :
if (i != 0 && i != 15 && n2 != 0 && n2 != 15 && n != 0 && n != 15) continue; double d = (float)i / 15.0f * 2.0f - 1.0f; double d2 = (float)n2 / 15.0f * 2.0f - 1.0f; double d3 = (float)n / 15.0f * 2.0f - 1.0f; double d4 = Math.sqrt(d * d + d2 * d2 + d3 * d3); d /= d4; d2 /= d4; d3 /= d4; // this sets the starting position of the ray at the explosion point double d5 = this.x; double d6 = this.y; double d7 = this.z; float f = 0.3f; for (float f2 = this.radius * (0.7f + this.level.random.nextFloat() * 0.6f); f2 > 0.0f; f2 -= 0.22500001f) { object = new BlockPos(d5, d6, d7); // however as you can see, it checks for the blockstate and liquid state at that position, every time a new ray is made BlockState blockState = this.level.getBlockState((BlockPos)object); object2 = this.level.getFluidState((BlockPos)object); if (!this.level.isInWorldBounds((BlockPos)object)) continue block2; Optional<Float> optional = this.damageCalculator.getBlockExplosionResistance(this, this.level, (BlockPos)object, blockState, (FluidState)object2); if (optional.isPresent()) { f2 -= (optional.get().floatValue() + 0.3f) * 0.3f; }How to fix :
a good way to fix would be to have a boolean check if the blast resistance have already been checked at the origin position, keep the origin position as well (it's basically free skipping some calls to the world if the same ray calls multiple time the origin pos, which can happen, as jumps along the ray are only of 0.3F), and keep the blast resistance in a float.
Fixing this bug would allow explosions in general to perform better, especially in case they would happen inside of a liquid or a blast proof block, since instead of 1352 * 2 calls to the world, there would only be two.
![[Mod] Neko](../../../data/avatars/users/92901.png){kind=small}
redundant checks in explosion codeRedundant checks in explosion code
![[Mojang] Moesh](../../../data/avatars/users/10122.svg){kind=small}
Low![[Mod] Michael Wobst](../../../data/avatars/users/37868.png){kind=small}
![[Mod] ampolive](../../../data/avatars/users/18949.svg){kind=small}
![[Helper] Johnibur](../../../data/avatars/users/28381.png){kind=small}
relates to
relates to
![[Mojang] slicedlime](../../../data/assets/gravatar/slicedlime.jpeg){kind=small}
relates to
![[Mod] turbo](../../../data/avatars/users/92867.png){kind=small}
This ticket is invalid as it is a feature request. Optimization reports have to point out specifically what could be improved in the code and how, for example MC-238249.
A report specifically about the explosion calculations would be made redundant by MC-238249 which goes into a lot more detail on that.
If you have multiple TnT exploding in the same position, they will each perform their calculations of knockback and blocks broken independently. Despite the fact that the effect of their explosions can be combined linearly. This issue relates to MC-260743 and MC-238249. Features such as OptimizedTnT from carpet mod have working implmentantions which do not affect vanilla behviours:
OptimizedTnT is an ideal solution for optimizing TnT because if multiple TnT entities explode at the same location, instead of processing the blast knockback formula independently for each TnT, it simply combines their effects into a single calculation vastly improving performance while maintaining behaviour. This also includes a fix for MC-238249 which eliminates the redundant checks on the blast origin if the TnT is in a state where it does not deal damage to blocks.
As an example here is a world download with a TnT cannon positioned at 4K,4K in the overworld and setup to fire a payload at 0,0:
https://www.mediafire.com/file/vpf6ctq0cvfl0p7/Orbital_Strike_Crash_Test.zip/file
Instructions are as follows:
- Join the world and wait 1 minute
- Press the Button labelled "Start Cannon"
- The player will be teleported to 0,0,0 and the cannon will begin charging
Note the TPS drop significantly due to large quantities of TnT performing inefficient calculations- Note that eventually the game becomes unresponsive due to excessive collision checks as the cannon fires
- Note that the cannon works perfectly fine with optimizations running
You can also find the video about the cannon here:
can confirm for 1.20.6, here is the test setup for those who want to replicate: explosion_lag_issue.zip
how to use the setup:
-load the world in 1.20.2 (version before the issue is introduced), press the button on the command block (its near the diamond block), wait till the game unfreezes from lag, check how much server is lagging behind in the console
-load the world in 1.20.6 (version after the issue is introduced), press the button, wait till the game unfreezes from lag and check how much server is lagging behind in the console, it should be roughly twice as much behind as the test in 1.20.2 was.
in my own testing this issue indeed made explosions lag about twice as much compared to how it was before the issue was introduced, exact amount of lag is in the attached screenshots.
It is important to note, that even though this issue is marked as related to two other reports, MC-238249 is about a completely different issue (the one related to block breaking and not entity interaction) and the MC-260762 just being a feature request asking to implement an optimization from a certain mod even though it is not vanilla-preserving (as in it breaks vanilla behavior thus breaking even more explosion related contraptions).
However this report (MC-272414), states that since 23w45a the entire exposure calculation is needlessly done twice which worsens the lag from it even more, this leads to tnt cannons becoming too laggy to use for many players. thus entire field of cannon and tnt related redstone suffers a lot, i (and many others from tnt tech community) really hope this issue gets fixed soon.
id like to add, could cache even a 3x3x3 cube around origin position, to optimise this even further
actually easiest way would probably just be to use a hashmap and use blockpos as a key, and keep blockstate as entry tbh
Can confirm in 1.18 Pre-release 1.