Reduce the chance of concurrent Long2ObjectLinkedOpenHashMap issues

This commit is contained in:
James Seibel
2023-10-31 19:56:04 -05:00
parent 49f90aa5c9
commit 4ac4daeac1
4 changed files with 64 additions and 16 deletions
@@ -21,6 +21,7 @@ package com.seibel.distanthorizons.common.wrappers.worldGeneration.mimicObject;
import java.lang.invoke.MethodHandles;
import java.util.List;
import java.util.concurrent.locks.ReentrantLock;
import com.seibel.distanthorizons.common.wrappers.worldGeneration.BatchGenerationEnvironment;
import com.seibel.distanthorizons.core.logging.DhLoggerBuilder;
@@ -66,6 +67,12 @@ public class DhLitWorldGenRegion extends WorldGenRegion
private final List<ChunkAccess> cache;
Long2ObjectOpenHashMap<ChunkAccess> chunkMap = new Long2ObjectOpenHashMap<ChunkAccess>();
/**
* Present to reduce the chance that we accidentally break underlying MC code that isn't thread safe,
* specifically: "it.unimi.dsi.fastutil.longs.Long2ObjectLinkedOpenHashMap.getAndMoveToFirst()"
*/
ReentrantLock getChunkLock = new ReentrantLock();
#if PRE_MC_1_18_2
private ChunkPos overrideCenterPos = null;
@@ -90,6 +97,8 @@ public class DhLitWorldGenRegion extends WorldGenRegion
#endif
#endif
public DhLitWorldGenRegion(
ServerLevel serverLevel, DummyLightEngine lightEngine,
List<ChunkAccess> chunkList, ChunkStatus chunkStatus, int writeRadius,
@@ -104,6 +113,8 @@ public class DhLitWorldGenRegion extends WorldGenRegion
this.size = Mth.floor(Math.sqrt(chunkList.size()));
}
#if POST_MC_1_17_1
// Bypass BCLib mixin overrides.
@Override
@@ -205,7 +216,16 @@ public class DhLitWorldGenRegion extends WorldGenRegion
@Override
public ChunkAccess getChunk(int i, int j)
{
return this.getChunk(i, j, ChunkStatus.EMPTY);
try
{
// lock is to prevent issues with underlying MC code that doesn't support multithreading
this.getChunkLock.lock();
return this.getChunk(i, j, ChunkStatus.EMPTY);
}
finally
{
this.getChunkLock.unlock();
}
}
// Override to ensure no other mod mixins cause skipping the overrided
@@ -213,7 +233,16 @@ public class DhLitWorldGenRegion extends WorldGenRegion
@Override
public ChunkAccess getChunk(int i, int j, ChunkStatus chunkStatus)
{
return this.getChunk(i, j, chunkStatus, true);
try
{
// lock is to prevent issues with underlying MC code that doesn't support multithreading
this.getChunkLock.lock();
return this.getChunk(i, j, chunkStatus, true);
}
finally
{
this.getChunkLock.unlock();
}
}
// Use this instead of super.getChunk() to bypass C2ME concurrency checks
@@ -13,21 +13,20 @@ import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.locks.ReentrantLock;
public class RegionFileStorageExternalCache implements AutoCloseable
{
public final RegionFileStorage storage;
public static final int MAX_CACHE_SIZE = 16;
@Override
public void close() throws IOException
{
RegionFileCache cache;
while ((cache = this.regionFileCache.poll()) != null)
{
cache.file.close();
}
}
/**
* Present to reduce the chance that we accidentally break underlying MC code that isn't thread safe,
* specifically: "it.unimi.dsi.fastutil.longs.Long2ObjectLinkedOpenHashMap.getAndMoveToFirst()"
*/
ReentrantLock getRegionFileLock = new ReentrantLock();
static class RegionFileCache
{
@@ -42,6 +41,8 @@ public class RegionFileStorageExternalCache implements AutoCloseable
}
public ConcurrentLinkedQueue<RegionFileCache> regionFileCache = new ConcurrentLinkedQueue<>();
public RegionFileStorageExternalCache(RegionFileStorage storage) { this.storage = storage; }
@@ -61,6 +62,8 @@ public class RegionFileStorageExternalCache implements AutoCloseable
try
{
this.getRegionFileLock.lock();
#if MC_1_16_5 || MC_1_17_1
rFile = this.storage.getRegionFile(pos);
#else
@@ -85,6 +88,10 @@ public class RegionFileStorageExternalCache implements AutoCloseable
}
#endif
}
finally
{
this.getRegionFileLock.unlock();
}
}
if (retryCount >= maxRetryCount)
@@ -140,7 +147,7 @@ public class RegionFileStorageExternalCache implements AutoCloseable
@Nullable
public CompoundTag read(ChunkPos pos) throws IOException
{
RegionFile file = getRegionFile(pos);
RegionFile file = this.getRegionFile(pos);
if (file == null)
{
return null;
@@ -162,4 +169,16 @@ public class RegionFileStorageExternalCache implements AutoCloseable
}
}
@Override
public void close() throws IOException
{
RegionFileCache cache;
while ((cache = this.regionFileCache.poll()) != null)
{
cache.file.close();
}
}
}
@@ -63,7 +63,7 @@ public final class StepStructureReference
for (ChunkAccess chunk : chunksToDo)
{
// System.out.println("StepStructureReference: "+chunk.getPos());
environment.params.generator.createReferences(worldGenRegion, tParams.structFeat.forWorldGenRegion(worldGenRegion), chunk);
this.environment.params.generator.createReferences(worldGenRegion, tParams.structFeat.forWorldGenRegion(worldGenRegion), chunk);
}
}
@@ -38,7 +38,7 @@ public final class StepStructureStart
{
private static final Logger LOGGER = DhLoggerBuilder.getLogger();
private static final ChunkStatus STATUS = ChunkStatus.STRUCTURE_STARTS;
private static final ReentrantLock structurePlacementLock = new ReentrantLock();
private static final ReentrantLock STRUCTURE_PLACEMENT_LOCK = new ReentrantLock();
private final BatchGenerationEnvironment environment;
@@ -96,7 +96,7 @@ public final class StepStructureStart
// hopefully this shouldn't cause any performance issues (this step is generally quite quick so hopefully it should be fine)
// and should prevent some concurrency issues
structurePlacementLock.lock();
STRUCTURE_PLACEMENT_LOCK.lock();
#if PRE_MC_1_19_2
environment.params.generator.createStructures(environment.params.registry, tParams.structFeat, chunk, environment.params.structures,
@@ -140,7 +140,7 @@ public final class StepStructureStart
#endif
structurePlacementLock.unlock();
STRUCTURE_PLACEMENT_LOCK.unlock();
}
}
}