Improve overall memory footprint per player (#359)

* fix: memory leak if player login is cancelled * feat: reduce memory footprint of proximity block data * chore: streamline nms instance constructor * fix: ci warning
Imprex-Development · Mar 1, 2024 · 43b6807 · 43b6807
1 parent 25eef37
commit 43b6807
Show file tree

Hide file tree

Showing 36 changed files with 323 additions and 308 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -11,20 +11,20 @@ jobs:
 
     steps:
     - name: Checkout
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
       with:
         fetch-depth: 0
     - name: Set BUILD_VERSION
       run: echo "BUILD_VERSION=$(git describe --tags $(git rev-list --tags --max-count=1))-b$GITHUB_RUN_NUMBER" >> $GITHUB_ENV
     - name: Maven cache
-      uses: actions/cache@v3
+      uses: actions/cache@v4
       with:
         path: ~/.m2/repository
         key: ${{ runner.os }}-maven-${{ secrets.CACHE_VERSION }}-${{ hashFiles('./.github/workflows/buildtools.sh') }}
         restore-keys: |
           ${{ runner.os }}-maven-${{ secrets.CACHE_VERSION }}-
     - name: Set up JDK 8/17
-      uses: actions/setup-java@v3
+      uses: actions/setup-java@v4
       with:
         distribution: 'temurin'
         java-version: |
@@ -38,7 +38,7 @@ jobs:
         mvn clean package --batch-mode -Drevision=$BUILD_VERSION
         mv orebfuscator-plugin/target/orebfuscator-*.jar ./
     - name: Upload artifacts
-      uses: actions/upload-artifact@v3
+      uses: actions/upload-artifact@v4
       with:
         name: orebfuscator-plugin
         path: ./orebfuscator-*.jar
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -11,20 +11,20 @@ jobs:
 
     steps:
     - name: Checkout
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
       with:
         fetch-depth: 0
     - name: Set RELEASE_VERSION
       run: echo "RELEASE_VERSION=${GITHUB_REF#refs/*/}" >> $GITHUB_ENV
     - name: Maven cache
-      uses: actions/cache@v3
+      uses: actions/cache@v4
       with:
         path: ~/.m2/repository
         key: ${{ runner.os }}-maven-${{ secrets.CACHE_VERSION }}-${{ hashFiles('./.github/workflows/buildtools.sh') }}
         restore-keys: |
           ${{ runner.os }}-maven-${{ secrets.CACHE_VERSION }}-
     - name: Set up JDK 8/17
-      uses: actions/setup-java@v3
+      uses: actions/setup-java@v4
       with:
         distribution: 'temurin'
         java-version: |

diff --git a/orebfuscator-common/src/main/java/net/imprex/orebfuscator/util/BlockPos.java b/orebfuscator-common/src/main/java/net/imprex/orebfuscator/util/BlockPos.java
@@ -58,6 +58,17 @@ public static BlockPos fromLong(long value) {
 		return new BlockPos(x, y, z);
 	}
 
+	public int toSectionPos() {
+		return (this.x & 0xF) << 12 | (this.y & 0xFFF) << 0 | (this.z & 0xF) << 16;
+	}
+
+	public static BlockPos fromSectionPos(int x, int z, int sectionPos) {
+		x    += (sectionPos >> 12) & 0xF;
+		int y = (sectionPos << 20 >> 20);
+		z    += (sectionPos >> 16) & 0xF;
+		return new BlockPos(x, y, z);
+	}
+
 	@Override
 	public int compareTo(BlockPos other) {
 		if (this.y == other.y) {

diff --git a/orebfuscator-common/src/main/java/net/imprex/orebfuscator/util/BlockProperties.java b/orebfuscator-common/src/main/java/net/imprex/orebfuscator/util/BlockProperties.java
@@ -1,6 +1,8 @@
 package net.imprex.orebfuscator.util;
 
+import java.util.HashSet;
 import java.util.Objects;
+import java.util.Set;
 
 import com.google.common.collect.ImmutableList;
 
@@ -12,12 +14,12 @@ public static Builder builder(NamespacedKey key) {
 
 	private final NamespacedKey key;
 	private final BlockStateProperties defaultBlockState;
-	private final ImmutableList<BlockStateProperties> possibleBlockStates;
+	private final ImmutableList<BlockStateProperties> blockStates;
 
 	private BlockProperties(Builder builder) {
 		this.key = builder.key;
 		this.defaultBlockState = builder.defaultBlockState;
-		this.possibleBlockStates = builder.possibleBlockStates;
+		this.blockStates = ImmutableList.copyOf(builder.blockStates);
 	}
 
 	public NamespacedKey getKey() {
@@ -28,8 +30,8 @@ public BlockStateProperties getDefaultBlockState() {
 		return defaultBlockState;
 	}
 
-	public ImmutableList<BlockStateProperties> getPossibleBlockStates() {
-		return possibleBlockStates;
+	public ImmutableList<BlockStateProperties> getBlockStates() {
+		return blockStates;
 	}
 
 	@Override
@@ -51,34 +53,44 @@ public boolean equals(Object obj) {
 
 	@Override
 	public String toString() {
-		return "BlockProperties [key=" + key + ", defaultBlockState=" + defaultBlockState + ", possibleBlockStates="
-				+ possibleBlockStates + "]";
+		return "BlockProperties [key=" + key + ", defaultBlockState=" + defaultBlockState + ", blockStates="
+				+ blockStates + "]";
 	}
 
 	public static class Builder {
 
 		private final NamespacedKey key;
 
 		private BlockStateProperties defaultBlockState;
-		private ImmutableList<BlockStateProperties> possibleBlockStates;
+		private final Set<BlockStateProperties> blockStates = new HashSet<>();
 
 		private Builder(NamespacedKey key) {
 			this.key = key;
 		}
 
-		public Builder withDefaultBlockState(BlockStateProperties defaultBlockState) {
-			this.defaultBlockState = defaultBlockState;
-			return this;
-		}
+		public Builder withBlockState(BlockStateProperties blockState) {
+			if (!blockStates.add(blockState)) {
+				throw new IllegalStateException(String.format("duplicate block state id (%s) for block: %s", blockState.getId(), key));
+			}
+
+			if (blockState.isDefaultState()) {
+				// check for multiple default blocks
+				if (this.defaultBlockState != null) {
+					throw new IllegalStateException(String.format("multiple default block states for block: %s", blockState.getId(), key));
+				}
+
+				this.defaultBlockState = blockState;
+			}
 
-		public Builder withPossibleBlockStates(ImmutableList<BlockStateProperties> possibleBlockStates) {
-			this.possibleBlockStates = possibleBlockStates;
 			return this;
 		}
-		
+
 		public BlockProperties build() {
-			Objects.requireNonNull(this.defaultBlockState, "missing default block state for " + this.key);
-			Objects.requireNonNull(this.possibleBlockStates, "missing possible block states for " + this.key);
+			Objects.requireNonNull(this.defaultBlockState, "missing default block state for block: " + this.key);
+
+			if (this.blockStates.size() == 0) {
+				throw new IllegalStateException("missing block states for block: " + this.key);
+			}
 
 			return new BlockProperties(this);
 		}

diff --git a/orebfuscator-common/src/main/java/net/imprex/orebfuscator/util/BlockStateProperties.java b/orebfuscator-common/src/main/java/net/imprex/orebfuscator/util/BlockStateProperties.java
@@ -11,12 +11,14 @@ public static Builder builder(int id) {
 	private final boolean isAir;
 	private final boolean isOccluding;
 	private final boolean isBlockEntity;
+	private final boolean isDefaultState;
 
 	private BlockStateProperties(Builder builder) {
 		this.id = builder.id;
 		this.isAir = builder.isAir;
 		this.isOccluding = builder.isOccluding;
 		this.isBlockEntity = builder.isBlockEntity;
+		this.isDefaultState = builder.isDefaultState;
 	}
 
 	public int getId() {
@@ -35,6 +37,10 @@ public boolean isBlockEntity() {
 		return isBlockEntity;
 	}
 
+	public boolean isDefaultState() {
+		return isDefaultState;
+	}
+
 	@Override
 	public int hashCode() {
 		return id;
@@ -54,8 +60,8 @@ public boolean equals(Object obj) {
 
 	@Override
 	public String toString() {
-		return "BlockStateProperties [id=" + id + ", isAir=" + isAir + ", isOccluding=" + isOccluding
-				+ ", isBlockEntity=" + isBlockEntity + "]";
+		return "BlockStateProperties [id=" + id + ", isDefaultState=" + isDefaultState + ", isAir=" + isAir
+				+ ", isOccluding=" + isOccluding + ", isBlockEntity=" + isBlockEntity + "]";
 	}
 
 	public static class Builder {
@@ -65,6 +71,7 @@ public static class Builder {
 		private boolean isAir;
 		private boolean isOccluding;
 		private boolean isBlockEntity;
+		private boolean isDefaultState;
 
 		private Builder(int id) {
 			this.id = id;
@@ -85,6 +92,11 @@ public Builder withIsBlockEntity(boolean isBlockEntity) {
 			return this;
 		}
 
+		public Builder withIsDefaultState(boolean isDefaultState) {
+			this.isDefaultState = isDefaultState;
+			return this;
+		}
+
 		public BlockStateProperties build() {
 			return new BlockStateProperties(this);
 		}

diff --git a/orebfuscator-common/src/test/java/net/imprex/orebfuscator/util/BlockPosTest.java b/orebfuscator-common/src/test/java/net/imprex/orebfuscator/util/BlockPosTest.java
@@ -0,0 +1,44 @@
+package net.imprex.orebfuscator.util;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+public class BlockPosTest {
+
+	@Test
+	public void testLongFormat() {
+		BlockPos positionA = new BlockPos(-52134, BlockPos.MAX_Y, 6243234);
+		BlockPos positionB = new BlockPos(0, BlockPos.MIN_Y, -4);
+		BlockPos positionC = new BlockPos(15, 0, -5663423);
+		BlockPos positionD = new BlockPos(21523, 16, -5663423);
+
+		long valueA = positionA.toLong();
+		long valueB = positionB.toLong();
+		long valueC = positionC.toLong();
+		long valueD = positionD.toLong();
+
+		assertEquals(positionA, BlockPos.fromLong(valueA));
+		assertEquals(positionB, BlockPos.fromLong(valueB));
+		assertEquals(positionC, BlockPos.fromLong(valueC));
+		assertEquals(positionD, BlockPos.fromLong(valueD));
+	}
+
+	@Test
+	public void testSectionPos() {
+		final int chunkX = -42 << 4;
+		final int chunkZ = 6521 << 4;
+
+		BlockPos positionA = new BlockPos(chunkX + 8, BlockPos.MAX_Y, chunkZ);
+		BlockPos positionB = new BlockPos(chunkX, BlockPos.MIN_Y, chunkZ + 15);
+		BlockPos positionC = new BlockPos(chunkX + 15, 0, chunkZ + 4);
+
+		int sectionPosA = positionA.toSectionPos();
+		int sectionPosB = positionB.toSectionPos();
+		int sectionPosC = positionC.toSectionPos();
+
+		assertEquals(positionA, BlockPos.fromSectionPos(chunkX, chunkZ, sectionPosA));
+		assertEquals(positionB, BlockPos.fromSectionPos(chunkX, chunkZ, sectionPosB));
+		assertEquals(positionC, BlockPos.fromSectionPos(chunkX, chunkZ, sectionPosC));
+	}
+}
diff --git a/...ms/orebfuscator-nms-api/src/main/java/net/imprex/orebfuscator/nms/AbstractNmsManager.java b/...ms/orebfuscator-nms-api/src/main/java/net/imprex/orebfuscator/nms/AbstractNmsManager.java
@@ -27,16 +27,12 @@ public AbstractNmsManager(int uniqueBlockStateCount, AbstractRegionFileCache<?>
 		this.blockStates = new BlockStateProperties[uniqueBlockStateCount];
 	}
 
-	protected final void registerBlockStateProperties(BlockStateProperties properties) {
-		this.blockStates[properties.getId()] = properties;
-	}
-
-	protected final void registerBlockProperties(BlockProperties properties) {
-		this.blocks.put(properties.getKey(), properties);
-	}
+	protected final void registerBlockProperties(BlockProperties block) {
+		this.blocks.put(block.getKey(), block);
 
-	protected final BlockStateProperties getBlockStateProperties(int id) {
-		return this.blockStates[id];
+		for (BlockStateProperties blockState : block.getBlockStates()) {
+			this.blockStates[blockState.getId()] = blockState;
+		}
 	}
 
 	@Override

diff --git a/...bfuscator-nms-v1_10_R1/src/main/java/net/imprex/orebfuscator/nms/v1_10_R1/NmsManager.java b/...bfuscator-nms-v1_10_R1/src/main/java/net/imprex/orebfuscator/nms/v1_10_R1/NmsManager.java
@@ -4,6 +4,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.bukkit.World;
 import org.bukkit.craftbukkit.v1_10_R1.CraftWorld;
@@ -82,33 +83,25 @@ public NmsManager(Config config) {
 			Block block = Block.REGISTRY.get(key);
 
 			ImmutableList<IBlockData> possibleBlockStates = block.t().a();
-			List<BlockStateProperties> possibleBlockStateProperties = new ArrayList<>();
+			BlockProperties.Builder builder = BlockProperties.builder(namespacedKey);
 
 			for (IBlockData blockState : possibleBlockStates) {
 
 				BlockStateProperties properties = BlockStateProperties.builder(getBlockId(blockState))
 						.withIsAir(block instanceof BlockAir)
 						/**
-						* p -> for barrier/slime_block/spawner
-						* j -> for every other block
-						*/
+						 * p -> for barrier/slime_block/spawner
+						 * j -> for every other block
+						 */
 						.withIsOccluding(blockState.p() && block.j()/*canOcclude*/)
 						.withIsBlockEntity(block.isTileEntity())
+						.withIsDefaultState(Objects.equals(block.getBlockData(), blockState))
 						.build();
 
-				possibleBlockStateProperties.add(properties);
-				this.registerBlockStateProperties(properties);
+				builder.withBlockState(properties);
 			}
 
-			int defaultBlockStateId = getBlockId(block.getBlockData());
-			BlockStateProperties defaultBlockState = getBlockStateProperties(defaultBlockStateId);
-
-			BlockProperties blockProperties = BlockProperties.builder(namespacedKey)
-				.withDefaultBlockState(defaultBlockState)
-				.withPossibleBlockStates(ImmutableList.copyOf(possibleBlockStateProperties))
-				.build();
-
-			this.registerBlockProperties(blockProperties);
+			this.registerBlockProperties(builder.build());
 		}
 	}
 

diff --git a/...bfuscator-nms-v1_11_R1/src/main/java/net/imprex/orebfuscator/nms/v1_11_R1/NmsManager.java b/...bfuscator-nms-v1_11_R1/src/main/java/net/imprex/orebfuscator/nms/v1_11_R1/NmsManager.java
@@ -4,6 +4,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.bukkit.World;
 import org.bukkit.craftbukkit.v1_11_R1.CraftWorld;
@@ -82,7 +83,7 @@ public NmsManager(Config config) {
 			Block block = Block.REGISTRY.get(key);
 
 			ImmutableList<IBlockData> possibleBlockStates = block.s().a();
-			List<BlockStateProperties> possibleBlockStateProperties = new ArrayList<>();
+			BlockProperties.Builder builder = BlockProperties.builder(namespacedKey);
 
 			for (IBlockData blockState : possibleBlockStates) {
 
@@ -94,21 +95,13 @@ public NmsManager(Config config) {
 						 */
 						.withIsOccluding(blockState.q() && blockState.s()/*canOcclude*/)
 						.withIsBlockEntity(block.isTileEntity())
+						.withIsDefaultState(Objects.equals(block.getBlockData(), blockState))
 						.build();
 
-				possibleBlockStateProperties.add(properties);
-				this.registerBlockStateProperties(properties);
+				builder.withBlockState(properties);
 			}
 
-			int defaultBlockStateId = getBlockId(block.getBlockData());
-			BlockStateProperties defaultBlockState = getBlockStateProperties(defaultBlockStateId);
-
-			BlockProperties blockProperties = BlockProperties.builder(namespacedKey)
-				.withDefaultBlockState(defaultBlockState)
-				.withPossibleBlockStates(ImmutableList.copyOf(possibleBlockStateProperties))
-				.build();
-
-			this.registerBlockProperties(blockProperties);
+			this.registerBlockProperties(builder.build());
 		}
 	}