From feac34581b0811ff0611bced1914761f041dc749 Mon Sep 17 00:00:00 2001 From: DesktopFolder Date: Sun, 17 Nov 2024 15:56:05 -0500 Subject: [PATCH 1/3] Fix: Attempt delayed thread creation as long-term viable f3f lag spike fix. --- .../render/chunk/ChunkRenderManager.java | 2 + .../render/chunk/compile/ChunkBuilder.java | 55 +++++++++++++++---- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/ChunkRenderManager.java b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/ChunkRenderManager.java index 6986d006..906fa896 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/ChunkRenderManager.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/ChunkRenderManager.java @@ -467,6 +467,8 @@ public void updateChunks() { if (!futures.isEmpty()) { this.backend.upload(RenderDevice.INSTANCE.createCommandList(), new FutureDequeDrain<>(futures)); } + + this.builder.maybeIncreaseThreadLimit(); } public void markDirty() { diff --git a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java index f52b7769..dccdd94f 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java @@ -51,6 +51,10 @@ public class ChunkBuilder { private BlockRenderPassManager renderPassManager; private final int limitThreads; + private final int initialThreads; + private boolean hasThreadSpace = true; + private int updates = 0; + private final ChunkVertexType vertexType; private final ChunkRenderBackend backend; @@ -58,6 +62,7 @@ public ChunkBuilder(ChunkVertexType vertexType, ChunkRenderBackend backend) { this.vertexType = vertexType; this.backend = backend; this.limitThreads = getOptimalThreadCount(); + this.initialThreads = Math.max(2, this.limitThreads / 6 /* Arbitrarily chosen :) */); } /** @@ -68,6 +73,19 @@ public int getSchedulingBudget() { return Math.max(0, (this.limitThreads * TASK_QUEUE_LIMIT_PER_WORKER) - this.buildQueue.size()); } + private void spawnThread(MinecraftClient client) { + ChunkBuildBuffers buffers = new ChunkBuildBuffers(this.vertexType, this.renderPassManager); + ChunkRenderCacheLocal pipeline = new ChunkRenderCacheLocal(client, this.world); + + WorkerRunnable worker = new WorkerRunnable(buffers, pipeline); + + Thread thread = new Thread(worker, "Chunk Render Task Executor #" + this.threads.size()); + thread.setPriority(Math.max(0, Thread.NORM_PRIORITY - 2)); + thread.start(); + + this.threads.add(thread); + } + /** * Spawns a number of work-stealing threads to process results in the build queue. If the builder is already * running, this method does nothing and exits. @@ -83,20 +101,28 @@ public void startWorkers() { MinecraftClient client = MinecraftClient.getInstance(); - for (int i = 0; i < this.limitThreads; i++) { - ChunkBuildBuffers buffers = new ChunkBuildBuffers(this.vertexType, this.renderPassManager); - ChunkRenderCacheLocal pipeline = new ChunkRenderCacheLocal(client, this.world); + for (int i = 0; i < this.initialThreads; i++) { + this.spawnThread(client); + } - WorkerRunnable worker = new WorkerRunnable(buffers, pipeline); + // If we still have overhead space for more threads, keep that information for later. + // We can start more threads if we need them for work in the future. + this.hasThreadSpace = this.threads.size() < this.limitThreads; - Thread thread = new Thread(worker, "Chunk Render Task Executor #" + i); - thread.setPriority(Math.max(0, Thread.NORM_PRIORITY - 2)); - thread.start(); + LOGGER.info("Started {} worker threads", this.threads.size()); + } - this.threads.add(thread); + public void maybeIncreaseThreadLimit() + { + // Sometimes, we want more threads :) + // In that case, let's slowly add them! + // :) Doesn't everyone love more threads? + if (!this.hasThreadSpace || (this.updates++ < 60 /* Arbitrary! */) || !this.running.get()) { + return; } - - LOGGER.info("Started {} worker threads", this.threads.size()); + this.updates = 0; // Only gradually increase thread count. + this.spawnThread(MinecraftClient.getInstance()); + this.hasThreadSpace = this.threads.size() < this.limitThreads; } /** @@ -113,7 +139,7 @@ public void stopWorkers() { throw new IllegalStateException("No threads are alive but the executor is in the RUNNING state"); } - LOGGER.info("Stopping worker threads"); + LOGGER.info("Stopping {} worker threads", this.threads.size()); // Notify all worker threads to wake up, where they will then terminate synchronized (this.jobNotifier) { @@ -184,6 +210,7 @@ public boolean isBuildQueueEmpty() { * Initializes this chunk builder for the given world. If the builder is already running (which can happen during * a world teleportation event), the worker threads will first be stopped and all pending tasks will be discarded * before being started again. + * Note: The above comment appears to be false. We only ever call init() once, directly after ChunkBuilder creation. * @param world The world instance * @param renderPassManager The render pass manager used for the world */ @@ -192,7 +219,11 @@ public void init(ClientWorld world, BlockRenderPassManager renderPassManager) { throw new NullPointerException("World is null"); } - this.stopWorkers(); + if (this.running.get()) { + throw new IllegalStateException("Init called on already-running chunk builder."); + } + + this.stopWorkers(); // Does nothing, ever. this.world = world; this.renderPassManager = renderPassManager; From aca35fc1b3b87b3c40fb6d1d60a8202d42fcc35e Mon Sep 17 00:00:00 2001 From: DesktopFolder Date: Sun, 17 Nov 2024 18:18:32 -0500 Subject: [PATCH 2/3] Compat: Change minimal code to prevent conflict with sq. --- .../render/chunk/compile/ChunkBuilder.java | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java index dccdd94f..2e17942d 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java @@ -62,7 +62,10 @@ public ChunkBuilder(ChunkVertexType vertexType, ChunkRenderBackend backend) { this.vertexType = vertexType; this.backend = backend; this.limitThreads = getOptimalThreadCount(); + // We assume nobody is running with 1 CPU, I guess. this.initialThreads = Math.max(2, this.limitThreads / 6 /* Arbitrarily chosen :) */); + + this.hasThreadSpace = this.limitThreads > this.initialThreads; } /** @@ -102,13 +105,21 @@ public void startWorkers() { MinecraftClient client = MinecraftClient.getInstance(); for (int i = 0; i < this.initialThreads; i++) { - this.spawnThread(client); - } + // Don't change any of this code (even though we factored it out into spawnThread()) because SeedQueue + // injects like crazy in here. + ChunkBuildBuffers buffers = new ChunkBuildBuffers(this.vertexType, this.renderPassManager); + ChunkRenderCacheLocal pipeline = new ChunkRenderCacheLocal(client, this.world); - // If we still have overhead space for more threads, keep that information for later. - // We can start more threads if we need them for work in the future. - this.hasThreadSpace = this.threads.size() < this.limitThreads; + WorkerRunnable worker = new WorkerRunnable(buffers, pipeline); + + Thread thread = new Thread(worker, "Chunk Render Task Executor #" + this.threads.size()); + thread.setPriority(Math.max(0, Thread.NORM_PRIORITY - 2)); + thread.start(); + this.threads.add(thread); + } + + // More SeedQueue compatibility, ideally we would log the number of threads. LOGGER.info("Started {} worker threads", this.threads.size()); } @@ -123,6 +134,10 @@ public void maybeIncreaseThreadLimit() this.updates = 0; // Only gradually increase thread count. this.spawnThread(MinecraftClient.getInstance()); this.hasThreadSpace = this.threads.size() < this.limitThreads; + if (!this.hasThreadSpace) + { + LOGGER.info("Maxed out chunk builder threads at {}", this.threads.size()); + } } /** @@ -139,7 +154,9 @@ public void stopWorkers() { throw new IllegalStateException("No threads are alive but the executor is in the RUNNING state"); } - LOGGER.info("Stopping {} worker threads", this.threads.size()); + // Temporary seedqueue compatibility - log less so that its inject works + // LOGGER.info("Stopping {} worker threads", this.threads.size()); + LOGGER.info("Stopping worker threads"); // Notify all worker threads to wake up, where they will then terminate synchronized (this.jobNotifier) { From c613aced7c19f788b021551548e56a1f16332648 Mon Sep 17 00:00:00 2001 From: DesktopFolder Date: Sun, 17 Nov 2024 19:37:01 -0500 Subject: [PATCH 3/3] Improvement: Set a hard limit on thread count based on view distance. --- .../client/render/chunk/ChunkRenderManager.java | 1 + .../render/chunk/compile/ChunkBuilder.java | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/ChunkRenderManager.java b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/ChunkRenderManager.java index 906fa896..e5bb260c 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/ChunkRenderManager.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/ChunkRenderManager.java @@ -103,6 +103,7 @@ public ChunkRenderManager(SodiumWorldRenderer renderer, ChunkRenderBackend ba this.world = world; this.builder = new ChunkBuilder<>(backend.getVertexType(), this.backend); + this.builder.SetupWithRender(renderDistance); // Split out for compat... this.builder.init(world, renderPassManager); this.dirty = true; diff --git a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java index 2e17942d..54b21725 100644 --- a/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java +++ b/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/compile/ChunkBuilder.java @@ -50,9 +50,9 @@ public class ChunkBuilder { private World world; private BlockRenderPassManager renderPassManager; - private final int limitThreads; - private final int initialThreads; - private boolean hasThreadSpace = true; + private int limitThreads; + private int initialThreads; + private boolean hasThreadSpace = false; private int updates = 0; private final ChunkVertexType vertexType; @@ -63,7 +63,15 @@ public ChunkBuilder(ChunkVertexType vertexType, ChunkRenderBackend backend) { this.backend = backend; this.limitThreads = getOptimalThreadCount(); // We assume nobody is running with 1 CPU, I guess. - this.initialThreads = Math.max(2, this.limitThreads / 6 /* Arbitrarily chosen :) */); + this.initialThreads = this.limitThreads; + } + + public void SetupWithRender(int viewDistance) { + // Choose threads based on viewDistance. + final int maxThreads = getOptimalThreadCount(); + this.limitThreads = maxThreads * Math.min(Math.max(viewDistance, 5), 32) / 32; + // We assume nobody is running with 1 CPU, I guess. + this.initialThreads = Math.max(2, this.limitThreads / 2 /* Arbitrarily chosen :) */); this.hasThreadSpace = this.limitThreads > this.initialThreads; }