Simplify chunk section cloning significantly

The complexity in trying to re-use objects makes no sense and only hurts performance while creating opportunity for concurrency bugs.
CaffeineMC · Aug 1, 2023 · a3111c1 · a3111c1
1 parent 262bdcb
commit a3111c1
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 233 deletions.
diff --git a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/RenderSectionManager.java b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/RenderSectionManager.java
@@ -360,8 +360,8 @@ public boolean isSectionVisible(int x, int y, int z) {
     }
 
     public void updateChunks(boolean updateImmediately) {
+        this.sectionCache.cleanup();
         this.regions.update();
-        this.sectionCache.update();
 
         this.submitRebuildTasks(ChunkUpdateType.IMPORTANT_REBUILD, false);
         this.submitRebuildTasks(ChunkUpdateType.REBUILD, !updateImmediately);
@@ -732,7 +732,6 @@ public Collection<String> getDebugStrings() {
                 this.rebuildQueues.get(ChunkUpdateType.REBUILD).size(),
                 this.rebuildQueues.get(ChunkUpdateType.INITIAL_BUILD).size())
         );
-        list.add("Chunk cache: " + this.sectionCache.getDebugString());
 
         return list;
     }

diff --git a/...in/java/me/jellysquid/mods/sodium/client/render/chunk/compile/executor/ChunkJobTyped.java b/...in/java/me/jellysquid/mods/sodium/client/render/chunk/compile/executor/ChunkJobTyped.java
@@ -1,6 +1,5 @@
 package me.jellysquid.mods.sodium.client.render.chunk.compile.executor;
 
-import me.jellysquid.mods.sodium.client.SodiumClientMod;
 import me.jellysquid.mods.sodium.client.render.chunk.compile.ChunkBuildContext;
 import me.jellysquid.mods.sodium.client.render.chunk.compile.tasks.ChunkBuilderTask;
 
@@ -50,8 +49,6 @@ public void execute(ChunkBuildContext context) {
         } catch (Throwable throwable) {
             result = ChunkJobResult.exceptionally(throwable);
             ChunkBuilder.LOGGER.error("Chunk build failed", throwable);
-        } finally {
-            this.task.releaseResources();
         }
 
         try {

diff --git a/.../me/jellysquid/mods/sodium/client/render/chunk/compile/tasks/ChunkBuilderMeshingTask.java b/.../me/jellysquid/mods/sodium/client/render/chunk/compile/tasks/ChunkBuilderMeshingTask.java
@@ -168,9 +168,4 @@ private CrashException fillCrashInfo(CrashReport report, WorldSlice slice, Block
 
         return new CrashException(report);
     }
-
-    @Override
-    public void releaseResources() {
-        this.renderContext.releaseResources();
-    }
 }
diff --git a/...in/java/me/jellysquid/mods/sodium/client/render/chunk/compile/tasks/ChunkBuilderTask.java b/...in/java/me/jellysquid/mods/sodium/client/render/chunk/compile/tasks/ChunkBuilderTask.java
@@ -24,6 +24,4 @@ public abstract class ChunkBuilderTask<OUTPUT> {
      *         if the task was cancelled.
      */
     public abstract OUTPUT execute(ChunkBuildContext context, CancellationToken cancellationToken);
-
-    public abstract void releaseResources();
 }
diff --git a/src/main/java/me/jellysquid/mods/sodium/client/world/cloned/ChunkRenderContext.java b/src/main/java/me/jellysquid/mods/sodium/client/world/cloned/ChunkRenderContext.java
@@ -26,12 +26,4 @@ public ChunkSectionPos getOrigin() {
     public BlockBox getVolume() {
         return this.volume;
     }
-
-    public void releaseResources() {
-        for (var section : this.sections) {
-            if (section != null) {
-                section.releaseReference();
-            }
-        }
-    }
 }
diff --git a/src/main/java/me/jellysquid/mods/sodium/client/world/cloned/ClonedChunkSection.java b/src/main/java/me/jellysquid/mods/sodium/client/world/cloned/ClonedChunkSection.java
@@ -1,6 +1,8 @@
 package me.jellysquid.mods.sodium.client.world.cloned;
 
-import it.unimi.dsi.fastutil.shorts.Short2ShortOpenHashMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMap;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceMaps;
+import it.unimi.dsi.fastutil.ints.Int2ReferenceOpenHashMap;
 import me.jellysquid.mods.sodium.client.world.cloned.palette.ClonedPalette;
 import me.jellysquid.mods.sodium.client.world.cloned.palette.ClonedPaletteFallback;
 import me.jellysquid.mods.sodium.client.world.cloned.palette.ClonedPalleteArray;
@@ -18,40 +20,28 @@
 import net.minecraft.world.World;
 import net.minecraft.world.biome.Biome;
 import net.minecraft.world.chunk.*;
-import org.apache.commons.lang3.Validate;
 
-import java.util.Arrays;
+import java.util.EnumMap;
 import java.util.Map;
-import java.util.concurrent.atomic.AtomicInteger;
 
 public class ClonedChunkSection {
     private static final LightType[] LIGHT_TYPES = LightType.values();
 
-    private final BlockEntity[] blockEntity = new BlockEntity[16 * 16 * 16];
-    private final Object[] blockEntityAttachments = new Object[16 * 16 * 16];
+    private Int2ReferenceMap<BlockEntity> blockEntities;
+    private Int2ReferenceMap<Object> blockEntityAttachments;
 
-    private final Short2ShortOpenHashMap blockEntityIds;
-    private final ChunkNibbleArray[] lightDataArrays;
+    private final ChunkNibbleArray[] lightDataArrays = new ChunkNibbleArray[LIGHT_TYPES.length];
 
-    private ChunkSectionPos pos;
+    private final ChunkSectionPos pos;
 
     private PackedIntegerArray blockStateData;
     private ClonedPalette<BlockState> blockStatePalette;
 
     private ReadableContainer<RegistryEntry<Biome>> biomeData;
 
-    private boolean empty = true;
-
-    ClonedChunkSection() {
-        this.blockEntityIds = new Short2ShortOpenHashMap();
-        this.blockEntityIds.defaultReturnValue((short) -1);
-
-        this.lightDataArrays = new ChunkNibbleArray[LIGHT_TYPES.length];
-    }
-
-    public void copy(World world, WorldChunk chunk, ChunkSection section, ChunkSectionPos pos) {
-        Validate.isTrue(this.empty);
+    private long lastUsedTimestamp = Long.MAX_VALUE;
 
+    public ClonedChunkSection(World world, WorldChunk chunk, ChunkSection section, ChunkSectionPos pos) {
         this.pos = pos;
 
         this.copyBlockData(section);
@@ -60,22 +50,6 @@ public void copy(World world, WorldChunk chunk, ChunkSection section, ChunkSecti
         this.copyBlockEntities(chunk, pos);
     }
 
-    void clear() {
-        Arrays.fill(this.blockEntity, 0, this.blockEntityIds.size(), null);
-        Arrays.fill(this.blockEntityAttachments, 0, this.blockEntityIds.size(), null);
-
-        this.blockEntityIds.clear();
-
-        this.blockStateData = null;
-        this.blockStatePalette = null;
-
-        this.biomeData = null;
-
-        Arrays.fill(this.lightDataArrays, null);
-
-        this.empty = true;
-    }
-
     private void copyBlockData(ChunkSection section) {
         PalettedContainer.Data<BlockState> container = ((PalettedContainerAccessor<BlockState>) section.getBlockStateContainer()).getData();
 
@@ -103,56 +77,53 @@ private void copyBlockEntities(WorldChunk chunk, ChunkSectionPos chunkCoord) {
         BlockBox box = new BlockBox(chunkCoord.getMinX(), chunkCoord.getMinY(), chunkCoord.getMinZ(),
                 chunkCoord.getMaxX(), chunkCoord.getMaxY(), chunkCoord.getMaxZ());
 
+        Int2ReferenceOpenHashMap<BlockEntity> blockEntities = null;
+
         // Copy the block entities from the chunk into our cloned section
         for (Map.Entry<BlockPos, BlockEntity> entry : chunk.getBlockEntities().entrySet()) {
             BlockPos pos = entry.getKey();
             BlockEntity entity = entry.getValue();
 
             if (box.contains(pos)) {
-                var id = (short) this.blockEntityIds.size();
-                var prev = this.blockEntityIds.put(ChunkSectionPos.packLocal(pos), id);
-
-                if (prev != this.blockEntityIds.defaultReturnValue()) {
-                    throw new IllegalStateException("Already inserted block entity at " + pos);
+                if (blockEntities == null) {
+                    blockEntities = new Int2ReferenceOpenHashMap<>();
                 }
 
-                this.blockEntity[id] = entity;
+                blockEntities.put(packLocal(pos.getX() & 15, pos.getY() & 15, pos.getZ() & 15), entity);
             }
         }
 
+        this.blockEntities = blockEntities != null ? blockEntities : Int2ReferenceMaps.emptyMap();
+
+        Int2ReferenceOpenHashMap<Object> blockEntityAttachments = null;
+
         // Retrieve any render attachments after we have copied all block entities, as this will call into the code of
         // other mods. This could potentially result in the chunk being modified, which would cause problems if we
         // were iterating over any data in that chunk.
         // See https://github.com/CaffeineMC/sodium-fabric/issues/942 for more info.
-        for (int i = 0; i < this.blockEntityIds.size(); i++) {
-            if (this.blockEntity[i] instanceof RenderAttachmentBlockEntity holder) {
-                this.blockEntityAttachments[i] = holder.getRenderAttachmentData();
+        for (var entry : Int2ReferenceMaps.fastIterable(this.blockEntities)) {
+            if (entry.getValue() instanceof RenderAttachmentBlockEntity holder) {
+                if (blockEntityAttachments == null) {
+                    blockEntityAttachments = new Int2ReferenceOpenHashMap<>();
+                }
+
+                blockEntityAttachments.put(entry.getIntKey(), holder.getRenderAttachmentData());
             }
         }
+
+        this.blockEntityAttachments = blockEntityAttachments != null ? blockEntityAttachments : Int2ReferenceMaps.emptyMap();
     }
 
     public RegistryEntry<Biome> getBiome(int x, int y, int z) {
         return this.biomeData.get(x, y, z);
     }
 
     public BlockEntity getBlockEntity(int x, int y, int z) {
-        var id = this.blockEntityIds.get(packLocal(x, y, z));
-
-        if (id < 0) {
-            return null;
-        }
-
-        return this.blockEntity[id];
+        return this.blockEntities.get(packLocal(x, y, z));
     }
 
     public Object getBlockEntityRenderAttachment(int x, int y, int z) {
-        var id = this.blockEntityIds.get(packLocal(x, y, z));
-
-        if (id < 0) {
-            return null;
-        }
-
-        return this.blockEntityAttachments[id];
+        return this.blockEntityAttachments.get(packLocal(x, y, z));
     }
 
     public PackedIntegerArray getBlockData() {
@@ -207,7 +178,7 @@ private static short packLocal(int x, int y, int z) {
     }
 
     public int getLightLevel(LightType type, int x, int y, int z) {
-        var array = this.lightDataArrays[type.ordinal()];
+        var array = this.getLightArray(type);
 
         // The sky-light array may not exist in certain dimensions.
         if (array == null) {
@@ -217,27 +188,11 @@ public int getLightLevel(LightType type, int x, int y, int z) {
         return array.get(x, y, z);
     }
 
-    private final AtomicInteger referenceCount = new AtomicInteger(0);
-
-    private long lastUsedTimestamp;
-
-    public int getReferenceCount() {
-        return this.referenceCount.get();
-    }
-
     public long getLastUsedTimestamp() {
         return this.lastUsedTimestamp;
     }
 
     public void setLastUsedTimestamp(long timestamp) {
         this.lastUsedTimestamp = timestamp;
     }
-
-    public void acquireReference() {
-        this.referenceCount.getAndIncrement();
-    }
-
-    public void releaseReference() {
-        this.referenceCount.getAndDecrement();
-    }
 }