Fix cached RenderSource closing while in use

This commit is contained in:
James Seibel
2025-01-20 21:50:33 -06:00
parent fab8191ddd
commit fce1fa3f41
3 changed files with 147 additions and 63 deletions
@@ -0,0 +1,83 @@
package com.seibel.distanthorizons.core.dataObjects.render;
import com.google.common.cache.Cache;
import org.jetbrains.annotations.NotNull;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.ReentrantLock;
/**
* Wrapper for {@link ColumnRenderSource} that handles reference counting
* and cache tracking.
*/
public class CachedColumnRenderSource implements AutoCloseable
{
public final ColumnRenderSource columnRenderSource;
private final AtomicInteger referenceCount;
private final Cache<Long, CachedColumnRenderSource> cachedRenderSourceByPos;
private final ReentrantLock getterLock;
//=============//
// constructor //
//=============//
public CachedColumnRenderSource(
@NotNull ColumnRenderSource columnRenderSource,
@NotNull ReentrantLock getterLock,
@NotNull Cache<Long, CachedColumnRenderSource> cachedRenderSourceByPos)
{
this.columnRenderSource = columnRenderSource;
this.getterLock = getterLock;
this.referenceCount = new AtomicInteger(1);
this.cachedRenderSourceByPos = cachedRenderSourceByPos;
}
//====================//
// reference counting //
//====================//
public void markInUse() { this.referenceCount.getAndIncrement(); }
//================//
// base overrides //
//================//
/**
* Will be called multiple times,
* however it will only close the underlying data once
* all references have closed.
*/
@Override
public void close() throws IllegalStateException
{
try
{
// lock to prevent other threads for accessing the cache if we invalidate it
this.getterLock.lock();
// only close once everyone is done with this datasource
int refCount = this.referenceCount.decrementAndGet();
if (refCount == 0)
{
this.cachedRenderSourceByPos.invalidate(this.columnRenderSource.pos);
this.columnRenderSource.close();
}
else if (refCount < 0)
{
throw new IllegalStateException("Render source ["+this.columnRenderSource.pos+"] reference count incorrect. Object already closed.");
}
}
finally
{
this.getterLock.unlock();
}
}
}
@@ -24,6 +24,7 @@ import com.google.common.cache.CacheBuilder;
import com.google.common.cache.RemovalCause;
import com.google.common.cache.RemovalNotification;
import com.seibel.distanthorizons.core.config.Config;
import com.seibel.distanthorizons.core.dataObjects.render.CachedColumnRenderSource;
import com.seibel.distanthorizons.core.dataObjects.render.ColumnRenderSource;
import com.seibel.distanthorizons.core.dataObjects.render.bufferBuilding.ColumnRenderBuffer;
import com.seibel.distanthorizons.core.enums.EDhDirection;
@@ -98,48 +99,16 @@ public class LodQuadTree extends QuadTree<LodRenderSection> implements IDebugRen
* caching the loaded positions significantly improves initial loading performance
* since the same position doesn't need to be loaded 5 times.
*/
private final Cache<Long, ColumnRenderSource> cachedRenderSourceByPos
private final Cache<Long, CachedColumnRenderSource> cachedRenderSourceByPos
= CacheBuilder.newBuilder()
// availableProcessors() : each process may need to be loading a render source
// +1 : add 1 thread count buffer to reduce the chance of accidentally unloading a render source before it's used
// *5 : each render source needs it's 4 adjacent sides, so a total of 5 render sources are needed per load
.maximumSize((Runtime.getRuntime().availableProcessors() + 1) * 5L)
.removalListener((RemovalNotification<Long, ColumnRenderSource> removalNotification) ->
{
RemovalCause cause = removalNotification.getCause();
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();
executor.execute(() ->
{
// close the render source after it's been
ColumnRenderSource renderSource = removalNotification.getValue();
if (renderSource != null)
{
ReentrantLock lock = renderLoadLockContainer.getLockForPos(renderSource.getPos());
try
{
lock.lock();
renderSource.close();
}
finally
{
lock.unlock();
}
}
else
{
// shouldn't happen, but just in case
LOGGER.error("Unable to close null cached render source.");
}
});
}
})
.<Long, ColumnRenderSource>build();
// No closing logic since the CachedColumnRenderSource is in charge
// of freeing the underlying ColumnRenderSource.
// That way we don't have to worry about accidentally closing an in-use object.
.<Long, CachedColumnRenderSource>build();
@Nullable
public final BeaconRenderHandler beaconRenderHandler;
@@ -655,7 +624,22 @@ public class LodQuadTree extends QuadTree<LodRenderSection> implements IDebugRen
*/
public void reloadPos(long pos)
{
this.cachedRenderSourceByPos.invalidate(pos); // TODO will this cause issues? we may need to lock this invalidation if the cached data source is currently in use
// clear cache //
this.clearRenderCacheForPos(pos);
for (EDhDirection direction : EDhDirection.ADJ_DIRECTIONS)
{
long adjacentPos = DhSectionPos.getAdjacentPos(pos, direction);
this.clearRenderCacheForPos(adjacentPos);
}
// queue reloads //
// only queue each section for reloading
// after the cache has been cleared,
// this is done to prevent accidentally using old cached data
this.sectionsToReload.add(pos);
// the adjacent locations also need to be updated to make sure lighting
@@ -664,10 +648,24 @@ public class LodQuadTree extends QuadTree<LodRenderSection> implements IDebugRen
for (EDhDirection direction : EDhDirection.ADJ_DIRECTIONS)
{
long adjacentPos = DhSectionPos.getAdjacentPos(pos, direction);
this.cachedRenderSourceByPos.invalidate(adjacentPos); // TODO will this cause issues? we may need to lock this invalidation if the cached data source is currently in use
this.sectionsToReload.add(adjacentPos);
}
}
private void clearRenderCacheForPos(long pos)
{
// locking is needed to prevent another thread
// from accessing the cache while it's being cleared
ReentrantLock lock = this.renderLoadLockContainer.getLockForPos(pos);
try
{
lock.lock();
this.cachedRenderSourceByPos.invalidate(pos);
}
finally
{
lock.unlock();
}
}
@@ -23,6 +23,7 @@ import com.google.common.base.Suppliers;
import com.google.common.cache.Cache;
import com.seibel.distanthorizons.core.config.Config;
import com.seibel.distanthorizons.core.dataObjects.fullData.sources.FullDataSourceV2;
import com.seibel.distanthorizons.core.dataObjects.render.CachedColumnRenderSource;
import com.seibel.distanthorizons.core.dataObjects.render.ColumnRenderSource;
import com.seibel.distanthorizons.core.dataObjects.render.bufferBuilding.ColumnRenderBufferBuilder;
import com.seibel.distanthorizons.core.dataObjects.render.bufferBuilding.LodQuadBuilder;
@@ -85,7 +86,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
private final FullDataSourceProviderV2 fullDataSourceProvider;
private final LodQuadTree quadTree;
private final KeyedLockContainer<Long> renderLoadLockContainer;
private final Cache<Long, ColumnRenderSource> cachedRenderSourceByPos;
private final Cache<Long, CachedColumnRenderSource> cachedRenderSourceByPos;
/**
* contains the list of beacons currently being rendered in this section
@@ -143,7 +144,7 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
long pos,
LodQuadTree quadTree,
IDhClientLevel level, FullDataSourceProviderV2 fullDataSourceProvider,
Cache<Long, ColumnRenderSource> cachedRenderSourceByPos, KeyedLockContainer<Long> renderLoadLockContainer)
Cache<Long, CachedColumnRenderSource> cachedRenderSourceByPos, KeyedLockContainer<Long> renderLoadLockContainer)
{
this.pos = pos;
this.quadTree = quadTree;
@@ -234,10 +235,9 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
}
private void getAndUploadRenderDataToGpu()
{
try
try(CachedColumnRenderSource cachedRenderSource = this.getRenderSourceForPos(this.pos))
{
ColumnRenderSource renderSource = this.getRenderSourceForPos(this.pos);
if (renderSource == null)
if (cachedRenderSource == null)
{
// nothing needs to be rendered
// TODO how doesn't this cause infinite file handler loops?
@@ -245,23 +245,23 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
// setting the render buffer here
return;
}
ColumnRenderSource thisRenderSource = cachedRenderSource.columnRenderSource;
boolean enableTransparency = Config.Client.Advanced.Graphics.Quality.transparency.get().transparencyEnabled;
LodQuadBuilder lodQuadBuilder = new LodQuadBuilder(enableTransparency, this.level.getClientLevelWrapper());
// load adjacent render sources
try(CachedColumnRenderSource northRenderSource = this.getRenderSourceForPos(DhSectionPos.getAdjacentPos(this.pos, EDhDirection.NORTH));
CachedColumnRenderSource southRenderSource = this.getRenderSourceForPos(DhSectionPos.getAdjacentPos(this.pos, EDhDirection.SOUTH));
CachedColumnRenderSource eastRenderSource = this.getRenderSourceForPos(DhSectionPos.getAdjacentPos(this.pos, EDhDirection.EAST));
CachedColumnRenderSource westRenderSource = this.getRenderSourceForPos(DhSectionPos.getAdjacentPos(this.pos, EDhDirection.WEST)))
{
ColumnRenderSource northRenderSource = this.getRenderSourceForPos(DhSectionPos.getAdjacentPos(this.pos, EDhDirection.NORTH));
ColumnRenderSource southRenderSource = this.getRenderSourceForPos(DhSectionPos.getAdjacentPos(this.pos, EDhDirection.SOUTH));
ColumnRenderSource eastRenderSource = this.getRenderSourceForPos(DhSectionPos.getAdjacentPos(this.pos, EDhDirection.EAST));
ColumnRenderSource westRenderSource = this.getRenderSourceForPos(DhSectionPos.getAdjacentPos(this.pos, EDhDirection.WEST));
ColumnRenderSource[] adjacentRenderSections = new ColumnRenderSource[EDhDirection.ADJ_DIRECTIONS.length];
adjacentRenderSections[EDhDirection.NORTH.ordinal() - 2] = northRenderSource;
adjacentRenderSections[EDhDirection.SOUTH.ordinal() - 2] = southRenderSource;
adjacentRenderSections[EDhDirection.EAST.ordinal() - 2] = eastRenderSource;
adjacentRenderSections[EDhDirection.WEST.ordinal() - 2] = westRenderSource;
adjacentRenderSections[EDhDirection.NORTH.ordinal() - 2] = (northRenderSource != null) ? northRenderSource.columnRenderSource : null;
adjacentRenderSections[EDhDirection.SOUTH.ordinal() - 2] = (southRenderSource != null) ? southRenderSource.columnRenderSource : null;
adjacentRenderSections[EDhDirection.EAST.ordinal() - 2] = (eastRenderSource != null) ? eastRenderSource.columnRenderSource : null;
adjacentRenderSections[EDhDirection.WEST.ordinal() - 2] = (westRenderSource != null) ? westRenderSource.columnRenderSource : null;
boolean[] adjIsSameDetailLevel = new boolean[EDhDirection.ADJ_DIRECTIONS.length];
adjIsSameDetailLevel[EDhDirection.NORTH.ordinal() - 2] = this.isAdjacentPosSameDetailLevel(EDhDirection.NORTH);
@@ -269,9 +269,9 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
adjIsSameDetailLevel[EDhDirection.EAST.ordinal() - 2] = this.isAdjacentPosSameDetailLevel(EDhDirection.EAST);
adjIsSameDetailLevel[EDhDirection.WEST.ordinal() - 2] = this.isAdjacentPosSameDetailLevel(EDhDirection.WEST);
// the render sources are only needed in this synchronous method,
// the render sources are only needed by this synchronous method,
// then they can be closed
ColumnRenderBufferBuilder.makeLodRenderData(lodQuadBuilder, renderSource, this.level, adjacentRenderSections, adjIsSameDetailLevel);
ColumnRenderBufferBuilder.makeLodRenderData(lodQuadBuilder, thisRenderSource, this.level, adjacentRenderSections, adjIsSameDetailLevel);
}
this.uploadToGpuAsync(lodQuadBuilder);
@@ -282,33 +282,36 @@ public class LodRenderSection implements IDebugRenderable, AutoCloseable
}
}
@Nullable
private ColumnRenderSource getRenderSourceForPos(long pos)
private CachedColumnRenderSource getRenderSourceForPos(long pos)
{
ReentrantLock lock = this.renderLoadLockContainer.getLockForPos(pos);
try
{
// we don't want multiple threads attempting to load the same position at the same time
// we don't want multiple threads attempting to load the same position at the same time,
// and we don't want to access the cache while invalidating it on a different thread
lock.lock();
// use the cached data if possible
ColumnRenderSource renderSource = this.cachedRenderSourceByPos.getIfPresent(pos);
if (renderSource != null)
CachedColumnRenderSource cachedRenderSource = this.cachedRenderSourceByPos.getIfPresent(pos);
if (cachedRenderSource != null)
{
return renderSource;
cachedRenderSource.markInUse();
return cachedRenderSource;
}
// generate new render source
try (FullDataSourceV2 fullDataSource = this.fullDataSourceProvider.get(pos))
{
renderSource = FullDataToRenderDataTransformer.transformFullDataToRenderSource(fullDataSource, this.level);
ColumnRenderSource renderSource = FullDataToRenderDataTransformer.transformFullDataToRenderSource(fullDataSource, this.level);
// only add valid data to the cache (to prevent null pointers)
if (renderSource != null)
{
this.cachedRenderSourceByPos.put(pos, renderSource);
cachedRenderSource = new CachedColumnRenderSource(renderSource, lock, this.cachedRenderSourceByPos);
this.cachedRenderSourceByPos.put(pos, cachedRenderSource);
}
}
return renderSource;
return cachedRenderSource;
}
finally
{