improve concurrency handling in LOD render loading

This commit is contained in:
James Seibel
2026-02-09 07:44:03 -06:00
parent f0d71027f1
commit 4d3242a370
2 changed files with 71 additions and 31 deletions
@@ -21,6 +21,7 @@ package com.seibel.distanthorizons.core.dataObjects.render.bufferBuilding;
import com.seibel.distanthorizons.core.logging.DhLogger;
import com.seibel.distanthorizons.core.logging.DhLoggerBuilder;
import com.seibel.distanthorizons.core.pos.DhSectionPos;
import com.seibel.distanthorizons.core.pos.blockPos.DhBlockPos;
import com.seibel.distanthorizons.core.render.glObject.GLProxy;
import com.seibel.distanthorizons.core.render.glObject.buffer.GLVertexBuffer;
@@ -32,6 +33,7 @@ import org.lwjgl.system.MemoryUtil;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicReference;
/**
* Java representation of one or more OpenGL buffers for rendering.
@@ -59,7 +61,7 @@ public class LodBufferContainer implements AutoCloseable
public GLVertexBuffer[] vbos;
public GLVertexBuffer[] vbosTransparent;
private CompletableFuture<LodBufferContainer> uploadFuture = null;
private final AtomicReference<CompletableFuture<LodBufferContainer>> uploadFutureRef = new AtomicReference<>(null);
@@ -85,16 +87,31 @@ public class LodBufferContainer implements AutoCloseable
public synchronized CompletableFuture<LodBufferContainer> makeAndUploadBuffersAsync(LodQuadBuilder builder)
{
// separate variable to prevent race condition when checking null
CompletableFuture<LodBufferContainer> future = this.uploadFuture;
if (future != null)
CompletableFuture<LodBufferContainer> oldFuture = this.uploadFutureRef.get();
if (oldFuture != null)
{
// upload already in process
return future;
return oldFuture;
}
// new upload needed
future = new CompletableFuture<>();
this.uploadFuture = future;
CompletableFuture<LodBufferContainer> future = new CompletableFuture<>();
future.handle((lodBufferContainer, throwable) ->
{
if (!this.uploadFutureRef.compareAndSet(future, null))
{
LOGGER.warn("upload future ref changed for pos ["+DhSectionPos.toString(this.pos)+"].");
}
return null;
});
if (!this.uploadFutureRef.compareAndSet(null, future))
{
oldFuture = this.uploadFutureRef.get();
LodUtil.assertTrue(oldFuture != null, "Concurrency error");
return oldFuture;
}
@@ -113,7 +130,7 @@ public class LodBufferContainer implements AutoCloseable
{
// skip this event if requested
if (Thread.interrupted()
|| this.uploadFuture.isCancelled())
|| future.isCancelled())
{
throw new InterruptedException();
}
@@ -126,20 +143,17 @@ public class LodBufferContainer implements AutoCloseable
this.buffersUploaded = true;
// success
this.uploadFuture.complete(this);
this.uploadFuture = null;
future.complete(this);
}
catch (InterruptedException ignore)
{
this.uploadFuture.complete(this);
this.uploadFuture = null;
future.complete(this);
}
catch (Exception e)
{
LOGGER.error("Unexpected issue uploading buffer ["+this.minCornerBlockPos +"], error: ["+e.getMessage()+"].", e);
this.uploadFuture.completeExceptionally(e);
this.uploadFuture = null;
future.completeExceptionally(e);
}
finally
{
@@ -252,13 +266,13 @@ public class LodBufferContainer implements AutoCloseable
return count;
}
public boolean uploadInProgress() { return this.uploadFuture != null; }
public boolean uploadInProgress() { return this.uploadFutureRef.get() != null; }
public void debugDumpStats(StatsMap statsMap)
{
statsMap.incStat("RenderBuffers");
statsMap.incStat("SimpleRenderBuffers");
for (GLVertexBuffer vertexBuffer : vbos)
for (GLVertexBuffer vertexBuffer : this.vbos)
{
if (vertexBuffer != null)
{
@@ -40,6 +40,7 @@ import com.seibel.distanthorizons.core.render.renderer.DebugRenderer;
import com.seibel.distanthorizons.core.render.renderer.generic.BeaconRenderHandler;
import com.seibel.distanthorizons.core.sql.dto.BeaconBeamDTO;
import com.seibel.distanthorizons.core.sql.repo.BeaconBeamRepo;
import com.seibel.distanthorizons.core.util.LodUtil;
import com.seibel.distanthorizons.core.util.threading.PriorityTaskPicker;
import com.seibel.distanthorizons.core.util.threading.ThreadPoolUtil;
import com.seibel.distanthorizons.core.wrapperInterfaces.minecraft.IMinecraftClientWrapper;
@@ -51,6 +52,7 @@ import java.awt.*;
import java.util.*;
import java.util.List;
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicReference;
/**
* A render section represents an area that could be rendered.
@@ -94,20 +96,20 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
* Encapsulates everything between pulling data from the database (including neighbors)
* up to the point when geometry data is uploaded to the GPU.
*/
private CompletableFuture<Void> getAndBuildRenderDataFuture = null;
private final AtomicReference<CompletableFuture<Void>> getAndBuildRenderDataFutureRef = new AtomicReference<>(null);
/**
* used alongside {@link LodRenderSection#getAndBuildRenderDataFuture} so we can remove
* used alongside {@link LodRenderSection#getAndBuildRenderDataFutureRef} so we can remove
* unnecessary tasks from the executor.
*/
private Runnable getAndBuildRenderDataRunnable = null;
/**
* Represents just uploading the {@link LodQuadBuilder} to the GPU. <br>
* Separate from {@link LodRenderSection#getAndBuildRenderDataFuture} because they run on
* Separate from {@link LodRenderSection#getAndBuildRenderDataFutureRef} because they run on
* different threads (buffer uploading is on the MC render thread) and need to be canceled separately.
*/
private CompletableFuture<LodBufferContainer> bufferUploadFuture = null;
private final AtomicReference<CompletableFuture<LodBufferContainer>> bufferUploadFutureRef = new AtomicReference<>(null);
@@ -152,7 +154,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
return false;
}
if (this.getAndBuildRenderDataFuture != null)
if (this.getAndBuildRenderDataFutureRef.get() != null)
{
// don't accidentally queue multiple uploads at the same time
return false;
@@ -168,7 +170,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
future.handle((voidObj, throwable) ->
{
// the task is done, we don't need to track these anymore
this.getAndBuildRenderDataFuture = null;
this.getAndBuildRenderDataFutureRef.compareAndSet(future, null);
this.getAndBuildRenderDataRunnable = null;
return null;
@@ -176,7 +178,14 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
try
{
this.getAndBuildRenderDataFuture = future;
if (!this.getAndBuildRenderDataFutureRef.compareAndSet(null, future))
{
CompletableFuture<Void> oldFuture = this.getAndBuildRenderDataFutureRef.get();
LodUtil.assertTrue(oldFuture != null, "Concurrency error");
return true;
}
this.getAndBuildRenderDataRunnable = () ->
{
try
@@ -216,7 +225,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
}
}
@Nullable
private LodQuadBuilder getAndBuildRenderData()
private synchronized LodQuadBuilder getAndBuildRenderData()
{
try (ColumnRenderSource thisRenderSource = this.getRenderSourceForPos(this.pos, null))
{
@@ -301,16 +310,27 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
detailLevel += DhSectionPos.SECTION_MINIMUM_DETAIL_LEVEL;
return detailLevel == DhSectionPos.getDetailLevel(this.pos);
}
private CompletableFuture<LodBufferContainer> uploadToGpuAsync(LodQuadBuilder lodQuadBuilder)
private synchronized CompletableFuture<LodBufferContainer> uploadToGpuAsync(LodQuadBuilder lodQuadBuilder)
{
if (this.bufferUploadFuture != null)
CompletableFuture<LodBufferContainer> oldFuture = this.bufferUploadFutureRef.getAndSet(null);
if (oldFuture != null)
{
// shouldn't normally happen, but just in case canceling the previous future
// prevents the CPU from working on something that won't be used
this.bufferUploadFuture.cancel(true);
oldFuture.cancel(true);
}
CompletableFuture<LodBufferContainer> future = ColumnRenderBufferBuilder.uploadBuffersAsync(this.level, this.pos, lodQuadBuilder);
future.handle((a, throwable) ->
{
if (!this.bufferUploadFutureRef.compareAndSet(future, null))
{
LOGGER.error("Buffer upload future ref changed for pos: ["+DhSectionPos.toString(this.pos)+"].");
}
return null;
});
future.thenAccept((LodBufferContainer buffer) ->
{
// needed to clean up the old data
@@ -324,7 +344,13 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
previousContainer.close();
}
});
this.bufferUploadFuture = future;
if (!this.bufferUploadFutureRef.compareAndSet(null, future))
{
LodUtil.assertNotReach("Buffer upload future ref couldn't be set due to concurrency error, pos: ["+DhSectionPos.toString(this.pos)+"].");
}
return future;
}
@@ -342,7 +368,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
public boolean getRenderingEnabled() { return this.renderingEnabled; }
public void setRenderingEnabled(boolean enabled) { this.renderingEnabled = enabled;}
public boolean gpuUploadInProgress() { return this.getAndBuildRenderDataFuture != null; }
public boolean gpuUploadInProgress() { return this.getAndBuildRenderDataFutureRef.get() != null; }
//endregion
@@ -447,7 +473,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
//color = Color.green;
return;
}
else if (this.getAndBuildRenderDataFuture != null)
else if (this.getAndBuildRenderDataFutureRef.get() != null)
{
color = Color.yellow;
}
@@ -503,7 +529,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
}
// removes any in-progress futures since they aren't needed any more
CompletableFuture<Void> buildFuture = this.getAndBuildRenderDataFuture;
CompletableFuture<Void> buildFuture = this.getAndBuildRenderDataFutureRef.get();
if (buildFuture != null)
{
// remove the task from our executor if present
@@ -520,7 +546,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
}
}
CompletableFuture<LodBufferContainer> uploadFuture = this.bufferUploadFuture;
CompletableFuture<LodBufferContainer> uploadFuture = this.bufferUploadFutureRef.get();
if (uploadFuture != null)
{
uploadFuture.cancel(true);