Fix LodQuadTree render source leaks

This commit is contained in:
James Seibel
2025-01-11 13:36:37 -06:00
parent 5c96937ab4
commit c80abeb567
3 changed files with 84 additions and 26 deletions
@@ -25,6 +25,7 @@ import com.seibel.distanthorizons.core.util.RenderDataPointUtil;
import it.unimi.dsi.fastutil.longs.LongArrayList;
import java.util.Arrays;
import java.util.ConcurrentModificationException;
public final class ColumnArrayView implements IColumnDataView
{
@@ -53,12 +54,18 @@ public final class ColumnArrayView implements IColumnDataView
// constructor //
//=============//
public ColumnArrayView(LongArrayList data, int size, int offset, int verticalSize)
/** @throws IllegalArgumentException if the offset is greater than the data's size */
public ColumnArrayView(LongArrayList data, int size, int offset, int verticalSize) throws IllegalArgumentException
{
this.data = data;
this.size = size;
this.offset = offset;
this.verticalSize = verticalSize;
if (this.data.size() < this.offset)
{
throw new IllegalArgumentException("data size ["+this.data.size()+"] is shorter than offset ["+this.offset+"].");
}
}
@@ -68,7 +75,20 @@ public final class ColumnArrayView implements IColumnDataView
//=====================//
@Override
public long get(int index) { return data.getLong(index + offset); }
public long get(int index)
{
try
{
return this.data.getLong(index + this.offset);
}
catch (IndexOutOfBoundsException e)
{
// we can fairly confidently say this is a concurrent exception over an actual
// index out of bounds, since we're generally iterating over the whole
// array any time we use this getter.
throw new ConcurrentModificationException("Potential concurrent modification detected. Make sure the parent ColumnRenderSource isn't being closed before the ColumnArrayView processing is complete.", e);
}
}
public void set(int index, long value) { data.set(index + offset, value); }
@Override
@@ -47,6 +47,7 @@ import org.apache.logging.log4j.Logger;
import javax.annotation.WillNotClose;
import java.awt.*;
import java.util.*;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -103,10 +104,10 @@ public class LodQuadTree extends QuadTree<LodRenderSection> implements IDebugRen
.removalListener((RemovalNotification<Long, ColumnRenderSource> removalNotification) ->
{
RemovalCause cause = removalNotification.getCause();
if (cause == RemovalCause.EXPIRED
|| cause == RemovalCause.COLLECTED
|| cause == RemovalCause.SIZE
|| cause == RemovalCause.EXPLICIT)
if (cause == RemovalCause.EXPLICIT
|| cause == RemovalCause.EXPIRED
|| cause == RemovalCause.COLLECTED
|| cause == RemovalCause.SIZE)
{
// cleanup needs to be handled on a different thread to prevent locking up the main loading threads
ThreadPoolExecutor executor = ThreadPoolUtil.getCleanupExecutor();
@@ -788,29 +789,69 @@ public class LodQuadTree extends QuadTree<LodRenderSection> implements IDebugRen
@Override
public void close()
{
LOGGER.info("Shutting down " + LodQuadTree.class.getSimpleName() + "...");
LOGGER.info("Shutting down LodQuadTree...");
DebugRenderer.unregister(this, Config.Client.Advanced.Debugging.DebugWireframe.showQuadTreeRenderStatus);
ThreadPoolExecutor executor = ThreadPoolUtil.getCleanupExecutor();
// closing each node make take a few moments
ThreadPoolExecutor mainCleanupExecutor = ThreadPoolUtil.getCleanupExecutor();
// closing every node may take a few moments
// so this is run on a separate thread to prevent lagging the render thread
executor.execute(() ->
mainCleanupExecutor.execute(() ->
{
Iterator<QuadNode<LodRenderSection>> nodeIterator = this.nodeIterator();
while (nodeIterator.hasNext())
this.treeReadWriteLock.lock();
try
{
QuadNode<LodRenderSection> quadNode = nodeIterator.next();
if (quadNode.value != null)
// walk through each node
Iterator<QuadNode<LodRenderSection>> nodeIterator = this.nodeIterator();
ArrayList<CompletableFuture<Void>> renderDataBuildFutures = new ArrayList<>();
while (nodeIterator.hasNext())
{
quadNode.value.close();
quadNode.value = null;
QuadNode<LodRenderSection> quadNode = nodeIterator.next();
LodRenderSection renderSection = quadNode.value;
if (renderSection != null)
{
// we need to wait for the render data to finish building before we can close the cache
CompletableFuture<Void> future = renderSection.getRenderDataBuildFuture();
if (future != null)
{
renderDataBuildFutures.add(future);
}
renderSection.close();
quadNode.value = null;
}
}
// close the render cache after it is done being used
LOGGER.info("waiting for ["+renderDataBuildFutures.size()+"] futures before closing render cache...");
CompletableFuture.allOf(renderDataBuildFutures.toArray(new CompletableFuture[0]))
.handle((voidObj, throwable) ->
{
// run on a separate thread so we don't lock up the main cleanup thread
// with the sleep() call
new Thread(() ->
{
// Sleep shouldn't be necessary, but James found a few cases where
// the futures incorrectly claimed they were done.
// Sleeping solved those issues.
try { Thread.sleep(5_000); } catch (InterruptedException ignore) { }
LOGGER.debug("closing render cache");
this.cachedRenderSourceByPos.invalidateAll();
}).start();
return null;
});
}
finally
{
this.treeReadWriteLock.unlock();
}
});
LOGGER.info("Finished shutting down " + LodQuadTree.class.getSimpleName());
LOGGER.info("Finished shutting down LodQuadTree");
}
@@ -70,8 +70,8 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
@WillNotClose
private final FullDataSourceProviderV2 fullDataSourceProvider;
private final LodQuadTree quadTree;
private final Cache<Long, ColumnRenderSource> cachedRenderSourceByPos;
private final KeyedLockContainer<Long> renderLoadLockContainer;
private final Cache<Long, ColumnRenderSource> cachedRenderSourceByPos;
@@ -86,6 +86,9 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
* up to the point when geometry data is uploaded to the GPU.
*/
private CompletableFuture<Void> getAndBuildRenderDataFuture = null;
@Nullable
public CompletableFuture<Void> getRenderDataBuildFuture() { return this.getAndBuildRenderDataFuture; }
/**
* used alongside {@link LodRenderSection#getAndBuildRenderDataFuture} so we can remove
* unnecessary tasks from the executor.
@@ -254,6 +257,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
return renderSource;
}
// generate new render source
try (FullDataSourceV2 fullDataSource = this.fullDataSourceProvider.get(pos))
{
renderSource = FullDataToRenderDataTransformer.transformFullDataToRenderSource(fullDataSource, this.level);
@@ -498,14 +502,8 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
CompletableFuture<Void> buildFuture = this.getAndBuildRenderDataFuture;
if (buildFuture != null)
{
// Note to self:
// canceling a task prevents it from running, but doesn't allow
// us to purge it from the executor it was queued in.
// As far as James can tell this appears to be a Java bug.
buildFuture.cancel(true);
// remove the task from our executor if present
// note: don't cancel the task since that prevents cleanup, we just don't want it to run
PriorityTaskPicker.Executor executor = ThreadPoolUtil.getFileHandlerExecutor();
if (executor != null && !executor.isTerminated())
{
@@ -521,7 +519,6 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
if (uploadFuture != null)
{
uploadFuture.cancel(true);
this.bufferUploadFuture = null;
}