BitsAndChisels icon indicating copy to clipboard operation
BitsAndChisels copied to clipboard

Stutter when reading large number B&C block entities on the client

Open SquidDev opened this issue 2 years ago • 0 comments

I've been drying to debug some slight FPS stutter I've been having on the client. One of the (many!) causes of this appears to be due to deserialising B&C block entities on the client.

Taking a client-side profile using F3+L (plus some custom instrumentation to also track BE deserialisation) shows that reading 503 bitsandchisels:bits_block_entitys takes approximately 204ms.

Version: 1.18.2
Time span: 583 ms
Tick span: 1 ticks
// This is approximately 1.71 ticks per second. It should be 20 ticks per second

--- BEGIN PROFILE DUMP ---

[00] scheduledExecutables(1/1) - 94.71%/94.71%
[01] |   #Loading ecotones:treetap 10/10
[01] |   #Loading computercraft:computer_advanced 2/2
[01] |   #Loading bitsandchisels:bits_block_entity 508/508
[01] |   #Loading botania:agricarnation 1/1
[01] |   unspecified(1/1) - 61.82%/58.55%
[01] |   BlockEntity deserialisation(521/521) - 38.14%/36.12%
[02] |   |   bitsandchisels:bits_block_entity(508/508) - 98.62%/35.62%
[02] |   |   unspecified(521/521) - 1.34%/0.49%
[02] |   |   ecotones:treetap(10/10) - 0.02%/0.01%
[02] |   |   computercraft:computer_advanced(2/2) - 0.01%/0.00%
[02] |   |   botania:agricarnation(1/1) - 0.00%/0.00%
[01] |   Purple Water Chunk Sync(90/90) - 0.05%/0.04%

This only happens on a single frame when moving across chunk boundaries, and so this doesn't show up on normally on Spark/VisualVM profiles. Using spark with a very low sampling interval and setting --only-ticks-over 500 gave me a report like this.

Some ideas for optimisations

I made a couple of small changes in the patch below, which helped a bit but not as much as I might have liked:

0001-Some-micro-optimisations.patch
commit 65d67a808eeb43e9ae8c2af8976e594e09ca1af3
Author: Jonathan Coates <[email protected]>
Date:   Sat Jun 11 23:37:30 2022 +0100

    Some micro-optimisations

diff --git a/src/main/java/io/github/coolmineman/bitsandchisels/BitMeshes.java b/src/main/java/io/github/coolmineman/bitsandchisels/BitMeshes.java
index f235d78..96855f5 100644
--- a/src/main/java/io/github/coolmineman/bitsandchisels/BitMeshes.java
+++ b/src/main/java/io/github/coolmineman/bitsandchisels/BitMeshes.java
@@ -27,7 +27,7 @@ public class BitMeshes {
     private static final Direction[] X_DIRECTIONS = {Direction.EAST, Direction.WEST};
     private static final Direction[] Y_DIRECTIONS = {Direction.UP, Direction.DOWN};
     private static final Direction[] Z_DIRECTIONS = {Direction.SOUTH, Direction.NORTH};
-    private static final float ONE_PIXEL = 1f/16f; 
+    private static final float ONE_PIXEL = 1f/16f;
 
     private BitMeshes() { }
 
@@ -35,7 +35,7 @@ public class BitMeshes {
         MeshBuilder builder = RendererAccess.INSTANCE.getRenderer().meshBuilder();
         QuadEmitter emitter = builder.getEmitter();
         Vec3f tmp = new Vec3f();
-        boolean[][] used = new boolean[16][16];
+        short[] used = new short[16];
 
         //X
         for (Direction d : X_DIRECTIONS) {
@@ -44,12 +44,12 @@ public class BitMeshes {
                 for (int cy = 0; cy < 16; cy++) {
                     for (int cz = 0; cz < 16; cz++) {
                         BlockState state = states[cx][cy][cz];
-                        if (state.isAir() || used[cy][cz] || !quadNeeded(states, d, cx, cy, cz)) continue;
+                        if (state.isAir() || isUsed(used, cy, cz) || !quadNeeded(states, d, cx, cy, cz)) continue;
                         int cy2 = cy;
                         int cz2 = cz;
                         //Greed Y
                         for (int ty = cy; ty < 16; ty++) {
-                            if (states[cx][ty][cz] == state && !used[ty][cz] && quadNeeded(states, d, cx, ty, cz)) {
+                            if (states[cx][ty][cz] == state && !isUsed(used, ty, cz) && quadNeeded(states, d, cx, ty, cz)) {
                                 cy2 = ty;
                             } else {
                                 break;
@@ -58,7 +58,7 @@ public class BitMeshes {
                         // Greed Z
                         greedz: for (int tz = cz; tz < 16; tz++) {
                             for (int ty = cy; ty <= cy2; ty++) {
-                                if (states[cx][ty][tz] != state || used[ty][tz] || !quadNeeded(states, d, cx, ty, tz)) {
+                                if (states[cx][ty][tz] != state || isUsed(used, ty, tz) || !quadNeeded(states, d, cx, ty, tz)) {
                                     break greedz;
                                 }
                             }
@@ -66,7 +66,7 @@ public class BitMeshes {
                         }
                         for (int qy = cy; qy <= cy2; qy++) {
                             for (int qz = cz; qz <= cz2; qz++) {
-                                used[qy][qz] = true;
+                                used[qy] |= 1 << qz;
                             }
                         }
 
@@ -83,12 +83,12 @@ public class BitMeshes {
                 for (int cx = 0; cx < 16; cx++) {
                     for (int cz = 0; cz < 16; cz++) {
                         BlockState state = states[cx][cy][cz];
-                        if (state.isAir() || used[cx][cz] || !quadNeeded(states, d, cx, cy, cz)) continue;
+                        if (state.isAir() || isUsed(used, cx, cz) || !quadNeeded(states, d, cx, cy, cz)) continue;
                         int cx2 = cx;
                         int cz2 = cz;
                         //Greed X
                         for (int tx = cx; tx < 16; tx++) {
-                            if (states[tx][cy][cz] == state && !used[tx][cz] && quadNeeded(states, d, tx, cy, cz)) {
+                            if (states[tx][cy][cz] == state && !isUsed(used, tx, cz) && quadNeeded(states, d, tx, cy, cz)) {
                                 cx2 = tx;
                             } else {
                                 break;
@@ -97,7 +97,7 @@ public class BitMeshes {
                         // Greed Z
                         greedz: for (int tz = cz; tz < 16; tz++) {
                             for (int tx = cx; tx <= cx2; tx++) {
-                                if (states[tx][cy][tz] != state || used[tx][tz] || !quadNeeded(states, d, tx, cy, tz)) {
+                                if (states[tx][cy][tz] != state || isUsed(used, tx, tz) || !quadNeeded(states, d, tx, cy, tz)) {
                                     break greedz;
                                 }
                             }
@@ -105,7 +105,7 @@ public class BitMeshes {
                         }
                         for (int qx = cx; qx <= cx2; qx++) {
                             for (int qz = cz; qz <= cz2; qz++) {
-                                used[qx][qz] = true;
+                                used[qx] |= 1 << qz;
                             }
                         }
 
@@ -122,12 +122,12 @@ public class BitMeshes {
                 for (int cx = 0; cx < 16; cx++) {
                     for (int cy = 0; cy < 16; cy++) {
                         BlockState state = states[cx][cy][cz];
-                        if (state.isAir() || used[cx][cy] || !quadNeeded(states, d, cx, cy, cz)) continue;
+                        if (state.isAir() || isUsed(used, cx, cy) || !quadNeeded(states, d, cx, cy, cz)) continue;
                         int cx2 = cx;
                         int cy2 = cy;
                         //Greed X
                         for (int tx = cx; tx < 16; tx++) {
-                            if (states[tx][cy][cz] == state && !used[tx][cz] && quadNeeded(states, d, tx, cy, cz)) {
+                            if (states[tx][cy][cz] == state && !isUsed(used, tx, cz) && quadNeeded(states, d, tx, cy, cz)) {
                                 cx2 = tx;
                             } else {
                                 break;
@@ -136,7 +136,7 @@ public class BitMeshes {
                         // Greed Y
                         greedy: for (int ty = cy; ty < 16; ty++) {
                             for (int tx = cx; tx <= cx2; tx++) {
-                                if (states[tx][ty][cz] != state || used[tx][ty] || !quadNeeded(states, d, tx, ty, cz)) {
+                                if (states[tx][ty][cz] != state || isUsed(used, tx, ty) || !quadNeeded(states, d, tx, ty, cz)) {
                                     break greedy;
                                 }
                             }
@@ -144,7 +144,7 @@ public class BitMeshes {
                         }
                         for (int qx = cx; qx <= cx2; qx++) {
                             for (int qy = cy; qy <= cy2; qy++) {
-                                used[qx][qy] = true;
+                                used[qx] |= 1 << qy;
                             }
                         }
 
@@ -173,32 +173,46 @@ public class BitMeshes {
 
     private static boolean quadNeeded(BlockState[][][] states, Direction d, int x, int y, int z) {
         switch (d) {
-            case UP:
-                if (y <= 14) return states[x][y + 1][z].isAir() || (RenderLayers.getBlockLayer(states[x][y + 1][z]) != RenderLayer.getSolid() && states[x][y][z] != states[x][y + 1][z]);
-                return true;
-            case DOWN:
-                if (y >= 1) return states[x][y - 1][z].isAir() || (RenderLayers.getBlockLayer(states[x][y - 1][z]) != RenderLayer.getSolid() && states[x][y][z] != states[x][y - 1][z]);
-                return true;
-            case SOUTH:
-                if (z <= 14) return states[x][y][z + 1].isAir() || (RenderLayers.getBlockLayer(states[x][y][z + 1]) != RenderLayer.getSolid() && states[x][y][z] != states[x][y][z + 1]);
-                return true;
-            case NORTH:
-                if (z >= 1) return states[x][y][z - 1].isAir() || (RenderLayers.getBlockLayer(states[x][y][z - 1]) != RenderLayer.getSolid() && states[x][y][z] != states[x][y][z - 1]);
-                return true;
-            case EAST:
-                if (x <= 14) return states[x + 1][y][z].isAir() || (RenderLayers.getBlockLayer(states[x + 1][y][z]) != RenderLayer.getSolid() && states[x][y][z] != states[x + 1][y][z]);
-                return true;
-            case WEST:
-                if (x >= 1) return states[x - 1][y][z].isAir() || (RenderLayers.getBlockLayer(states[x - 1][y][z]) != RenderLayer.getSolid() && states[x][y][z] != states[x - 1][y][z]);
-                return true;
+            case UP: {
+                if (y > 14) return true;
+                BlockState state = states[x][y + 1][z];
+                return state.isAir() || (states[x][y][z] != state && RenderLayers.getBlockLayer(state) != RenderLayer.getSolid());
+            }
+            case DOWN: {
+                if (y < 1) return true;
+                BlockState state = states[x][y - 1][z];
+                return state.isAir() || (states[x][y][z] != state && RenderLayers.getBlockLayer(state) != RenderLayer.getSolid());
+            }
+            case SOUTH: {
+                if (z > 14) return true;
+                BlockState state = states[x][y][z + 1];
+                return state.isAir() || (states[x][y][z] != state && RenderLayers.getBlockLayer(state) != RenderLayer.getSolid());
+            }
+            case NORTH: {
+                if (z < 1) return true;
+                BlockState state = states[x][y][z - 1];
+                return state.isAir() || (states[x][y][z] != state && RenderLayers.getBlockLayer(state) != RenderLayer.getSolid());
+            }
+            case EAST: {
+                if (x > 14) return true;
+                BlockState state = states[x + 1][y][z];
+                return state.isAir() || (states[x][y][z] != state && RenderLayers.getBlockLayer(state) != RenderLayer.getSolid());
+            }
+            case WEST: {
+                if (x < 1) return true;
+                BlockState state = states[x - 1][y][z];
+                return state.isAir() || (states[x][y][z] != state && RenderLayers.getBlockLayer(state) != RenderLayer.getSolid());
+            }
         }
         return true;
     }
 
-    private static void clear(boolean[][] a) {
-        for(int i = 0; i < a.length; i++) {
-            Arrays.fill(a[i], false);
-        }
+    private static void clear(short[] a) {
+        Arrays.fill(a, (short) 0);
+    }
+
+    private static boolean isUsed(short[] a, int x, int y) {
+        return (a[x] & (1 << y)) != 0;
     }
 
     private static boolean canCull(Direction d, int x, int y, int z) {
@@ -268,5 +282,5 @@ public class BitMeshes {
         float diff = Math.abs(desiredValue - actualValue);
         return diff < 0.01f;
     }
-    
+
 }
diff --git a/src/main/java/io/github/coolmineman/bitsandchisels/BitsBlockEntity.java b/src/main/java/io/github/coolmineman/bitsandchisels/BitsBlockEntity.java
index 76fcaed..19c7fd5 100644
--- a/src/main/java/io/github/coolmineman/bitsandchisels/BitsBlockEntity.java
+++ b/src/main/java/io/github/coolmineman/bitsandchisels/BitsBlockEntity.java
@@ -22,6 +22,8 @@ import net.minecraft.util.shape.BitSetVoxelSet;
 import net.minecraft.util.shape.VoxelShape;
 import net.minecraft.util.shape.VoxelShapes;
 
+import java.util.Arrays;
+
 public class BitsBlockEntity extends BlockEntity implements RenderAttachmentBlockEntity {
     private BlockState[][][] states;
     @Environment(EnvType.CLIENT)
@@ -38,9 +40,7 @@ public class BitsBlockEntity extends BlockEntity implements RenderAttachmentBloc
         this(new BlockState[16][16][16], pos, state, alive);
         for (int i = 0; i < 16; i++) {
             for (int j = 0; j < 16; j++) {
-                for (int k = 0; k < 16; k++) {
-                    states[i][j][k] = fillState;
-                }
+                Arrays.fill(states[i][j], fillState);
             }
         }
     }
  • Use a short[] instead of a boolean[][16]. Tiny micro-opimisation, but does actually help a bit with clearing the array.
  • Recorder conditionals in quadNeeded to calls to RenderLayers.getBlockLayer.

There's some other possible changes I can think of which might help[^1] but are more invasive:

  • Switch to using a BlockState[16 * 16 * 16] instead of a BlockState[16][16][16].

  • Don't call BitsBlockEntity.rebuildNbtCache on the client (actually, this isn't invasive and is probably an easy win!)

  • Move model baking off-thread. I don't actually know how feasible this is - what I'm thinking is make getRenderAttachmentData return a object which computes (and caches!) the model as part of chunk baking instead of on the main thread.

    This would of course massively complicate things - you'd still need to compute block colours on the render thread. It may just not be possible or not give you any performance gains.

[^1]: Emphasis on might! I'm afraid I haven't implemented any of these, let alone profiled them.

SquidDev avatar Jun 11 '22 22:06 SquidDev