Batch ticket ticks on fabric, forge and neoforge for lower server thread overhead

common/src/main/java/org/popcraft/chunky/GenerationTask.java

@@ -6,6 +6,7 @@
 import org.popcraft.chunky.event.task.GenerationTaskUpdateEvent;
 import org.popcraft.chunky.iterator.ChunkIterator;
 import org.popcraft.chunky.iterator.ChunkIteratorFactory;
+import org.popcraft.chunky.platform.Batcher;
 import org.popcraft.chunky.platform.Sender;
 import org.popcraft.chunky.shape.Shape;
 import org.popcraft.chunky.shape.ShapeFactory;
@@ -22,7 +23,7 @@
 import java.util.concurrent.atomic.AtomicLong;
 
 public class GenerationTask implements Runnable {
-    private static final int MAX_WORKING_COUNT = Input.tryInteger(System.getProperty("chunky.maxWorkingCount")).orElse(50);
+    public static final int MAX_WORKING_COUNT = Input.tryInteger(System.getProperty("chunky.maxWorkingCount")).orElse(50);
     private static final double SAMPLE_INTERVAL = 1000d * Math.max(Input.tryInteger(System.getProperty("chunky.sampleInterval")).orElse(30), 30);
     private static final double SAMPLE_SUB_INTERVAL = SAMPLE_INTERVAL / 30;
     private final Chunky chunky;
@@ -123,6 +124,8 @@ public void run() {
         }
         final Semaphore working = new Semaphore(MAX_WORKING_COUNT);
         final boolean forceLoadExistingChunks = chunky.getConfig().isForceLoadExistingChunks();
+        final Batcher batcher = selection.world().getBatcher();
+        batcher.resume();
         startTime.set(System.currentTimeMillis());
         while (!stopped && chunkIterator.hasNext()) {
             final ChunkCoordinate chunk = chunkIterator.next();
@@ -158,6 +161,7 @@ public void run() {
                         update(chunk.x(), chunk.z(), true);
                     });
         }
+        batcher.shutdown();
         if (stopped) {
             chunky.getServer().getConsole().sendMessagePrefixed(TranslationKey.TASK_STOPPED, selection.world().getName());
         } else {

common/src/main/java/org/popcraft/chunky/platform/Batcher.java

@@ -0,0 +1,7 @@
+package org.popcraft.chunky.platform;
+
+public interface Batcher {
+    void resume();
+
+    void shutdown();
+}

common/src/main/java/org/popcraft/chunky/platform/World.java

@@ -1,5 +1,6 @@
 package org.popcraft.chunky.platform;
 
+import org.popcraft.chunky.platform.impl.batcher.NOOPBatcher;
 import org.popcraft.chunky.platform.util.Location;
 
 import java.nio.file.Path;
@@ -49,4 +50,8 @@ default Optional<Path> getPOIDirectory() {
     default Optional<Path> getRegionDirectory() {
         return getDirectory("region");
     }
+
+    default Batcher getBatcher() {
+        return NOOPBatcher.INSTANCE;
+    }
 }

common/src/main/java/org/popcraft/chunky/platform/impl/batcher/AbstractBatcher.java

@@ -0,0 +1,114 @@
+package org.popcraft.chunky.platform.impl.batcher;
+
+import org.popcraft.chunky.GenerationTask;
+import org.popcraft.chunky.platform.Batcher;
+import org.popcraft.chunky.util.Input;
+
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.Executor;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+public abstract class AbstractBatcher implements Batcher {
+    public static final int BATCH_DIVISOR = Input.tryInteger(System.getProperty("chunky.batchDivisor")).orElse(4);
+    protected final ConcurrentLinkedQueue<Runnable> ticketAddTasks = new ConcurrentLinkedQueue<>();
+    protected final ConcurrentLinkedQueue<Runnable> ticketRemoveTasks = new ConcurrentLinkedQueue<>();
+    protected final ConcurrentLinkedQueue<Runnable> chunkLoadTasks = new ConcurrentLinkedQueue<>();
+    protected final Executor ticketAddExecutor = command -> {
+        if (this.shutdown.get()) {
+            this.runSync(command);
+        } else {
+            this.ticketAddTasks.add(command);
+            this.scheduleIfReady();
+        }
+    };
+    protected final Executor ticketRemoveExecutor = command -> {
+        if (this.shutdown.get()) {
+            this.runSync(command);
+        } else {
+            this.ticketRemoveTasks.add(command);
+            this.scheduleIfReady();
+        }
+    };
+    protected final Executor chunkLoadExecutor = command -> {
+        if (this.shutdown.get()) {
+            this.runSync(() -> {
+                this.tickTickets();
+                command.run();
+            });
+        } else {
+            this.chunkLoadTasks.add(command);
+        }
+    };
+    protected final int batchSize = Math.max(1, GenerationTask.MAX_WORKING_COUNT / BATCH_DIVISOR);
+    protected final AtomicBoolean scheduled = new AtomicBoolean(false);
+    protected final AtomicBoolean shutdown = new AtomicBoolean(true);
+
+    protected abstract void tickTickets();
+
+    protected abstract void runSync(Runnable command);
+
+    @Override
+    public void shutdown() {
+        if (this.shutdown.get()) {
+            throw new IllegalStateException("Batcher is already shutdown");
+        }
+        this.shutdown.set(true);
+        this.schedule();
+    }
+
+    @Override
+    public void resume() {
+        if (!this.shutdown.get()) {
+            throw new IllegalStateException("Batcher is running");
+        }
+        this.shutdown.set(false);
+    }
+
+    public Executor getTicketAddExecutor() {
+        return this.ticketAddExecutor;
+    }
+
+    public Executor getTicketRemoveExecutor() {
+        return this.ticketRemoveExecutor;
+    }
+
+    public Executor getChunkLoadExecutor() {
+        return this.chunkLoadExecutor;
+    }
+
+    private void processTaskQueue() {
+        final boolean wasShutdown = this.shutdown.get();
+        try {
+            runTasks(this.ticketAddTasks);
+            runTasks(this.ticketRemoveTasks);
+            this.tickTickets();
+            runTasks(this.chunkLoadTasks);
+        } finally {
+            this.scheduled.set(false);
+        }
+        this.scheduleIfReady();
+        if (!wasShutdown && this.shutdown.get()) {
+            this.schedule();
+        }
+    }
+
+    private void runTasks(final Queue<Runnable> queue) {
+        Runnable r;
+        while ((r = queue.poll()) != null) {
+            r.run();
+        }
+    }
+
+    private void schedule() {
+        if (this.scheduled.compareAndSet(false, true)) {
+            this.runSync(this::processTaskQueue);
+        }
+    }
+
+    private void scheduleIfReady() {
+        if (this.ticketRemoveTasks.size() >= this.batchSize || this.ticketAddTasks.size() >= this.batchSize) {
+            this.schedule();
+        }
+    }
+}

common/src/main/java/org/popcraft/chunky/platform/impl/batcher/NOOPBatcher.java

@@ -0,0 +1,18 @@
+package org.popcraft.chunky.platform.impl.batcher;
+
+import org.popcraft.chunky.platform.Batcher;
+
+public class NOOPBatcher implements Batcher {
+    public static final NOOPBatcher INSTANCE = new NOOPBatcher();
+
+    private NOOPBatcher() {
+    }
+
+    @Override
+    public void resume() {
+    }
+
+    @Override
+    public void shutdown() {
+    }
+}

fabric/src/main/java/org/popcraft/chunky/mixin/ChunkMapMixin.java

@@ -3,8 +3,10 @@
 import net.minecraft.nbt.CompoundTag;
 import net.minecraft.server.level.ChunkHolder;
 import net.minecraft.server.level.ChunkMap;
+import net.minecraft.util.thread.BlockableEventLoop;
 import net.minecraft.world.level.ChunkPos;
 import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.gen.Accessor;
 import org.spongepowered.asm.mixin.gen.Invoker;
 
 import java.util.Optional;
@@ -22,4 +24,7 @@ public interface ChunkMapMixin {
 
     @Invoker
     void invokeTick(BooleanSupplier booleanSupplier);
+
+    @Accessor
+    BlockableEventLoop<Runnable> getMainThreadExecutor();
 }

fabric/src/main/java/org/popcraft/chunky/mixin/MinecraftServerMixin.java

@@ -2,8 +2,8 @@
 
 import net.minecraft.server.MinecraftServer;
 import net.minecraft.server.level.ServerLevel;
+import net.minecraft.util.profiling.InactiveProfiler;
 import org.popcraft.chunky.ChunkyFabric;
-import org.popcraft.chunky.ChunkyProvider;
 import org.popcraft.chunky.ducks.MinecraftServerExtension;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.Shadow;
@@ -32,7 +32,8 @@ private void tickPaused(BooleanSupplier booleanSupplier, CallbackInfo ci) {
     public void chunky$runChunkSystemHousekeeping(BooleanSupplier haveTime) {
         if (this.chunky$needChunkSystemHousekeeping.compareAndSet(true, false)) {
             for (ServerLevel level : this.getAllLevels()) {
-                ((ChunkMapMixin) level.getChunkSource().chunkMap).invokeTick(haveTime);
+                ((ChunkMapMixin) level.getChunkSource().chunkMap).invokeTick(() -> true); // push the vanilla chunk system to unload unneeded chunks ASAP
+                ((ServerChunkCacheMixin) level.getChunkSource()).invokeBroadcastChangedChunks(InactiveProfiler.INSTANCE);
                 if (!ChunkyFabric.ENABLE_MOONRISE_WORKAROUNDS) {
                     // note: Moonrise destroys the vanilla entity system, so skip it here if it's present
                     ((ServerLevelMixin) level).getEntityManager().tick();

fabric/src/main/java/org/popcraft/chunky/mixin/ServerChunkCacheMixin.java

@@ -2,6 +2,7 @@
 
 import net.minecraft.server.level.ChunkResult;
 import net.minecraft.server.level.ServerChunkCache;
+import net.minecraft.util.profiling.ProfilerFiller;
 import net.minecraft.world.level.chunk.ChunkAccess;
 import net.minecraft.world.level.chunk.status.ChunkStatus;
 import org.spongepowered.asm.mixin.Mixin;
@@ -20,4 +21,7 @@ public CompletableFuture<ChunkResult<ChunkAccess>> invokeGetChunkFutureMainThrea
 
     @Invoker
     boolean invokeRunDistanceManagerUpdates();
+
+    @Invoker
+    void invokeBroadcastChangedChunks(ProfilerFiller arg);
 }

fabric/src/main/java/org/popcraft/chunky/platform/FabricBatcher.java

@@ -0,0 +1,25 @@
+package org.popcraft.chunky.platform;
+
+import net.minecraft.server.MinecraftServer;
+import net.minecraft.server.level.ServerLevel;
+import org.popcraft.chunky.mixin.ServerChunkCacheMixin;
+import org.popcraft.chunky.platform.impl.batcher.AbstractBatcher;
+
+public class FabricBatcher extends AbstractBatcher {
+    private final ServerLevel world;
+
+    public FabricBatcher(final ServerLevel world) {
+        this.world = world;
+    }
+
+    @Override
+    protected void tickTickets() {
+        ((ServerChunkCacheMixin) this.world.getChunkSource()).invokeRunDistanceManagerUpdates();
+    }
+
+    @Override
+    protected void runSync(final Runnable command) {
+        final MinecraftServer server = this.world.getServer();
+        server.schedule(server.wrapRunnable(command));
+    }
+}

fabric/src/main/java/org/popcraft/chunky/platform/FabricWorld.java

@@ -42,10 +42,12 @@ public class FabricWorld implements World {
     private static final boolean UPDATE_CHUNK_NBT = Boolean.getBoolean("chunky.updateChunkNbt");
     private final ServerLevel world;
     private final Border worldBorder;
+    private final FabricBatcher batcher;
 
     public FabricWorld(final ServerLevel world) {
         this.world = world;
         this.worldBorder = new FabricBorder(world.getWorldBorder());
+        this.batcher = new FabricBatcher(world);
     }
 
     @Override
@@ -95,28 +97,30 @@ public CompletableFuture<Boolean> isChunkGenerated(final int x, final int z) {
     @Override
     public CompletableFuture<Void> getChunkAtAsync(final int x, final int z) {
         if (Thread.currentThread() != world.getServer().getRunningThread()) {
-            return CompletableFuture.supplyAsync(() -> getChunkAtAsync(x, z), world.getServer()).thenCompose(Function.identity());
+            return CompletableFuture.supplyAsync(() -> getChunkAtAsync(x, z), this.batcher.getTicketAddExecutor()).thenCompose(Function.identity());
         } else {
             final ChunkPos chunkPos = new ChunkPos(x, z);
             final ServerChunkCache serverChunkCache = world.getChunkSource();
             serverChunkCache.addTicketWithRadius(CHUNKY, chunkPos, 0);
             if (TICKING_LOAD_DURATION > 0) {
                 serverChunkCache.addTicketWithRadius(CHUNKY_TICKING, chunkPos, 1);
             }
-            ((ServerChunkCacheMixin) serverChunkCache).invokeRunDistanceManagerUpdates();
-            // note: when Moonrise is present, holders do not get created most of the time even after explicit distance manager update
-            // so we force `create = true` *only if* Moonrise is present, as it breaks pausing for everyone else
-            boolean create = ChunkyFabric.ENABLE_MOONRISE_WORKAROUNDS;
-            return ((ServerChunkCacheMixin) world.getChunkSource()).invokeGetChunkFutureMainThread(x, z, ChunkStatus.FULL, create)
-                    .whenCompleteAsync((ignored, throwable) -> {
-                        serverChunkCache.removeTicketWithRadius(CHUNKY, chunkPos, 0);
-                        ((MinecraftServerExtension) world.getServer()).chunky$markChunkSystemHousekeeping();
-                        if (ChunkyFabric.ENABLE_MOONRISE_WORKAROUNDS) {
-                            // note: to prevent pausing on dedicated server when Moonrise is present
-                            ((MinecraftServerAccess) world.getServer()).setEmptyTicks(0);
-                        }
-                    }, world.getServer())
-                    .thenApply(ignored -> null);
+            return CompletableFuture.supplyAsync(() -> {
+                // note: when Moonrise is present, holders do not get created most of the time even after explicit distance manager update
+                // so we force `create = true` *only if* Moonrise is present, as it breaks pausing for everyone else
+                boolean create = ChunkyFabric.ENABLE_MOONRISE_WORKAROUNDS;
+                return ((ServerChunkCacheMixin) world.getChunkSource()).invokeGetChunkFutureMainThread(x, z, ChunkStatus.FULL, create)
+                        .thenApplyAsync(Function.identity(), ((ChunkMapMixin) serverChunkCache.chunkMap).getMainThreadExecutor()) // workaround to prevent memory leaks in vanilla chunk system
+                        .whenCompleteAsync((ignored, throwable) -> {
+                            serverChunkCache.removeTicketWithRadius(CHUNKY, chunkPos, 0);
+                            ((MinecraftServerExtension) world.getServer()).chunky$markChunkSystemHousekeeping();
+                            if (ChunkyFabric.ENABLE_MOONRISE_WORKAROUNDS) {
+                                // note: to prevent pausing on dedicated server when Moonrise is present
+                                ((MinecraftServerAccess) world.getServer()).setEmptyTicks(0);
+                            }
+                        }, this.batcher.getTicketRemoveExecutor())
+                        .thenApply(ignored -> (Void) null);
+            }, this.batcher.getChunkLoadExecutor()).thenCompose(Function.identity());
         }
     }
 
@@ -193,6 +197,11 @@ public Optional<Path> getDirectory(final String name) {
         return Files.exists(directory) ? Optional.of(directory) : Optional.empty();
     }
 
+    @Override
+    public Batcher getBatcher() {
+        return this.batcher;
+    }
+
     public ServerLevel getWorld() {
         return world;
     }

forge/src/main/java/org/popcraft/chunky/ChunkyForge.java

@@ -46,6 +46,7 @@
 @Mod(ChunkyForge.MOD_ID)
 public class ChunkyForge {
     public static final String MOD_ID = "chunky";
+    public static final boolean ENABLE_MOONRISE_WORKAROUNDS = false;
     private Chunky chunky;
     private final Map<Identifier, ServerBossEvent> bossBars = new ConcurrentHashMap<>();
 

forge/src/main/java/org/popcraft/chunky/mixin/MinecraftServerMixin.java

@@ -2,6 +2,7 @@
 
 import net.minecraft.server.MinecraftServer;
 import net.minecraft.server.level.ServerLevel;
+import net.minecraft.util.profiling.InactiveProfiler;
 import org.popcraft.chunky.ducks.MinecraftServerExtension;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.Shadow;
@@ -31,6 +32,7 @@ private void tickPaused(BooleanSupplier booleanSupplier, CallbackInfo ci) {
         if (this.chunky$needChunkSystemHousekeeping.compareAndSet(true, false)) {
             for (ServerLevel level : this.getAllLevels()) {
                 level.getChunkSource().chunkMap.tick(haveTime);
+                level.getChunkSource().broadcastChangedChunks(InactiveProfiler.INSTANCE);
                 level.entityManager.tick();
             }
         }

forge/src/main/java/org/popcraft/chunky/platform/ForgeBatcher.java

@@ -0,0 +1,24 @@
+package org.popcraft.chunky.platform;
+
+import net.minecraft.server.MinecraftServer;
+import net.minecraft.server.level.ServerLevel;
+import org.popcraft.chunky.platform.impl.batcher.AbstractBatcher;
+
+public class ForgeBatcher extends AbstractBatcher {
+    private final ServerLevel world;
+
+    public ForgeBatcher(final ServerLevel world) {
+        this.world = world;
+    }
+
+    @Override
+    protected void tickTickets() {
+        this.world.getChunkSource().runDistanceManagerUpdates();
+    }
+
+    @Override
+    protected void runSync(final Runnable command) {
+        final MinecraftServer server = this.world.getServer();
+        server.schedule(server.wrapRunnable(command));
+    }
+}