From 203948d4bbfc3c56e06f9e32adef44de77bcfd68 Mon Sep 17 00:00:00 2001 From: dordsor21 Date: Thu, 1 Aug 2024 21:24:42 +0100 Subject: [PATCH] feat: alter clipboard thread to move all clipboard loading from main thread --- .../worldedit/bukkit/WorldEditPlugin.java | 22 +++-- .../com/fastasyncworldedit/core/Fawe.java | 79 ++++++++++++++-- .../com/sk89q/worldedit/LocalSession.java | 4 +- .../worldedit/command/ClipboardCommands.java | 2 +- .../com/sk89q/worldedit/entity/Player.java | 30 +------ .../platform/AbstractPlayerActor.java | 89 +++++++++++++++---- 6 files changed, 167 insertions(+), 59 deletions(-) diff --git a/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/WorldEditPlugin.java b/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/WorldEditPlugin.java index faac1c955..3d6634b1f 100644 --- a/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/WorldEditPlugin.java +++ b/worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/WorldEditPlugin.java @@ -21,6 +21,7 @@ package com.sk89q.worldedit.bukkit; import com.fastasyncworldedit.bukkit.BukkitPermissionAttachmentManager; import com.fastasyncworldedit.bukkit.FaweBukkit; +import com.fastasyncworldedit.core.util.TaskManager; import com.fastasyncworldedit.core.util.UpdateNotification; import com.fastasyncworldedit.core.Fawe; import com.fastasyncworldedit.core.util.WEManager; @@ -91,6 +92,7 @@ import java.nio.file.Paths; import java.util.List; import java.util.Locale; import java.util.Optional; +import java.util.function.Supplier; import static com.google.common.base.Preconditions.checkNotNull; import static com.sk89q.worldedit.internal.anvil.ChunkDeleter.DELCHUNKS_FILE_NAME; @@ -565,14 +567,15 @@ public class WorldEditPlugin extends JavaPlugin { //FAWE start - Use cache over returning a direct BukkitPlayer BukkitPlayer wePlayer = getCachedPlayer(player); if (wePlayer == null) { - synchronized (player) { - wePlayer = getCachedPlayer(player); - if (wePlayer == null) { - wePlayer = new BukkitPlayer(this, player); - player.setMetadata("WE", new FixedMetadataValue(this, wePlayer)); - return wePlayer; + Supplier task = () -> { + BukkitPlayer bukkitPlayer = getCachedPlayer(player); + if (bukkitPlayer == null) { + bukkitPlayer = new BukkitPlayer(this, player); + player.setMetadata("WE", new FixedMetadataValue(this, bukkitPlayer)); } - } + return bukkitPlayer; + }; + TaskManager.taskManager().sync(task); } return wePlayer; //FAWE end @@ -588,11 +591,12 @@ public class WorldEditPlugin extends JavaPlugin { } BukkitPlayer reCachePlayer(Player player) { - synchronized (player) { + Supplier task = () -> { BukkitPlayer wePlayer = new BukkitPlayer(this, player); player.setMetadata("WE", new FixedMetadataValue(this, wePlayer)); return wePlayer; - } + }; + return TaskManager.taskManager().sync(task); } //FAWE end diff --git a/worldedit-core/src/main/java/com/fastasyncworldedit/core/Fawe.java b/worldedit-core/src/main/java/com/fastasyncworldedit/core/Fawe.java index 829f29da6..867a0d655 100644 --- a/worldedit-core/src/main/java/com/fastasyncworldedit/core/Fawe.java +++ b/worldedit-core/src/main/java/com/fastasyncworldedit/core/Fawe.java @@ -21,6 +21,7 @@ import com.sk89q.worldedit.internal.util.LogManagerCompat; import net.jpountz.lz4.LZ4Factory; import org.apache.logging.log4j.Logger; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.management.InstanceAlreadyExistsException; import javax.management.NotificationEmitter; @@ -37,7 +38,12 @@ import java.lang.management.MemoryUsage; import java.util.Date; import java.util.List; import java.util.UUID; +import java.util.concurrent.Callable; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -81,6 +87,7 @@ import java.util.concurrent.TimeUnit; public class Fawe { private static final Logger LOGGER = LogManagerCompat.getLogger(); + private static final ThreadLocal IS_UUID_KEYED_EXECUTOR_THREAD = ThreadLocal.withInitial(() -> false); private static Fawe instance; @@ -92,7 +99,7 @@ public class Fawe { * The platform specific implementation. */ private final IFawe implementation; - private final KeyQueuedExecutorService clipboardExecutor; + private final KeyQueuedExecutorService uuidKeyQueuedExecutorService; private FaweVersion version; private TextureUtil textures; private QueueHandler queueHandler; @@ -140,14 +147,27 @@ public class Fawe { }, 0); TaskManager.taskManager().repeat(timer, 1); - - clipboardExecutor = new KeyQueuedExecutorService<>(new ThreadPoolExecutor( + uuidKeyQueuedExecutorService = new KeyQueuedExecutorService<>(new ThreadPoolExecutor( 1, Settings.settings().QUEUE.PARALLEL_THREADS, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(), - new ThreadFactoryBuilder().setNameFormat("FAWE Clipboard - %d").build() + new ThreadFactoryBuilder().setNameFormat("FAWE UUID-key-queued - %d").setThreadFactory(new ThreadFactory() { + private final ThreadFactory defaultFactory = Executors.defaultThreadFactory(); + + @Override + public Thread newThread(@Nonnull Runnable r) { + return defaultFactory.newThread(() -> { + IS_UUID_KEYED_EXECUTOR_THREAD.set(true); + try { + r.run(); + } finally { + IS_UUID_KEYED_EXECUTOR_THREAD.set(false); // Clear the mark after execution + } + }); + } + }).build() )); } @@ -451,9 +471,58 @@ public class Fawe { * * @return Executor used for clipboard IO if clipboard on disk is enabled or null * @since 2.6.2 + * @deprecated Use any of {@link Fawe#submitUUIDKeyQueuedTask(UUID, Runnable)}, + * {@link Fawe#submitUUIDKeyQueuedTask(UUID, Runnable, Object), {@link Fawe#submitUUIDKeyQueuedTask(UUID, Callable)} + * to ensure if a thread is already a UUID-queued thread, the task is immediately run */ + @Deprecated(forRemoval = true, since = "TODO") public KeyQueuedExecutorService getClipboardExecutor() { - return this.clipboardExecutor; + return this.uuidKeyQueuedExecutorService; + } + + /** + * Submit a task to the UUID key-queued executor + * + * @return Future representing the tank + * @since TODO + */ + public Future submitUUIDKeyQueuedTask(UUID uuid, Runnable runnable) { + if (IS_UUID_KEYED_EXECUTOR_THREAD.get()) { + runnable.run(); + return CompletableFuture.completedFuture(null); + } + return this.uuidKeyQueuedExecutorService.submit(uuid, runnable); + } + + /** + * Submit a task to the UUID key-queued executor + * + * @return Future representing the tank + * @since TODO + */ + public Future submitUUIDKeyQueuedTask(UUID uuid, Runnable runnable, T result) { + if (IS_UUID_KEYED_EXECUTOR_THREAD.get()) { + runnable.run(); + return CompletableFuture.completedFuture(result); + } + return this.uuidKeyQueuedExecutorService.submit(uuid, runnable, result); + } + + /** + * Submit a task to the UUID key-queued executor + * + * @return Future representing the tank + * @since TODO + */ + public Future submitUUIDKeyQueuedTask(UUID uuid, Callable callable) { + if (IS_UUID_KEYED_EXECUTOR_THREAD.get()) { + try { + CompletableFuture.completedFuture(callable.call()); + } catch (Throwable t) { + CompletableFuture.failedFuture(t); + } + } + return this.uuidKeyQueuedExecutorService.submit(uuid, callable); } } diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/LocalSession.java b/worldedit-core/src/main/java/com/sk89q/worldedit/LocalSession.java index d82bcd463..f316467b2 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/LocalSession.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/LocalSession.java @@ -930,7 +930,7 @@ public class LocalSession implements TextureHolder { } } catch (EmptyClipboardException ignored) { } - DiskOptimizedClipboard doc = Fawe.instance().getClipboardExecutor().submit( + DiskOptimizedClipboard doc = Fawe.instance().submitUUIDKeyQueuedTask( uuid, () -> DiskOptimizedClipboard.loadFromFile(file) ).get(); @@ -954,7 +954,7 @@ public class LocalSession implements TextureHolder { } else { continue; } - Fawe.instance().getClipboardExecutor().submit(uuid, () -> { + Fawe.instance().submitUUIDKeyQueuedTask(uuid, () -> { doc.close(); // Ensure closed before deletion doc.getFile().delete(); }); diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/command/ClipboardCommands.java b/worldedit-core/src/main/java/com/sk89q/worldedit/command/ClipboardCommands.java index 378449755..ce7aa0369 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/command/ClipboardCommands.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/command/ClipboardCommands.java @@ -324,7 +324,7 @@ public class ClipboardCommands { } else { throw e; } - Fawe.instance().getClipboardExecutor().submit(actor.getUniqueId(), () -> { + Fawe.instance().submitUUIDKeyQueuedTask(actor.getUniqueId(), () -> { clipboard.close(); doc.getFile().delete(); }); diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/entity/Player.java b/worldedit-core/src/main/java/com/sk89q/worldedit/entity/Player.java index cc203190b..11561f313 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/entity/Player.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/entity/Player.java @@ -423,7 +423,7 @@ public interface Player extends Entity, Actor { if (Settings.settings().CLIPBOARD.USE_DISK && Settings.settings().CLIPBOARD.DELETE_ON_LOGOUT) { session.deleteClipboardOnDisk(); } else if (Settings.settings().CLIPBOARD.USE_DISK) { - Fawe.instance().getClipboardExecutor().submit(getUniqueId(), () -> session.setClipboard(null)); + Fawe.instance().submitUUIDKeyQueuedTask(getUniqueId(), () -> session.setClipboard(null)); } else if (Settings.settings().CLIPBOARD.DELETE_ON_LOGOUT) { session.setClipboard(null); } @@ -436,32 +436,8 @@ public interface Player extends Entity, Actor { void sendTitle(Component title, Component sub); /** - * Loads any history items from disk: - Should already be called if history on disk is enabled. + * Loads clipboard file from disk if it exists */ - default void loadClipboardFromDisk() { - File file = MainUtil.getFile( - Fawe.platform().getDirectory(), - Settings.settings().PATHS.CLIPBOARD + File.separator + getUniqueId() + ".bd" - ); - try { - getSession().loadClipboardFromDisk(file); - } catch (FaweClipboardVersionMismatchException e) { - print(e.getComponent()); - } catch (RuntimeException e) { - print(Caption.of("fawe.error.clipboard.invalid")); - e.printStackTrace(); - print(Caption.of("fawe.error.stacktrace")); - print(Caption.of("fawe.error.clipboard.load.failure")); - print(Caption.of("fawe.error.clipboard.invalid.info", file.getName(), file.length())); - print(Caption.of("fawe.error.stacktrace")); - } catch (Exception e) { - print(Caption.of("fawe.error.clipboard.invalid")); - e.printStackTrace(); - print(Caption.of("fawe.error.stacktrace")); - print(Caption.of("fawe.error.no-failure")); - print(Caption.of("fawe.error.clipboard.invalid.info", file.getName(), file.length())); - print(Caption.of("fawe.error.stacktrace")); - } - } + void loadClipboardFromDisk(); //FAWE end } diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/extension/platform/AbstractPlayerActor.java b/worldedit-core/src/main/java/com/sk89q/worldedit/extension/platform/AbstractPlayerActor.java index 5337fe720..c65397d11 100644 --- a/worldedit-core/src/main/java/com/sk89q/worldedit/extension/platform/AbstractPlayerActor.java +++ b/worldedit-core/src/main/java/com/sk89q/worldedit/extension/platform/AbstractPlayerActor.java @@ -19,10 +19,14 @@ package com.sk89q.worldedit.extension.platform; +import com.fastasyncworldedit.core.Fawe; import com.fastasyncworldedit.core.configuration.Caption; +import com.fastasyncworldedit.core.configuration.Settings; +import com.fastasyncworldedit.core.internal.exception.FaweClipboardVersionMismatchException; import com.fastasyncworldedit.core.internal.exception.FaweException; import com.fastasyncworldedit.core.math.MutableBlockVector3; import com.fastasyncworldedit.core.regions.FaweMaskManager; +import com.fastasyncworldedit.core.util.MainUtil; import com.fastasyncworldedit.core.util.TaskManager; import com.fastasyncworldedit.core.util.WEManager; import com.fastasyncworldedit.core.util.task.AsyncNotifyKeyedQueue; @@ -34,6 +38,7 @@ import com.sk89q.worldedit.entity.Player; import com.sk89q.worldedit.extent.Extent; import com.sk89q.worldedit.function.mask.Mask; import com.sk89q.worldedit.internal.cui.CUIEvent; +import com.sk89q.worldedit.internal.util.LogManagerCompat; import com.sk89q.worldedit.math.BlockVector3; import com.sk89q.worldedit.math.Vector3; import com.sk89q.worldedit.regions.ConvexPolyhedralRegion; @@ -62,12 +67,15 @@ import com.sk89q.worldedit.world.gamemode.GameMode; import com.sk89q.worldedit.world.gamemode.GameModes; import com.sk89q.worldedit.world.item.ItemType; import com.sk89q.worldedit.world.item.ItemTypes; +import org.apache.logging.log4j.Logger; import javax.annotation.Nullable; import java.io.File; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.StampedLock; /** * An abstract implementation of both a {@link Actor} and a {@link Player} @@ -76,8 +84,11 @@ import java.util.concurrent.atomic.AtomicInteger; */ public abstract class AbstractPlayerActor implements Actor, Player, Cloneable { + private static final Logger LOGGER = LogManagerCompat.getLogger(); + //FAWE start private final Map meta; + private final StampedLock clipboardLoading = new StampedLock(); // Queue for async tasks private final AtomicInteger runningCount = new AtomicInteger(); @@ -93,7 +104,7 @@ public abstract class AbstractPlayerActor implements Actor, Player, Cloneable { if (fe != null) { printError(fe.getComponent()); } else { - throwable.printStackTrace(); + LOGGER.error("Error in asyncNotifyQueue", throwable); } } }, this::getUniqueId); @@ -520,22 +531,71 @@ public abstract class AbstractPlayerActor implements Actor, Player, Cloneable { @Override public void setSelection(Region region) { - RegionSelector selector; - if (region instanceof ConvexPolyhedralRegion) { - selector = new ConvexPolyhedralRegionSelector((ConvexPolyhedralRegion) region); - } else if (region instanceof CylinderRegion) { - selector = new CylinderRegionSelector((CylinderRegion) region); - } else if (region instanceof Polygonal2DRegion) { - selector = new Polygonal2DRegionSelector((Polygonal2DRegion) region); - } else { - selector = new CuboidRegionSelector(null, region.getMinimumPoint(), - region.getMaximumPoint() - ); - } + RegionSelector selector = switch (region) { + case ConvexPolyhedralRegion blockVector3s -> new ConvexPolyhedralRegionSelector(blockVector3s); + case CylinderRegion blockVector3s -> new CylinderRegionSelector(blockVector3s); + case Polygonal2DRegion blockVector3s -> new Polygonal2DRegionSelector(blockVector3s); + default -> new CuboidRegionSelector(null, region.getMinimumPoint(), region.getMaximumPoint()); + }; selector.setWorld(region.getWorld()); getSession().setRegionSelector(getWorld(), selector); } + + @Override + public void loadClipboardFromDisk() { + if (clipboardLoading.isWriteLocked()) { + if (!Fawe.isMainThread()) { + long stamp = clipboardLoading.writeLock(); + clipboardLoading.unlockWrite(stamp); + } + return; + } + + final long stamp = clipboardLoading.writeLock(); + File file = MainUtil.getFile( + Fawe.platform().getDirectory(), + Settings.settings().PATHS.CLIPBOARD + File.separator + getUniqueId() + ".bd" + ); + Future fut = Fawe.instance().submitUUIDKeyQueuedTask(getUniqueId(), () -> { + try { + try { + getSession().loadClipboardFromDisk(file); + } catch (FaweClipboardVersionMismatchException e) { + print(e.getComponent()); + } catch (RuntimeException e) { + print(Caption.of("fawe.error.clipboard.invalid")); + LOGGER.error("Error loading clipboard form disk", e); + print(Caption.of("fawe.error.stacktrace")); + print(Caption.of("fawe.error.clipboard.load.failure")); + print(Caption.of("fawe.error.clipboard.invalid.info", file.getName(), file.length())); + print(Caption.of("fawe.error.stacktrace")); + } catch (Exception e) { + print(Caption.of("fawe.error.clipboard.invalid")); + LOGGER.error("Error loading clipboard form disk", e); + print(Caption.of("fawe.error.stacktrace")); + print(Caption.of("fawe.error.no-failure")); + print(Caption.of("fawe.error.clipboard.invalid.info", file.getName(), file.length())); + print(Caption.of("fawe.error.stacktrace")); + } + } finally { + clipboardLoading.unlockWrite(stamp); + } + }); + if (Fawe.isMainThread()) { + return; + } + try { + fut.get(); + } catch (Exception e) { + print(Caption.of("fawe.error.clipboard.invalid")); + LOGGER.error("Error loading clipboard form disk", e); + print(Caption.of("fawe.error.stacktrace")); + print(Caption.of("fawe.error.no-failure")); + print(Caption.of("fawe.error.clipboard.invalid.info", file.getName(), file.length())); + print(Caption.of("fawe.error.stacktrace")); + } + } //FAWE end @Override @@ -694,10 +754,9 @@ public abstract class AbstractPlayerActor implements Actor, Player, Cloneable { @Override public boolean equals(Object other) { - if (!(other instanceof Player)) { + if (!(other instanceof Player other2)) { return false; } - Player other2 = (Player) other; return other2.getName().equals(getName()); }