From d3865551a56b94b7fde500d09c1a181fafe1a827 Mon Sep 17 00:00:00 2001 From: James Seibel Date: Sun, 10 Sep 2023 17:09:33 -0500 Subject: [PATCH] Minor Lighting Engine GC optimizations --- .../core/api/internal/ServerApi.java | 3 +- .../core/generation/DhLightingEngine.java | 48 ++++++++++++------- .../distanthorizons/core/pos/DhBlockPos.java | 12 ++--- .../chunk/IChunkWrapper.java | 3 +- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/com/seibel/distanthorizons/core/api/internal/ServerApi.java b/core/src/main/java/com/seibel/distanthorizons/core/api/internal/ServerApi.java index d18ff93c3..241fb7ae9 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/api/internal/ServerApi.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/api/internal/ServerApi.java @@ -37,6 +37,7 @@ import com.seibel.distanthorizons.core.wrapperInterfaces.world.ILevelWrapper; import com.seibel.distanthorizons.core.wrapperInterfaces.world.IServerLevelWrapper; import org.apache.logging.log4j.Logger; +import java.util.ArrayList; import java.util.LinkedList; import java.util.List; import java.util.concurrent.ThreadPoolExecutor; @@ -204,7 +205,7 @@ public class ServerApi { // generate the chunk's lighting, ignoring neighbors. // not a perfect solution, but should prevent chunks from having completely broken lighting - List nearbyChunkList = new LinkedList<>(); + ArrayList nearbyChunkList = new ArrayList<>(1); nearbyChunkList.add(chunkWrapper); DhLightingEngine.INSTANCE.lightChunk(chunkWrapper, nearbyChunkList, level.hasSkyLight() ? 15 : 0); chunkWrapper.setUseDhLighting(true); diff --git a/core/src/main/java/com/seibel/distanthorizons/core/generation/DhLightingEngine.java b/core/src/main/java/com/seibel/distanthorizons/core/generation/DhLightingEngine.java index ece655536..47d3066e4 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/generation/DhLightingEngine.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/generation/DhLightingEngine.java @@ -19,7 +19,6 @@ package com.seibel.distanthorizons.core.generation; -import com.seibel.distanthorizons.core.config.Config; import com.seibel.distanthorizons.core.enums.EDhDirection; import com.seibel.distanthorizons.core.logging.DhLoggerBuilder; import com.seibel.distanthorizons.core.pos.DhBlockPos; @@ -42,6 +41,15 @@ public class DhLightingEngine private static final Logger LOGGER = DhLoggerBuilder.getLogger(); public static final DhLightingEngine INSTANCE = new DhLightingEngine(); + /** + * Minor garbage collection optimization.
+ * Since these objects are always mutated anyway, using a {@link ThreadLocal} will allow us to + * only create as many of these {@link DhBlockPos} as necessary. + */ + private static final ThreadLocal PRIMARY_BLOCK_POS_REF = ThreadLocal.withInitial(() -> new DhBlockPos()); + private static final ThreadLocal SECONDARY_BLOCK_POS_REF = ThreadLocal.withInitial(() -> new DhBlockPos()); + + private DhLightingEngine() { } @@ -56,7 +64,7 @@ public class DhLightingEngine * @param nearbyChunkList should also contain centerChunk * @param maxSkyLight should be a value between 0 and 15 */ - public void lightChunk(IChunkWrapper centerChunk, List nearbyChunkList, int maxSkyLight) + public void lightChunk(IChunkWrapper centerChunk, ArrayList nearbyChunkList, int maxSkyLight) { DhChunkPos centerChunkPos = centerChunk.getChunkPos(); AdjacentChunkHolder adjacentChunkHolder = new AdjacentChunkHolder(centerChunk); @@ -88,8 +96,10 @@ public class DhLightingEngine // find all adjacent chunks // and get any necessary info from them boolean warningLogged = false; - for (IChunkWrapper chunk : nearbyChunkList) + for (int chunkIndex = 0; chunkIndex < nearbyChunkList.size(); chunkIndex++) // using iterators in high traffic areas can cause GC issues due to allocating a bunch of iterators, use an indexed for-loop instead { + IChunkWrapper chunk = nearbyChunkList.get(chunkIndex); + if (chunk != null && requestedAdjacentPositions.contains(chunk.getChunkPos())) { // remove the newly found position @@ -101,17 +111,22 @@ public class DhLightingEngine // get and set the adjacent chunk's initial block lights - List blockLightPosList = chunk.getBlockLightPosList(); - for (DhBlockPos blockLightPos : blockLightPosList) + final DhBlockPos relLightBlockPos = PRIMARY_BLOCK_POS_REF.get(); + final DhBlockPos relBlockPos = SECONDARY_BLOCK_POS_REF.get(); + + ArrayList blockLightPosList = chunk.getBlockLightPosList(); + for (int blockLightIndex = 0; blockLightIndex < blockLightPosList.size(); blockLightIndex++) // using iterators in high traffic areas can cause GC issues due to allocating a bunch of iterators, use an indexed for-loop instead { + DhBlockPos blockLightPos = blockLightPosList.get(blockLightIndex); + blockLightPos.mutateToChunkRelativePos(relLightBlockPos); + // get the light - DhBlockPos relLightBlockPos = blockLightPos.convertToChunkRelativePos(); IBlockStateWrapper blockState = chunk.getBlockState(relLightBlockPos); int lightValue = blockState.getLightEmission(); blockLightPosQueue.push(blockLightPos.x, blockLightPos.y, blockLightPos.z, lightValue); // set the light - DhBlockPos relBlockPos = blockLightPos.convertToChunkRelativePos(); + blockLightPos.mutateToChunkRelativePos(relBlockPos); chunk.setDhBlockLight(relBlockPos.x, relBlockPos.y, relBlockPos.z, lightValue); } @@ -142,7 +157,7 @@ public class DhLightingEngine // set the light - DhBlockPos relBlockPos = skyLightPos.convertToChunkRelativePos(); + skyLightPos.mutateToChunkRelativePos(relBlockPos); chunk.setDhSkyLight(relBlockPos.x, relBlockPos.y, relBlockPos.z, maxSkyLight); } } @@ -199,8 +214,8 @@ public class DhLightingEngine { // these objects are saved so they can be mutated throughout the method, // this reduces the number of allocations necessary, reducing GC pressure - final DhBlockPos neighbourBlockPos = new DhBlockPos(); - final DhBlockPos relNeighbourBlockPos = new DhBlockPos(); + final DhBlockPos neighbourBlockPos = PRIMARY_BLOCK_POS_REF.get(); + final DhBlockPos relNeighbourBlockPos = SECONDARY_BLOCK_POS_REF.get(); // update each light position @@ -215,11 +230,10 @@ public class DhLightingEngine // propagate the lighting in each cardinal direction, IE: -x, +x, -y, +y, -z, +z - for (EDhDirection direction : EDhDirection.CARDINAL_DIRECTIONS) + for (EDhDirection direction : EDhDirection.CARDINAL_DIRECTIONS) // since this is an array instead of an ArrayList this advanced for-loop shouldn't cause any GC issues { - pos.offset(direction, neighbourBlockPos); // mutates neighbourBlockPos - // converting the block pos into a relative position is necessary for accessing the light values in the chunk - neighbourBlockPos.convertToChunkRelativePos(relNeighbourBlockPos); // mutates relNeighbourBlockPos + pos.mutateOffset(direction, neighbourBlockPos); + neighbourBlockPos.mutateToChunkRelativePos(relNeighbourBlockPos); // only continue if the light position is inside one of our chunks @@ -313,9 +327,11 @@ public class DhLightingEngine int chunkZ = blockZ >> 4; // since there will only ever be 9 items in the array, this sequential search should be fast enough - for (IChunkWrapper chunk : this.chunkArray) + for (int i = 0; i < this.chunkArray.size(); i++) // using iterators in high traffic areas can cause GC issues due to allocating a bunch of iterators, use an indexed for-loop instead { - if (chunk != null + IChunkWrapper chunk = this.chunkArray.get(i); + + if (chunk != null && chunk.getChunkPos().x == chunkX && chunk.getChunkPos().z == chunkZ) { return chunk; diff --git a/core/src/main/java/com/seibel/distanthorizons/core/pos/DhBlockPos.java b/core/src/main/java/com/seibel/distanthorizons/core/pos/DhBlockPos.java index 1e915d335..d07a5da8c 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/pos/DhBlockPos.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/pos/DhBlockPos.java @@ -130,12 +130,12 @@ public class DhBlockPos } /** creates a new {@link DhBlockPos} with the given offset from the current pos. */ - public DhBlockPos offset(EDhDirection direction) { return this.offset(direction, null); } + public DhBlockPos offset(EDhDirection direction) { return this.mutateOffset(direction, null); } /** if not null, mutates "mutablePos" so it matches the current pos after being offset. Otherwise creates a new {@link DhBlockPos}. */ - public DhBlockPos offset(EDhDirection direction, @Nullable DhBlockPos mutablePos) { return this.offset(direction.getNormal().x, direction.getNormal().y, direction.getNormal().z, mutablePos); } + public DhBlockPos mutateOffset(EDhDirection direction, @Nullable DhBlockPos mutablePos) { return this.mutateOffset(direction.getNormal().x, direction.getNormal().y, direction.getNormal().z, mutablePos); } - public DhBlockPos offset(int x, int y, int z) { return this.offset(x,y,z, null); } - public DhBlockPos offset(int x, int y, int z, @Nullable DhBlockPos mutablePos) + public DhBlockPos offset(int x, int y, int z) { return this.mutateOffset(x,y,z, null); } + public DhBlockPos mutateOffset(int x, int y, int z, @Nullable DhBlockPos mutablePos) { int newX = this.x + x; int newY = this.y + y; @@ -156,12 +156,12 @@ public class DhBlockPos } /** Returns a new {@link DhBlockPos} limits to a value between 0 and 15 (inclusive) */ - public DhBlockPos convertToChunkRelativePos() { return this.convertToChunkRelativePos(null); } + public DhBlockPos convertToChunkRelativePos() { return this.mutateToChunkRelativePos(null); } /** * Limits the block position to a value between 0 and 15 (inclusive) * If not null, mutates "mutableBlockPos" */ - public DhBlockPos convertToChunkRelativePos(@Nullable DhBlockPos mutableBlockPos) + public DhBlockPos mutateToChunkRelativePos(@Nullable DhBlockPos mutableBlockPos) { // move the position into the range -15 and +15 int relX = (this.x % LodUtil.CHUNK_WIDTH); diff --git a/core/src/main/java/com/seibel/distanthorizons/core/wrapperInterfaces/chunk/IChunkWrapper.java b/core/src/main/java/com/seibel/distanthorizons/core/wrapperInterfaces/chunk/IChunkWrapper.java index da6b8421e..e8f7a56d9 100644 --- a/core/src/main/java/com/seibel/distanthorizons/core/wrapperInterfaces/chunk/IChunkWrapper.java +++ b/core/src/main/java/com/seibel/distanthorizons/core/wrapperInterfaces/chunk/IChunkWrapper.java @@ -28,6 +28,7 @@ import com.seibel.distanthorizons.coreapi.interfaces.dependencyInjection.IBindab import com.seibel.distanthorizons.core.util.LodUtil; import com.seibel.distanthorizons.core.wrapperInterfaces.world.IBiomeWrapper; +import java.util.ArrayList; import java.util.List; public interface IChunkWrapper extends IBindable @@ -98,7 +99,7 @@ public interface IChunkWrapper extends IBindable - List getBlockLightPosList(); + ArrayList getBlockLightPosList(); default boolean blockPosInsideChunk(DhBlockPos blockPos) { return this.blockPosInsideChunk(blockPos.x, blockPos.y, blockPos.z); }