fix: some improvements to GET chunk writing (#2853)

* fix: some improvements to GET chunk writing
 - ensure levelChunk is loaded before giving to copy GET - this is not necessarily guaranteed to be nonnull if two edits overlap. Whilst not advised, such an easy failure should not occur when two edits collide

* Prevent writing chunk sections when FAWE is also sending packets for a chunk and vice versa
- alter IntPair hashcode to be more often unique
- Utilise ConcurrentHashMap for free synchronisation

* Minor comment changes

* Use one-per-world-instance FaweBukkitWorld to store world chunk map
This commit is contained in:
Jordan
2024-09-25 18:20:49 +01:00
committed by GitHub
parent 5ac60f0d2e
commit b4635e85c9
16 changed files with 425 additions and 166 deletions

View File

@ -0,0 +1,67 @@
package com.fastasyncworldedit.bukkit;
import com.fastasyncworldedit.bukkit.adapter.NMSAdapter;
import com.fastasyncworldedit.bukkit.util.WorldUnloadedException;
import com.fastasyncworldedit.core.math.IntPair;
import com.sk89q.worldedit.bukkit.BukkitWorld;
import org.bukkit.Bukkit;
import org.bukkit.World;
import java.lang.ref.WeakReference;
import java.util.Collections;
import java.util.Map;
import java.util.WeakHashMap;
import java.util.concurrent.ConcurrentHashMap;
public class FaweBukkitWorld extends BukkitWorld {
private static final Map<World, FaweBukkitWorld> CACHE = Collections.synchronizedMap(new WeakHashMap<>());
private final ConcurrentHashMap<IntPair, NMSAdapter.ChunkSendLock> SENDING_CHUNKS = new ConcurrentHashMap<>();
/**
* Construct the object.
*
* @param world the world
*/
private FaweBukkitWorld(final World world) {
super(world);
}
public static FaweBukkitWorld of(World world) {
return CACHE.compute(world, (__, val) -> {
if (val == null) {
return new FaweBukkitWorld(world);
}
val.updateReference();
return val;
});
}
public static FaweBukkitWorld of(String worldName) {
World world = Bukkit.getWorld(worldName);
if (world == null) {
throw new UnsupportedOperationException("Unable to find org.bukkit.World instance for " + worldName + ". Is it loaded?");
}
return of(world);
}
public static ConcurrentHashMap<IntPair, NMSAdapter.ChunkSendLock> getWorldSendingChunksMap(FaweBukkitWorld world) {
return world.SENDING_CHUNKS;
}
public static ConcurrentHashMap<IntPair, NMSAdapter.ChunkSendLock> getWorldSendingChunksMap(String worldName) {
return of(worldName).SENDING_CHUNKS;
}
private void updateReference() {
World world = getWorld();
World bukkitWorld = Bukkit.getWorld(worldNameRef);
if (bukkitWorld == null) {
throw new WorldUnloadedException(worldNameRef);
} else if (bukkitWorld != world) {
worldRef = new WeakReference<>(bukkitWorld);
}
}
}

View File

@ -1,12 +1,17 @@
package com.fastasyncworldedit.bukkit.adapter;
import com.fastasyncworldedit.bukkit.FaweBukkitWorld;
import com.fastasyncworldedit.core.FAWEPlatformAdapterImpl;
import com.fastasyncworldedit.core.math.IntPair;
import com.fastasyncworldedit.core.queue.IChunkGet;
import com.fastasyncworldedit.core.util.MathMan;
import com.fastasyncworldedit.core.util.ReflectionUtils;
import com.sk89q.worldedit.world.block.BlockTypesCache;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.StampedLock;
import java.util.function.Function;
public class NMSAdapter implements FAWEPlatformAdapterImpl {
@ -140,4 +145,118 @@ public class NMSAdapter implements FAWEPlatformAdapterImpl {
((BukkitGetBlocks) chunk).send();
}
/**
* Atomically set the given chunk section to the chunk section array stored in the chunk, given the expected existing chunk
* section instance at the given layer position.
* <p>
* Acquires a (FAWE-implemented only) write-lock on the chunk packet lock, waiting if required before writing, then freeing
* the lock. Also sets a boolean to indicate a write is waiting and therefore reads should not occur.
* <p>
* Utilises ConcurrentHashMap#compute for easy synchronisation for all of the above. Only tryWriteLock is used in blocks
* synchronised using ConcurrentHashMap methods.
*
* @since TODO
*/
protected static <LevelChunkSection> boolean setSectionAtomic(
String worldName,
IntPair pair,
LevelChunkSection[] sections,
LevelChunkSection expected,
LevelChunkSection value,
int layer
) {
if (layer < 0 || layer >= sections.length) {
return false;
}
StampLockHolder holder = new StampLockHolder();
ConcurrentHashMap<IntPair, ChunkSendLock> chunks = FaweBukkitWorld.getWorldSendingChunksMap(worldName);
chunks.compute(pair, (k, lock) -> {
if (lock == null) {
lock = new ChunkSendLock();
} else if (lock.writeWaiting) {
throw new IllegalStateException("Attempting to write chunk section when write is already ongoing?!");
}
holder.stamp = lock.lock.tryWriteLock();
holder.chunkLock = lock;
lock.writeWaiting = true;
return lock;
});
try {
if (holder.stamp == 0) {
holder.stamp = holder.chunkLock.lock.writeLock();
}
return ReflectionUtils.compareAndSet(sections, expected, value, layer);
} finally {
chunks = FaweBukkitWorld.getWorldSendingChunksMap(worldName);
chunks.computeIfPresent(pair, (k, lock) -> {
if (lock != holder.chunkLock) {
throw new IllegalStateException("SENDING_CHUNKS stored lock does not equal lock attempted to be unlocked?!");
}
lock.lock.unlockWrite(holder.stamp);
lock.writeWaiting = false;
// Keep the lock, etc. in the map as we're going to be accessing again later when sending
return lock;
});
}
}
/**
* Called before sending a chunk packet, filling the given stamp and stampedLock arrays' zeroth indices if the chunk packet
* send should go ahead.
* <p>
* Chunk packets should be sent if both of the following are met:
* - There is no more than one current packet send ongoing
* - There is no chunk section "write" waiting or ongoing,
* which are determined by the number of readers currently locking the StampedLock (i.e. the number of sends), if the
* stamped lock is currently write-locked and if the boolean for waiting write is true.
* <p>
* Utilises ConcurrentHashMap#compute for easy synchronisation
*
* @since TODO
*/
protected static void beginChunkPacketSend(String worldName, IntPair pair, StampLockHolder stampedLock) {
ConcurrentHashMap<IntPair, ChunkSendLock> chunks = FaweBukkitWorld.getWorldSendingChunksMap(worldName);
chunks.compute(pair, (k, lock) -> {
if (lock == null) {
lock = new ChunkSendLock();
}
// Allow twice-read-locking, so if the packets have been created but not sent, we can queue another read
if (lock.writeWaiting || lock.lock.getReadLockCount() > 1 || lock.lock.isWriteLocked()) {
return lock;
}
stampedLock.stamp = lock.lock.readLock();
stampedLock.chunkLock = lock;
return lock;
});
}
/**
* Releases the read lock acquired when sending a chunk packet for a chunk
*
* @since TODO
*/
protected static void endChunkPacketSend(String worldName, IntPair pair, StampLockHolder lockHolder) {
ConcurrentHashMap<IntPair, ChunkSendLock> chunks = FaweBukkitWorld.getWorldSendingChunksMap(worldName);
chunks.computeIfPresent(pair, (k, lock) -> {
if (lock.lock != lockHolder.chunkLock.lock) {
throw new IllegalStateException("SENDING_CHUNKS stored lock does not equal lock attempted to be unlocked?!");
}
lock.lock.unlockRead(lockHolder.stamp);
// Do not continue to store the lock if we may not need it (i.e. chunk has been sent, may not be sent again)
return null;
});
}
public static final class StampLockHolder {
public long stamp;
public ChunkSendLock chunkLock = null;
}
public static final class ChunkSendLock {
public final StampedLock lock = new StampedLock();
public boolean writeWaiting = false;
}
}

View File

@ -118,9 +118,9 @@ public class BukkitWorld extends AbstractWorld {
HAS_MIN_Y = temp;
}
private WeakReference<World> worldRef;
protected WeakReference<World> worldRef;
//FAWE start
private final String worldNameRef;
protected final String worldNameRef;
//FAWE end
private final WorldNativeAccess<?, ?, ?> worldNativeAccess;