Cache block state conversions in the WorldEdit and FAWE loggers

src/main/java/net/coreprotect/worldedit/BlockStateConversion.java

@@ -0,0 +1,71 @@
+package net.coreprotect.worldedit;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.bukkit.Material;
+import org.bukkit.block.data.BlockData;
+
+import com.sk89q.worldedit.bukkit.BukkitAdapter;
+import com.sk89q.worldedit.world.block.BlockState;
+
+/**
+ * The Bukkit conversions of one WorldEdit block state: its Material, a BlockData template and the BlockData string.
+ * Converting a state builds a new BlockData each time and serializing it builds a new string, while an edit usually
+ * repeats a few states many times, so the loggers convert each state once.
+ *
+ * All fields are final, so an instance is fully visible to any thread that reads a reference to it, however that
+ * reference was published. The block state registry is fixed once the server is running, so the cache is bounded by
+ * the number of block states.
+ */
+final class BlockStateConversion {
+
+    private static final Map<BlockState, BlockStateConversion> CACHE = new ConcurrentHashMap<>();
+
+    private final Material material;
+    private final BlockData template;
+    private final String dataString;
+
+    private BlockStateConversion(BlockState state) {
+        this.material = BukkitAdapter.adapt(state.getBlockType());
+        this.template = BukkitAdapter.adapt(state);
+        this.dataString = template.getAsString();
+    }
+
+    /**
+     * WorldEdit keeps one instance per block state, so lookups normally match by identity.
+     */
+    static BlockStateConversion of(BlockState state) {
+        BlockStateConversion conversion = CACHE.get(state);
+        if (conversion == null) {
+            conversion = new BlockStateConversion(state);
+            BlockStateConversion existing = CACHE.putIfAbsent(state, conversion);
+            if (existing != null) {
+                conversion = existing;
+            }
+        }
+        return conversion;
+    }
+
+    Material getMaterial() {
+        return material;
+    }
+
+    /**
+     * The shared template, for reading only.
+     */
+    BlockData getTemplate() {
+        return template;
+    }
+
+    /**
+     * A copy of the template that the caller owns, since logging may modify it.
+     */
+    BlockData createBlockData() {
+        return template.clone();
+    }
+
+    String getDataString() {
+        return dataString;
+    }
+}

src/main/java/net/coreprotect/worldedit/CoreProtectLogger.java

@@ -56,8 +56,9 @@ public <T extends BlockStateHolder<T>> boolean setBlock(BlockVector3 position, T
             return eventExtent.setBlock(position, block);
         }
 
-        Material oldType = BukkitAdapter.adapt(oldBlock.getBlockType());
-        BlockData oldBlockData = BukkitAdapter.adapt(oldBlock);
+        BlockStateConversion oldConversion = BlockStateConversion.of(oldBlock);
+        Material oldType = oldConversion.getMaterial();
+        BlockData oldBlockData = oldConversion.createBlockData();
         Location location = new Location(world, position.getBlockX(), position.getBlockY(), position.getBlockZ());
         BaseBlock baseBlock = WorldEditLogger.needsBaseBlock(oldType, config) ? eventExtent.getFullBlock(position) : null;
         // Reads below this stage reach the live world, so capture the other half before setBlock or its physics can change it
@@ -69,7 +70,8 @@ public <T extends BlockStateHolder<T>> boolean setBlock(BlockVector3 position, T
         ItemStack[] containerData = !CoreProtectEditSessionEvent.isFAWE() && config.ITEM_TRANSACTIONS && BlockGroup.CONTAINERS.contains(oldType) ? ItemUtils.getContainerContents(oldType, null, location) : null;
 
         if (eventExtent.setBlock(position, block)) {
-            WorldEditLogger.postProcess(eventExtent, eventActor, position, location, block, baseBlock, oldType, oldBlockData, containerData, otherHalf);
+            BlockStateConversion newConversion = BlockStateConversion.of(block.toImmutableState());
+            WorldEditLogger.postProcess(eventActor, location, baseBlock, oldType, oldBlockData, oldConversion.getDataString(), newConversion.getMaterial(), newConversion.createBlockData(), newConversion.getDataString(), containerData, true, otherHalf);
             return true;
         }
 

src/main/java/net/coreprotect/worldedit/FastAsyncWorldEditLogger.java

@@ -12,7 +12,6 @@
 import com.sk89q.worldedit.bukkit.BukkitAdapter;
 import com.sk89q.worldedit.extension.platform.Actor;
 import com.sk89q.worldedit.extent.Extent;
-import com.sk89q.worldedit.math.BlockVector3;
 import com.sk89q.worldedit.world.World;
 import com.sk89q.worldedit.world.block.BaseBlock;
 import com.sk89q.worldedit.world.block.BlockState;
@@ -25,13 +24,11 @@ final class FastAsyncWorldEditLogger implements IBatchProcessor {
     private static final ProcessorScope SCOPE = resolveScope();
     private final Actor actor;
     private final World eventWorld;
-    private final Extent extent;
     private final org.bukkit.World world;
 
     FastAsyncWorldEditLogger(Actor actor, World world, Extent extent) {
         this.actor = actor;
         this.eventWorld = world;
-        this.extent = extent;
         this.world = BukkitAdapter.adapt(world);
     }
 
@@ -72,15 +69,14 @@ public IChunkSet processSet(IChunk chunk, IChunkGet get, IChunkSet set) {
                 int x = index & 15;
                 int y = (layer << 4) + (index >> 8);
                 int z = (index >> 4) & 15;
-                BlockState oldBlock = BlockState.getFromOrdinal(oldOrdinal);
-                BlockState newBlock = BlockState.getFromOrdinal(newOrdinal);
-                Material oldType = BukkitAdapter.adapt(oldBlock.getBlockType());
-                BlockData oldBlockData = BukkitAdapter.adapt(oldBlock);
-                BlockVector3 position = BlockVector3.at(chunkX + x, y, chunkZ + z);
+                BlockStateConversion oldConversion = conversion(oldOrdinal);
+                BlockStateConversion newConversion = conversion(newOrdinal);
+                Material oldType = oldConversion.getMaterial();
+                BlockData oldBlockData = oldConversion.createBlockData();
                 Location location = new Location(world, chunkX + x, y, chunkZ + z);
                 BaseBlock baseBlock = WorldEditLogger.needsBaseBlock(oldType, config) ? get.getFullBlock(x, y, z) : null;
                 org.bukkit.block.BlockState lowerHalf = WorldEditLogger.isLoggedTopHalf(oldType, oldBlockData) ? getLowerHalf(get, set, x, y, z, chunkX + x, chunkZ + z, oldBlockData) : null;
-                WorldEditLogger.postProcess(extent, actor, position, location, newBlock, baseBlock, oldType, oldBlockData, null, false, lowerHalf);
+                WorldEditLogger.postProcess(actor, location, baseBlock, oldType, oldBlockData, oldConversion.getDataString(), newConversion.getMaterial(), newConversion.createBlockData(), newConversion.getDataString(), null, false, lowerHalf);
             }
         }
         return set;
@@ -108,12 +104,28 @@ private org.bukkit.block.BlockState getLowerHalf(IChunkGet get, IChunkSet set, i
             return null;
         }
 
-        BlockData blockData = BukkitAdapter.adapt(BlockState.getFromOrdinal(ordinal));
-        if (!WorldEditLogger.isOtherHalf(topHalf, blockData)) {
+        BlockStateConversion conversion = conversion(ordinal);
+        if (!WorldEditLogger.isOtherHalf(topHalf, conversion.getTemplate())) {
             return null;
         }
 
-        return new WorldEditBlockState(new Location(world, worldX, lowerY, worldZ), blockData.getMaterial(), blockData);
+        return new WorldEditBlockState(new Location(world, worldX, lowerY, worldZ), conversion.getMaterial(), conversion.createBlockData());
+    }
+
+    /*
+     * BlockStateConversion indexed by FAWE block state ordinal, which avoids hashing the state for every changed block.
+     * FAWE runs this processor on several threads. A plain array is safe because every field of BlockStateConversion
+     * is final, and concurrent writers store equivalent conversions, so a lost race only repeats the work.
+     */
+    private static final BlockStateConversion[] CONVERSIONS = new BlockStateConversion[BlockTypesCache.states.length];
+
+    private static BlockStateConversion conversion(int ordinal) {
+        BlockStateConversion conversion = CONVERSIONS[ordinal];
+        if (conversion == null) {
+            conversion = BlockStateConversion.of(BlockState.getFromOrdinal(ordinal));
+            CONVERSIONS[ordinal] = conversion;
+        }
+        return conversion;
     }
 
     @Override

src/main/java/net/coreprotect/worldedit/WorldEditLogger.java

@@ -21,13 +21,11 @@
 
 import com.sk89q.jnbt.CompoundTag;
 import com.sk89q.jnbt.NBTUtils;
-import com.sk89q.worldedit.bukkit.BukkitAdapter;
 import com.sk89q.worldedit.bukkit.WorldEditPlugin;
 import com.sk89q.worldedit.extension.platform.Actor;
 import com.sk89q.worldedit.extent.Extent;
 import com.sk89q.worldedit.math.BlockVector3;
 import com.sk89q.worldedit.world.block.BaseBlock;
-import com.sk89q.worldedit.world.block.BlockStateHolder;
 
 import net.coreprotect.config.Config;
 import net.coreprotect.consumer.Queue;
@@ -79,30 +77,24 @@ protected static BlockState getOtherHalf(Extent extent, BlockVector3 position, L
             return null;
         }
 
-        BlockData otherData = BukkitAdapter.adapt(extent.getBlock(position.add(0, offset, 0)));
-        if (!isOtherHalf(blockData, otherData)) {
+        BlockStateConversion other = BlockStateConversion.of(extent.getBlock(position.add(0, offset, 0)));
+        if (!isOtherHalf(blockData, other.getTemplate())) {
             return null;
         }
 
         Location otherLocation = location.clone();
         otherLocation.setY(otherY);
-        return new WorldEditBlockState(otherLocation, otherData.getMaterial(), otherData);
-    }
-
-    protected static void postProcess(Extent extent, Actor actor, BlockVector3 position, Location location, BlockStateHolder<?> blockStateHolder, BaseBlock baseBlock, Material oldType, BlockData oldBlockData, ItemStack[] containerContents, BlockState otherHalf) {
-        postProcess(extent, actor, position, location, blockStateHolder, baseBlock, oldType, oldBlockData, containerContents, true, otherHalf);
+        return new WorldEditBlockState(otherLocation, other.getMaterial(), other.createBlockData());
     }
 
     /**
+     * The loggers pass Bukkit data already converted through BlockStateConversion.
+     *
+     * @param oldBlockData A fresh instance owned by this call, since it may be modified
+     * @param newBlockData A fresh instance owned by this call, since it may be modified
      * @param otherHalf The matching other half of a double block as it was before this edit changed it, or null. Callers that do not log block physics only supply the lower half of a top half.
      */
-    protected static void postProcess(Extent extent, Actor actor, BlockVector3 position, Location location, BlockStateHolder<?> blockStateHolder, BaseBlock baseBlock, Material oldType, BlockData oldBlockData, ItemStack[] containerContents, boolean logBlockPhysics, BlockState otherHalf) {
-        BlockData newBlockData = BukkitAdapter.adapt(blockStateHolder.toImmutableState());
-        Material newType = newBlockData.getMaterial();
-
-        String oldBlockDataString = oldBlockData.getAsString();
-        String newBlockDataString = newBlockData.getAsString();
-
+    protected static void postProcess(Actor actor, Location location, BaseBlock baseBlock, Material oldType, BlockData oldBlockData, String oldBlockDataString, Material newType, BlockData newBlockData, String newBlockDataString, ItemStack[] containerContents, boolean logBlockPhysics, BlockState otherHalf) {
         if (!oldType.equals(newType) || !oldBlockDataString.equals(newBlockDataString)) {
             BlockState oldBlock = new WorldEditBlockState(location, oldType, oldBlockData);
             BlockState newBlock = new WorldEditBlockState(location, newType, newBlockData);