From d0ff3d31fe0d09a08307490bbdfdbee4f440788a Mon Sep 17 00:00:00 2001 From: rfresh2 <89827146+rfresh2@users.noreply.github.com> Date: Wed, 20 Nov 2024 21:40:46 -0800 Subject: [PATCH] simplify cache impl now that highlights are rendered by accessing highlights snapshots, there is little need for r/w locking. synchronized maps perform just as well and remove the need for lock management --- .../ChunkHighlightBaseCacheHandler.java | 80 +++---------------- .../ChunkHighlightCacheDimensionHandler.java | 71 +++++++--------- .../highlights/ChunkHighlightDatabase.java | 15 ++-- .../highlights/ChunkHighlightLocalCache.java | 11 +-- 4 files changed, 49 insertions(+), 128 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 79b05555..fc0cbbf3 100644 --- a/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightBaseCacheHandler.java +++ b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightBaseCacheHandler.java @@ -1,23 +1,13 @@ package xaeroplus.feature.render.highlights; -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 it.unimi.dsi.fastutil.longs.*; 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 ReadWriteLock lock = new StampedLock().asReadWriteLock(); - public final Long2LongMap chunks = new Long2LongOpenHashMap(); + public final Long2LongMap chunks = Long2LongMaps.synchronize(new Long2LongOpenHashMap()); @Override public boolean addHighlight(final int x, final int z) { @@ -26,28 +16,14 @@ 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); - 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); - } + chunks.put(chunkPos, foundTime); return true; } @Override public boolean removeHighlight(final int x, final int z) { final long chunkPos = chunkPosToLong(x, z); - 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); - } + chunks.remove(chunkPos); return true; } @@ -58,26 +34,11 @@ public boolean isHighlighted(final int x, final int z, ResourceKey dimens @Override public LongList getHighlightsSnapshot(final ResourceKey dimension) { - 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(); + return new LongArrayList(chunks.keySet()); } public boolean isHighlighted(final long 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; + return chunks.containsKey(chunkPos); } @Override @@ -88,36 +49,15 @@ public Long2LongMap getHighlightsState() { @Override public void loadPreviousState(final Long2LongMap state) { if (state == null) return; - 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); - } + chunks.putAll(state); } public void replaceState(final Long2LongOpenHashMap 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); - } + chunks.clear(); + chunks.putAll(state); } public void reset() { - 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); - } + chunks.clear(); } } 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 5ddeba26..896c639c 100644 --- a/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightCacheDimensionHandler.java +++ b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightCacheDimensionHandler.java @@ -5,12 +5,10 @@ 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; @@ -51,16 +49,11 @@ private void loadHighlightsInWindow() { windowRegionX - windowRegionSize, windowRegionX + windowRegionSize, 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(); + synchronized (this.chunks) { + for (int i = 0; i < chunks.size(); i++) { + final ChunkHighlightData chunk = chunks.get(i); + this.chunks.put(chunkPosToLong(chunk.x(), chunk.z()), chunk.foundTime()); } - } catch (final Exception e) { - XaeroPlus.LOGGER.error("Failed to load highlights in window for {} disk cache dimension: {}", database.databaseName, dimension.location(), e); } }); } @@ -68,29 +61,24 @@ private void loadHighlightsInWindow() { private void writeHighlightsOutsideWindowToDatabase() { executorService.execute(() -> { final List chunksToWrite = new ArrayList<>(); - 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(); - } + 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(); } - 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); }); @@ -99,19 +87,14 @@ private void writeHighlightsOutsideWindowToDatabase() { public ListenableFuture writeAllHighlightsToDatabase() { return executorService.submit(() -> { final List chunksToWrite = new ArrayList<>(chunks.size()); - 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(); + 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())); } - } 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/ChunkHighlightDatabase.java b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightDatabase.java index ceecea8a..569e3b15 100644 --- a/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightDatabase.java +++ b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightDatabase.java @@ -15,7 +15,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; import static xaeroplus.util.ChunkUtils.regionCoordToChunkCoord; @@ -84,12 +83,16 @@ public void insertHighlightList(final List chunks, final Res private void insertHighlightsListInternal(final List chunks, final ResourceKey dimension) { try { - String statement = "INSERT OR IGNORE INTO \"" + getTableName(dimension) + "\" VALUES "; - statement += chunks.stream() - .map(chunk -> "(" + chunk.x() + ", " + chunk.z() + ", " + chunk.foundTime() + ")") - .collect(Collectors.joining(", ")); + StringBuilder sb = new StringBuilder("INSERT OR IGNORE INTO \"" + getTableName(dimension) + "\" VALUES "); + for (int i = 0; i < chunks.size(); i++) { + ChunkHighlightData chunk = chunks.get(i); + sb.append("(").append(chunk.x()).append(", ").append(chunk.z()).append(", ").append(chunk.foundTime()).append(")"); + if (i < chunks.size() - 1) { + sb.append(", "); + } + } try (var stmt = connection.createStatement()) { - stmt.executeUpdate(statement); + stmt.executeUpdate(sb.toString()); } } catch (Exception e) { XaeroPlus.LOGGER.error("Error inserting {} chunks into {} database in dimension: {}", chunks.size(), databaseName, dimension.location(), e); 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 4ee2d362..dc041d96 100644 --- a/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightLocalCache.java +++ b/common/src/main/java/xaeroplus/feature/render/highlights/ChunkHighlightLocalCache.java @@ -4,7 +4,6 @@ import xaeroplus.XaeroPlus; import java.util.Map; -import java.util.concurrent.TimeUnit; public class ChunkHighlightLocalCache extends ChunkHighlightBaseCacheHandler { private static final int maxNumber = 5000; @@ -26,19 +25,15 @@ public boolean addHighlight(final int x, final int z, final long foundTime) { private void limitChunksSize() { try { if (chunks.size() > maxNumber) { - if (lock.readLock().tryLock(1, TimeUnit.SECONDS)) { + synchronized (chunks) { // remove oldest 500 chunks var toRemove = chunks.long2LongEntrySet().stream() .sorted(Map.Entry.comparingByValue()) .limit(500) .mapToLong(Long2LongMap.Entry::getLongKey) .toArray(); - 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(); + for (int i = 0; i < toRemove.length; i++) { + chunks.remove(toRemove[i]); } } }