From 520f80c01151e1a057b267f09d236bf6d87c14ad Mon Sep 17 00:00:00 2001 From: Arbousier1 Date: Sun, 19 Jul 2026 20:03:05 +0800 Subject: [PATCH] perf(scheduler): publish capability caches atomically Reuse scheduler capability lookups without allowing concurrent readers to pair one target with another target's scheduler. --- .../mahjong/runtime/ServerScheduler.java | 39 +++- .../ServerSchedulerReflectionCacheTest.java | 218 +++++++++++++++++- 2 files changed, 245 insertions(+), 12 deletions(-) diff --git a/src/main/java/top/ellan/mahjong/runtime/ServerScheduler.java b/src/main/java/top/ellan/mahjong/runtime/ServerScheduler.java index 7a984b0..f73747b 100644 --- a/src/main/java/top/ellan/mahjong/runtime/ServerScheduler.java +++ b/src/main/java/top/ellan/mahjong/runtime/ServerScheduler.java @@ -91,6 +91,16 @@ public boolean isCancelled() { private final Plugin plugin; + /** + * Each capability is published as one immutable entry so readers can never + * observe a target from one resolution paired with another target's scheduler. + * The null scheduler value is intentional: it negatively caches unavailable + * Folia capabilities on standard Paper runtimes. + */ + private volatile SchedulerCapability globalSchedulerCapability; + private volatile SchedulerCapability regionSchedulerCapability; + private volatile SchedulerCapability entitySchedulerCapability; + public ServerScheduler(Plugin plugin) { this.plugin = Objects.requireNonNull(plugin, "plugin"); } @@ -313,15 +323,35 @@ public PluginTask removeEntity(Entity entity, long delayTicks) { } private Object globalRegionScheduler() { - return this.invokeNoArgs(this.plugin.getServer(), GET_GLOBAL_REGION_SCHEDULER); + Object server = this.plugin.getServer(); + SchedulerCapability capability = this.globalSchedulerCapability; + if (capability != null && capability.target() == server) { + return capability.scheduler(); + } + Object scheduler = this.invokeNoArgs(server, GET_GLOBAL_REGION_SCHEDULER); + this.globalSchedulerCapability = new SchedulerCapability(server, scheduler); + return scheduler; } private Object regionScheduler() { - return this.invokeNoArgs(this.plugin.getServer(), GET_REGION_SCHEDULER); + Object server = this.plugin.getServer(); + SchedulerCapability capability = this.regionSchedulerCapability; + if (capability != null && capability.target() == server) { + return capability.scheduler(); + } + Object scheduler = this.invokeNoArgs(server, GET_REGION_SCHEDULER); + this.regionSchedulerCapability = new SchedulerCapability(server, scheduler); + return scheduler; } private Object entityScheduler(Entity entity) { - return this.invokeNoArgs(entity, GET_ENTITY_SCHEDULER); + SchedulerCapability capability = this.entitySchedulerCapability; + if (capability != null && capability.target() == entity) { + return capability.scheduler(); + } + Object scheduler = this.invokeNoArgs(entity, GET_ENTITY_SCHEDULER); + this.entitySchedulerCapability = new SchedulerCapability(entity, scheduler); + return scheduler; } private boolean isPluginEnabled() { @@ -367,6 +397,9 @@ private static PluginTask wrap(Object task) { return task == null ? NO_OP_TASK : new ScheduledTaskHandle(task); } + private record SchedulerCapability(Object target, Object scheduler) { + } + private record BukkitTaskHandle(BukkitTask task) implements PluginTask { @Override public void cancel() { diff --git a/src/test/java/top/ellan/mahjong/runtime/ServerSchedulerReflectionCacheTest.java b/src/test/java/top/ellan/mahjong/runtime/ServerSchedulerReflectionCacheTest.java index 6464bcc..2776e4b 100644 --- a/src/test/java/top/ellan/mahjong/runtime/ServerSchedulerReflectionCacheTest.java +++ b/src/test/java/top/ellan/mahjong/runtime/ServerSchedulerReflectionCacheTest.java @@ -7,21 +7,36 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import io.papermc.paper.threadedregions.scheduler.EntityScheduler; import io.papermc.paper.threadedregions.scheduler.GlobalRegionScheduler; +import io.papermc.paper.threadedregions.scheduler.RegionScheduler; import io.papermc.paper.threadedregions.scheduler.ScheduledTask; import java.lang.reflect.Method; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; -import org.junit.jupiter.api.Test; +import org.bukkit.Location; import org.bukkit.Server; +import org.bukkit.World; +import org.bukkit.entity.Entity; import org.bukkit.plugin.Plugin; +import org.bukkit.scheduler.BukkitScheduler; +import org.bukkit.scheduler.BukkitTask; +import org.junit.jupiter.api.Test; final class ServerSchedulerReflectionCacheTest { + private static final Runnable NO_OP = () -> { + }; + @Test void cachesAvailableAndUnavailableMethodResolutionsPerRuntimeClass() { assertEquals(0, ServerScheduler.cachedMethodResolutionCount(ReflectionProbe.class)); @@ -38,7 +53,7 @@ void cachesAvailableAndUnavailableMethodResolutionsPerRuntimeClass() { } @Test - void invokesPaperSchedulerAndWrapsItsTaskThroughCachedMethods() { + void invokesPaperSchedulerAndReusesItsCapabilityByServerIdentity() { Plugin plugin = mock(Plugin.class); Server server = mock(Server.class); GlobalRegionScheduler globalScheduler = mock(GlobalRegionScheduler.class); @@ -55,16 +70,179 @@ void invokesPaperSchedulerAndWrapsItsTaskThroughCachedMethods() { return scheduledTask; }); - PluginTask task = new ServerScheduler(plugin).runGlobal(executions::incrementAndGet); + ServerScheduler scheduler = new ServerScheduler(plugin); + PluginTask first = scheduler.runGlobal(executions::incrementAndGet); + PluginTask second = scheduler.runGlobal(executions::incrementAndGet); - assertEquals(1, executions.get()); - assertFalse(task.isCancelled()); + assertEquals(2, executions.get()); + verify(server, times(1)).getGlobalRegionScheduler(); + assertFalse(first.isCancelled()); + assertFalse(second.isCancelled()); when(scheduledTask.isCancelled()).thenReturn(true); - assertTrue(task.isCancelled()); - task.cancel(); + assertTrue(first.isCancelled()); + first.cancel(); verify(scheduledTask).cancel(); } + @Test + void negativelyCachesUnavailableCapabilitiesOnPaper() { + Plugin plugin = mock(Plugin.class); + Server server = mock(Server.class); + World world = mock(World.class); + Entity entity = mock(Entity.class); + BukkitScheduler bukkitScheduler = mock(BukkitScheduler.class); + BukkitTask bukkitTask = mock(BukkitTask.class); + Location location = new Location(world, 0.0, 64.0, 0.0); + + when(plugin.isEnabled()).thenReturn(true); + when(plugin.getServer()).thenReturn(server); + when(server.getScheduler()).thenReturn(bukkitScheduler); + when(bukkitScheduler.runTask(eq(plugin), any(Runnable.class))).thenReturn(bukkitTask); + + ServerScheduler scheduler = new ServerScheduler(plugin); + scheduler.runGlobal(NO_OP); + scheduler.runGlobal(NO_OP); + scheduler.runRegion(location, NO_OP); + scheduler.runRegion(location, NO_OP); + scheduler.runEntity(entity, NO_OP); + scheduler.runEntity(entity, NO_OP); + + verify(server, times(1)).getGlobalRegionScheduler(); + verify(server, times(1)).getRegionScheduler(); + verify(entity, times(1)).getScheduler(); + verify(bukkitScheduler, times(6)).runTask(eq(plugin), any(Runnable.class)); + } + + @Test + void invalidatesCapabilitiesWhenTargetIdentityChanges() { + Plugin globalPlugin = mock(Plugin.class); + Server firstServer = mock(Server.class); + Server secondServer = mock(Server.class); + GlobalRegionScheduler firstGlobal = mock(GlobalRegionScheduler.class); + GlobalRegionScheduler secondGlobal = mock(GlobalRegionScheduler.class); + ScheduledTask task = mock(ScheduledTask.class); + + when(globalPlugin.isEnabled()).thenReturn(true); + when(globalPlugin.getServer()).thenReturn(firstServer, firstServer, secondServer, secondServer); + when(firstServer.getGlobalRegionScheduler()).thenReturn(firstGlobal); + when(secondServer.getGlobalRegionScheduler()).thenReturn(secondGlobal); + when(firstGlobal.run(any(Plugin.class), any())).thenReturn(task); + when(secondGlobal.run(any(Plugin.class), any())).thenReturn(task); + + ServerScheduler globalScheduler = new ServerScheduler(globalPlugin); + globalScheduler.runGlobal(NO_OP); + globalScheduler.runGlobal(NO_OP); + globalScheduler.runGlobal(NO_OP); + globalScheduler.runGlobal(NO_OP); + + verify(firstServer, times(1)).getGlobalRegionScheduler(); + verify(secondServer, times(1)).getGlobalRegionScheduler(); + + Plugin regionPlugin = mock(Plugin.class); + RegionScheduler firstRegion = mock(RegionScheduler.class); + RegionScheduler secondRegion = mock(RegionScheduler.class); + World world = mock(World.class); + Location location = new Location(world, 0.0, 64.0, 0.0); + + when(regionPlugin.isEnabled()).thenReturn(true); + when(regionPlugin.getServer()).thenReturn(firstServer, firstServer, secondServer, secondServer); + when(firstServer.getRegionScheduler()).thenReturn(firstRegion); + when(secondServer.getRegionScheduler()).thenReturn(secondRegion); + when(firstRegion.run(any(Plugin.class), any(Location.class), any())).thenReturn(task); + when(secondRegion.run(any(Plugin.class), any(Location.class), any())).thenReturn(task); + + ServerScheduler regionScheduler = new ServerScheduler(regionPlugin); + regionScheduler.runRegion(location, NO_OP); + regionScheduler.runRegion(location, NO_OP); + regionScheduler.runRegion(location, NO_OP); + regionScheduler.runRegion(location, NO_OP); + + verify(firstServer, times(1)).getRegionScheduler(); + verify(secondServer, times(1)).getRegionScheduler(); + + Plugin entityPlugin = mock(Plugin.class); + Entity firstEntity = mock(Entity.class); + Entity secondEntity = mock(Entity.class); + EntityScheduler firstEntityScheduler = mock(EntityScheduler.class); + EntityScheduler secondEntityScheduler = mock(EntityScheduler.class); + + when(entityPlugin.isEnabled()).thenReturn(true); + when(firstEntity.getScheduler()).thenReturn(firstEntityScheduler); + when(secondEntity.getScheduler()).thenReturn(secondEntityScheduler); + when(firstEntityScheduler.run(any(Plugin.class), any(), any(Runnable.class))).thenReturn(task); + when(secondEntityScheduler.run(any(Plugin.class), any(), any(Runnable.class))).thenReturn(task); + + ServerScheduler entityScheduler = new ServerScheduler(entityPlugin); + entityScheduler.runEntity(firstEntity, NO_OP); + entityScheduler.runEntity(firstEntity, NO_OP); + entityScheduler.runEntity(secondEntity, NO_OP); + entityScheduler.runEntity(secondEntity, NO_OP); + entityScheduler.runEntity(firstEntity, NO_OP); + + verify(firstEntity, times(2)).getScheduler(); + verify(secondEntity, times(1)).getScheduler(); + } + + @Test + void neverRoutesConcurrentEntityCallsThroughAnotherEntityScheduler() throws InterruptedException { + Plugin plugin = mock(Plugin.class); + Entity firstEntity = mock(Entity.class); + Entity secondEntity = mock(Entity.class); + EntityScheduler firstEntityScheduler = mock(EntityScheduler.class); + EntityScheduler secondEntityScheduler = mock(EntityScheduler.class); + ScheduledTask task = mock(ScheduledTask.class); + AtomicInteger wrongRoutes = new AtomicInteger(); + + when(plugin.isEnabled()).thenReturn(true); + when(firstEntity.getScheduler()).thenReturn(firstEntityScheduler); + when(secondEntity.getScheduler()).thenReturn(secondEntityScheduler); + when(firstEntityScheduler.run(any(Plugin.class), any(), any(Runnable.class))).thenAnswer(invocation -> { + if (!Thread.currentThread().getName().equals("scheduler-entity-first")) { + wrongRoutes.incrementAndGet(); + } + return task; + }); + when(secondEntityScheduler.run(any(Plugin.class), any(), any(Runnable.class))).thenAnswer(invocation -> { + if (!Thread.currentThread().getName().equals("scheduler-entity-second")) { + wrongRoutes.incrementAndGet(); + } + return task; + }); + + ServerScheduler scheduler = new ServerScheduler(plugin); + CountDownLatch ready = new CountDownLatch(2); + CountDownLatch start = new CountDownLatch(1); + CountDownLatch done = new CountDownLatch(2); + AtomicReference failure = new AtomicReference<>(); + Thread firstThread = new Thread( + () -> runEntityLoop(scheduler, firstEntity, ready, start, done, failure), + "scheduler-entity-first" + ); + Thread secondThread = new Thread( + () -> runEntityLoop(scheduler, secondEntity, ready, start, done, failure), + "scheduler-entity-second" + ); + + firstThread.start(); + secondThread.start(); + try { + assertTrue(ready.await(5, TimeUnit.SECONDS)); + start.countDown(); + assertTrue(done.await(30, TimeUnit.SECONDS)); + } finally { + start.countDown(); + firstThread.interrupt(); + secondThread.interrupt(); + firstThread.join(); + secondThread.join(); + } + + if (failure.get() != null) { + throw new AssertionError("Concurrent scheduler invocation failed", failure.get()); + } + assertEquals(0, wrongRoutes.get()); + } + @Test void doesNotFallbackAndScheduleTwiceWhenPaperSchedulerThrows() { Plugin plugin = mock(Plugin.class); @@ -80,13 +258,35 @@ void doesNotFallbackAndScheduleTwiceWhenPaperSchedulerThrows() { throw new IllegalStateException("scheduler failed after entry"); }); - assertThrows(IllegalStateException.class, () -> new ServerScheduler(plugin).runGlobal(() -> { - })); + assertThrows(IllegalStateException.class, () -> new ServerScheduler(plugin).runGlobal(NO_OP)); assertEquals(1, schedulerCalls.get()); verify(server, never()).getScheduler(); } + private static void runEntityLoop( + ServerScheduler scheduler, + Entity entity, + CountDownLatch ready, + CountDownLatch start, + CountDownLatch done, + AtomicReference failure + ) { + ready.countDown(); + try { + if (!start.await(5, TimeUnit.SECONDS)) { + throw new AssertionError("Concurrent scheduler start timed out"); + } + for (int iteration = 0; iteration < 20_000; iteration++) { + scheduler.runEntity(entity, NO_OP); + } + } catch (Throwable throwable) { + failure.compareAndSet(null, throwable); + } finally { + done.countDown(); + } + } + private static final class ReflectionProbe { public void accept(String value) { }