From d0627bf03f8c25fca33f871c7e84f7398df4af3b Mon Sep 17 00:00:00 2001 From: rfresh2 <89827146+rfresh2@users.noreply.github.com> Date: Wed, 18 Dec 2024 10:04:08 -0800 Subject: [PATCH 1/2] use rw lock for cache mutations --- .../ChunkHighlightBaseCacheHandler.java | 80 ++++++++++++++++--- .../ChunkHighlightCacheDimensionHandler.java | 76 +++++++++++------- .../highlights/ChunkHighlightLocalCache.java | 11 ++- 3 files changed, 126 insertions(+), 41 deletions(-) diff --git a/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightBaseCacheHandler.java b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightBaseCacheHandler.java index fc0cbbf3..79b05555 100644 --- a/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightBaseCacheHandler.java +++ b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightBaseCacheHandler.java @@ -1,13 +1,23 @@ package xaeroplus.feature.render.highlights; -import it.unimi.dsi.fastutil.longs.*; +import it.unimi.dsi.fastutil.longs.Long2LongMap; +import it.unimi.dsi.fastutil.longs.Long2LongOpenHashMap; +import it.unimi.dsi.fastutil.longs.LongArrayList; +import it.unimi.dsi.fastutil.longs.LongList; import net.minecraft.resources.ResourceKey; import net.minecraft.world.level.Level; +import xaeroplus.XaeroPlus; +import xaeroplus.util.ChunkUtils; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.StampedLock; import static xaeroplus.util.ChunkUtils.chunkPosToLong; public abstract class ChunkHighlightBaseCacheHandler implements ChunkHighlightCache { - public final Long2LongMap chunks = Long2LongMaps.synchronize(new Long2LongOpenHashMap()); + public final ReadWriteLock lock = new StampedLock().asReadWriteLock(); + public final Long2LongMap chunks = new Long2LongOpenHashMap(); @Override public boolean addHighlight(final int x, final int z) { @@ -16,14 +26,28 @@ public boolean addHighlight(final int x, final int z) { public boolean addHighlight(final int x, final int z, final long foundTime) { final long chunkPos = chunkPosToLong(x, z); - chunks.put(chunkPos, foundTime); + try { + if (lock.writeLock().tryLock(1, TimeUnit.SECONDS)) { + chunks.put(chunkPos, foundTime); + lock.writeLock().unlock(); + } + } catch (final Exception e) { + XaeroPlus.LOGGER.error("Failed to add new highlight: {}, {}", x, z, e); + } return true; } @Override public boolean removeHighlight(final int x, final int z) { final long chunkPos = chunkPosToLong(x, z); - chunks.remove(chunkPos); + try { + if (lock.writeLock().tryLock(1, TimeUnit.SECONDS)) { + chunks.remove(chunkPos); + lock.writeLock().unlock(); + } + } catch (final Exception e) { + XaeroPlus.LOGGER.error("Failed to add new highlight: {}, {}", x, z, e); + } return true; } @@ -34,11 +58,26 @@ public boolean isHighlighted(final int x, final int z, ResourceKey dimens @Override public LongList getHighlightsSnapshot(final ResourceKey dimension) { - return new LongArrayList(chunks.keySet()); + try { + if (lock.readLock().tryLock(1, TimeUnit.SECONDS)) { + // copy is memory inefficient but we need a thread safe iterator for rendering + var list = new LongArrayList(chunks.keySet()); + lock.readLock().unlock(); + return list; + } + } catch (final Exception e) { + XaeroPlus.LOGGER.error("Error getting highlights snapshot in dimension: {}", dimension.location().getPath(), e); + } + return LongList.of(); } public boolean isHighlighted(final long chunkPos) { - return chunks.containsKey(chunkPos); + try { + return chunks.containsKey(chunkPos); + } catch (final Exception e) { + XaeroPlus.LOGGER.error("Error checking if chunk is highlighted: {}, {}", ChunkUtils.longToChunkX(chunkPos), ChunkUtils.longToChunkZ(chunkPos), e); + } + return false; } @Override @@ -49,15 +88,36 @@ public Long2LongMap getHighlightsState() { @Override public void loadPreviousState(final Long2LongMap state) { if (state == null) return; - chunks.putAll(state); + try { + if (lock.writeLock().tryLock(1, TimeUnit.SECONDS)) { + chunks.putAll(state); + lock.writeLock().unlock(); + } + } catch (final Exception e) { + XaeroPlus.LOGGER.error("Error loading previous highlight cache state", e); + } } public void replaceState(final Long2LongOpenHashMap state) { - chunks.clear(); - chunks.putAll(state); + try { + if (lock.writeLock().tryLock(1, TimeUnit.SECONDS)) { + this.chunks.clear(); + this.chunks.putAll(state); + lock.writeLock().unlock(); + } + } catch (final Exception e) { + XaeroPlus.LOGGER.error("Failed replacing highlight cache state", e); + } } public void reset() { - chunks.clear(); + try { + if (lock.writeLock().tryLock(1, TimeUnit.SECONDS)) { + chunks.clear(); + lock.writeLock().unlock(); + } + } catch (final Exception e) { + XaeroPlus.LOGGER.error("Failed resetting highlight cache", e); + } } } diff --git a/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightCacheDimensionHandler.java b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightCacheDimensionHandler.java index 58be4d05..5ddeba26 100644 --- a/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightCacheDimensionHandler.java +++ b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightCacheDimensionHandler.java @@ -5,10 +5,12 @@ import net.minecraft.resources.ResourceKey; import net.minecraft.world.level.Level; import org.jetbrains.annotations.NotNull; +import xaeroplus.XaeroPlus; import xaeroplus.util.ChunkUtils; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.TimeUnit; import static xaeroplus.util.ChunkUtils.chunkPosToLong; import static xaeroplus.util.ChunkUtils.regionCoordToChunkCoord; @@ -44,13 +46,21 @@ public void setWindow(int regionX, int regionZ, int regionSize) { private void loadHighlightsInWindow() { executorService.execute(() -> { - synchronized (this.chunks) { - database.getHighlightsInWindow( + final List chunks = database.getHighlightsInWindow( dimension, windowRegionX - windowRegionSize, windowRegionX + windowRegionSize, - windowRegionZ - windowRegionSize, windowRegionZ + windowRegionSize, - (x, y, time) -> this.chunks.put(chunkPosToLong(x, y), time) - ); + windowRegionZ - windowRegionSize, windowRegionZ + windowRegionSize + ); + try { + if (lock.writeLock().tryLock(1, TimeUnit.SECONDS)) { + for (int i = 0; i < chunks.size(); i++) { + final ChunkHighlightData chunk = chunks.get(i); + this.chunks.put(chunkPosToLong(chunk.x(), chunk.z()), chunk.foundTime()); + } + lock.writeLock().unlock(); + } + } catch (final Exception e) { + XaeroPlus.LOGGER.error("Failed to load highlights in window for {} disk cache dimension: {}", database.databaseName, dimension.location(), e); } }); } @@ -58,24 +68,29 @@ private void loadHighlightsInWindow() { private void writeHighlightsOutsideWindowToDatabase() { executorService.execute(() -> { final List chunksToWrite = new ArrayList<>(); - var minChunkX = regionCoordToChunkCoord(windowRegionX - windowRegionSize); - var maxChunkX = regionCoordToChunkCoord(windowRegionX + windowRegionSize); - var minChunkZ = regionCoordToChunkCoord(windowRegionZ - windowRegionSize); - var maxChunkZ = regionCoordToChunkCoord(windowRegionZ + windowRegionSize); - synchronized (this.chunks) { - for (var it = chunks.long2LongEntrySet().iterator(); it.hasNext(); ) { - var entry = it.next(); - final long chunkPos = entry.getLongKey(); - final int chunkX = ChunkUtils.longToChunkX(chunkPos); - final int chunkZ = ChunkUtils.longToChunkZ(chunkPos); - if (chunkX < minChunkX - || chunkX > maxChunkX - || chunkZ < minChunkZ - || chunkZ > maxChunkZ) { - chunksToWrite.add(new ChunkHighlightData(chunkX, chunkZ, entry.getLongValue())); - it.remove(); + try { + if (lock.writeLock().tryLock(1L, TimeUnit.SECONDS)) { + var minChunkX = regionCoordToChunkCoord(windowRegionX - windowRegionSize); + var maxChunkX = regionCoordToChunkCoord(windowRegionX + windowRegionSize); + var minChunkZ = regionCoordToChunkCoord(windowRegionZ - windowRegionSize); + var maxChunkZ = regionCoordToChunkCoord(windowRegionZ + windowRegionSize); + for (var it = chunks.long2LongEntrySet().iterator(); it.hasNext(); ) { + var entry = it.next(); + final long chunkPos = entry.getLongKey(); + final int chunkX = ChunkUtils.longToChunkX(chunkPos); + final int chunkZ = ChunkUtils.longToChunkZ(chunkPos); + if (chunkX < minChunkX + || chunkX > maxChunkX + || chunkZ < minChunkZ + || chunkZ > maxChunkZ) { + chunksToWrite.add(new ChunkHighlightData(chunkX, chunkZ, entry.getLongValue())); + it.remove(); + } } + lock.writeLock().unlock(); } + } catch (final Exception e) { + XaeroPlus.LOGGER.error("Error while writing highlights outside window to {} disk cache dimension: {}", database.databaseName, dimension.location(), e); } database.insertHighlightList(chunksToWrite, dimension); }); @@ -84,14 +99,19 @@ private void writeHighlightsOutsideWindowToDatabase() { public ListenableFuture writeAllHighlightsToDatabase() { return executorService.submit(() -> { final List chunksToWrite = new ArrayList<>(chunks.size()); - synchronized (chunks) { - for (var it = chunks.long2LongEntrySet().iterator(); it.hasNext(); ) { - var entry = it.next(); - final long chunkPos = entry.getLongKey(); - final int chunkX = ChunkUtils.longToChunkX(chunkPos); - final int chunkZ = ChunkUtils.longToChunkZ(chunkPos); - chunksToWrite.add(new ChunkHighlightData(chunkX, chunkZ, entry.getLongValue())); + try { + if (lock.readLock().tryLock(1, TimeUnit.SECONDS)) { + for (var it = chunks.long2LongEntrySet().iterator(); it.hasNext(); ) { + var entry = it.next(); + final long chunkPos = entry.getLongKey(); + final int chunkX = ChunkUtils.longToChunkX(chunkPos); + final int chunkZ = ChunkUtils.longToChunkZ(chunkPos); + chunksToWrite.add(new ChunkHighlightData(chunkX, chunkZ, entry.getLongValue())); + } + lock.readLock().unlock(); } + } catch (final Exception e) { + XaeroPlus.LOGGER.error("Error while writing all chunks to {} disk cache dimension: {}", database.databaseName, dimension.location(), e); } database.insertHighlightList(chunksToWrite, dimension); }); diff --git a/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightLocalCache.java b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightLocalCache.java index dc041d96..4ee2d362 100644 --- a/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightLocalCache.java +++ b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightLocalCache.java @@ -4,6 +4,7 @@ import xaeroplus.XaeroPlus; import java.util.Map; +import java.util.concurrent.TimeUnit; public class ChunkHighlightLocalCache extends ChunkHighlightBaseCacheHandler { private static final int maxNumber = 5000; @@ -25,15 +26,19 @@ public boolean addHighlight(final int x, final int z, final long foundTime) { private void limitChunksSize() { try { if (chunks.size() > maxNumber) { - synchronized (chunks) { + if (lock.readLock().tryLock(1, TimeUnit.SECONDS)) { // remove oldest 500 chunks var toRemove = chunks.long2LongEntrySet().stream() .sorted(Map.Entry.comparingByValue()) .limit(500) .mapToLong(Long2LongMap.Entry::getLongKey) .toArray(); - for (int i = 0; i < toRemove.length; i++) { - chunks.remove(toRemove[i]); + lock.readLock().unlock(); + if (lock.writeLock().tryLock(1, TimeUnit.SECONDS)) { + for (int i = 0; i < toRemove.length; i++) { + chunks.remove(toRemove[i]); + } + lock.writeLock().unlock(); } } } From 302d528acad85d6ac49d62f1811c339cefb2804d Mon Sep 17 00:00:00 2001 From: rfresh2 <89827146+rfresh2@users.noreply.github.com> Date: Wed, 18 Dec 2024 10:19:07 -0800 Subject: [PATCH 2/2] bump version --- settings.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/settings.gradle.kts b/settings.gradle.kts index 40c651a9..eb0b698b 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -8,7 +8,7 @@ pluginManagement { } } gradle.extra.apply { - set("mod_version", "2.24.8") + set("mod_version", "2.24.9") set("minecraft_version", "1.20.1") set("parchment_version", "2023.09.03") set("worldmap_version_fabric", "1.39.2")