GH-437 Improve schedulers

buildSrc/src/main/kotlin/Versions.kt

@@ -36,6 +36,7 @@ object Versions {
     const val FABRIC_LOADER = "0.16.9"
     const val FABRIC_COMMAND_API_V2 = "2.2.37+7feeb7331c"
     const val FABRIC_COMMAND_API_V1 = "1.2.56+f71b366f73"
+    const val FABRIC_LIFECYCLE_EVENTS_V1 = "2.3.12+6c1df36019"
 
     // ChatGPT
     const val GSON = "2.11.0"

examples/fabric/src/main/java/dev/rollczi/example/fabric/client/command/ClientCommands.java

@@ -1,6 +1,7 @@
 package dev.rollczi.example.fabric.client.command;
 
 import dev.rollczi.litecommands.annotations.argument.Arg;
+import dev.rollczi.litecommands.annotations.async.Async;
 import dev.rollczi.litecommands.annotations.command.Command;
 import dev.rollczi.litecommands.annotations.context.Sender;
 import dev.rollczi.litecommands.annotations.execute.Execute;
@@ -36,4 +37,14 @@ String health(@Arg ClientPlayerEntity player) {
         return String.valueOf(player.getHealth());
     }
 
+    @Execute(name = "thread1")
+    String thread1() {
+        return Thread.currentThread().getName();
+    }
+
+    @Execute(name = "thread2")
+    @Async
+    String thread2() {
+        return Thread.currentThread().getName();
+    }
 }

examples/fabric/src/main/java/dev/rollczi/example/fabric/server/command/ExampleCommand.java

@@ -1,6 +1,7 @@
 package dev.rollczi.example.fabric.server.command;
 
 import dev.rollczi.litecommands.annotations.argument.Arg;
+import dev.rollczi.litecommands.annotations.async.Async;
 import dev.rollczi.litecommands.annotations.command.Command;
 import dev.rollczi.litecommands.annotations.execute.Execute;
 import dev.rollczi.litecommands.annotations.join.Join;
@@ -19,4 +20,15 @@ void sendMessage(@Arg("player") ServerPlayerEntity player, @Join("reason") Strin
     Text sendMessage(@Quoted @Arg String message) {
         return Text.of("You saied: " + message);
     }
+
+    @Execute(name = "thread1")
+    String thread1() {
+        return Thread.currentThread().getName();
+    }
+
+    @Execute(name = "thread2")
+    @Async
+    String thread2() {
+        return Thread.currentThread().getName();
+    }
 }

litecommands-annotations/test/dev/rollczi/litecommands/annotations/async/AsyncArgumentsInitializeTest.java

@@ -50,7 +50,6 @@ void test() {
         List<CompletableFuture<AssertExecute>> futures = new ArrayList<>();
         for (int i = 0; i < 50; i++) {
             TestPlatform platform = LiteCommandsTestFactory.startPlatform(config -> config
-                .scheduler(new SchedulerExecutorPoolImpl("lite-commands", 10))
                 .commands(command)
             );
 

litecommands-annotations/test/dev/rollczi/litecommands/annotations/async/AsyncCommandTest.java

@@ -1,6 +1,5 @@
 package dev.rollczi.litecommands.annotations.async;
 
-import dev.rollczi.litecommands.unit.annotations.LiteTestSpec;
 import dev.rollczi.litecommands.annotations.argument.Arg;
 import dev.rollczi.litecommands.annotations.command.Command;
 import dev.rollczi.litecommands.annotations.context.Context;
@@ -10,10 +9,10 @@
 import dev.rollczi.litecommands.argument.resolver.ArgumentResolver;
 import dev.rollczi.litecommands.context.ContextResult;
 import dev.rollczi.litecommands.invocation.Invocation;
-import dev.rollczi.litecommands.scheduler.SchedulerExecutorPoolImpl;
+import dev.rollczi.litecommands.scheduler.SchedulerExecutorPoolBuilder;
 import dev.rollczi.litecommands.unit.AssertExecute;
 import dev.rollczi.litecommands.unit.TestSender;
-import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import dev.rollczi.litecommands.unit.annotations.LiteTestSpec;
 import org.junit.jupiter.api.Test;
 
 import java.util.Date;
@@ -24,11 +23,19 @@
 
 class AsyncCommandTest extends LiteTestSpec {
 
+    private static final String mainThreadName = "main_thread";
+    private static final String asyncThreadName = "async_thread";
+
     private static final int DELAY = 500;
     private static final int MARGIN = 200;
 
     static LiteTestConfig config = builder -> builder
-        .scheduler(new SchedulerExecutorPoolImpl("test", 1))
+        .scheduler(
+            new SchedulerExecutorPoolBuilder()
+                .setMainExecutorThreadName(mainThreadName)
+                .setAsyncExecutorThreadName(asyncThreadName)
+                .build()
+        )
         .context(Date.class, invocation -> ContextResult.ok(() -> {
             try {
                 Thread.sleep(DELAY);
@@ -79,7 +86,7 @@ public String testAsyncArgs(@Context Date date, @Arg String first, @Async @Arg S
 
         @Async
         @Execute(name = "async-args-and-method")
-        public String testAsyncArgs2(@Context Date date,  @Async @Arg String first, @Arg String second) {
+        public String testAsyncArgs2(@Context Date date, @Async @Arg String first, @Arg String second) {
             return Thread.currentThread().getName() + " args [first=" + first + ", second=" + second + "]";
         }
 
@@ -93,13 +100,13 @@ public String testAsyncArgs3(@Async @Arg SomeClass someClass) {
     @Test
     void testSync() {
         platform.execute("test sync")
-            .assertSuccess("scheduler-test-main");
+            .assertSuccess(mainThreadName);
     }
 
     @Test
     void testAsync() {
         platform.execute("test async")
-            .assertSuccess("scheduler-test-async-0");
+            .assertSuccess(asyncThreadName);
     }
 
     @Test
@@ -111,7 +118,7 @@ void testAsyncArgs() {
             .until(() -> result.isDone());
 
         result.join()
-            .assertSuccess("scheduler-test-main args [first=scheduler-test-main, second=scheduler-test-async-0]");
+            .assertSuccess(mainThreadName + " args [first=" + mainThreadName + ", second=" + asyncThreadName + "]");
     }
 
     @Test
@@ -123,7 +130,7 @@ void testAsyncArgsAndMethod() {
             .until(() -> result.isDone());
 
         result.join()
-            .assertSuccess("scheduler-test-async-0 args [first=scheduler-test-async-0, second=scheduler-test-main]");
+            .assertSuccess(asyncThreadName + " args [first=" + asyncThreadName + ", second=" + mainThreadName + "]");
     }
 
     @Test

litecommands-annotations/test/dev/rollczi/litecommands/annotations/async/ParallelAsyncCommandTest.java

@@ -24,7 +24,6 @@ public class ParallelAsyncCommandTest extends LiteTestSpec {
     private static final int DELAY = 100;
 
     static LiteTestConfig config = builder -> builder
-        .scheduler(new SchedulerExecutorPoolImpl("test"))
         .context(Date.class, invocation -> ContextResult.ok(() -> {
             try {
                 Thread.sleep(DELAY);

litecommands-bukkit/src/dev/rollczi/litecommands/bukkit/BukkitSchedulerImpl.java

@@ -1,5 +1,6 @@
 package dev.rollczi.litecommands.bukkit;
 
+import com.google.common.base.Throwables;
 import dev.rollczi.litecommands.scheduler.Scheduler;
 import dev.rollczi.litecommands.scheduler.SchedulerPoll;
 import dev.rollczi.litecommands.shared.ThrowingSupplier;
@@ -48,8 +49,7 @@ private <T> CompletableFuture<T> supplySync(SchedulerPoll type, ThrowingSupplier
 
         if (delay.isZero()) {
             bukkitScheduler.runTask(plugin, () -> tryRun(type, future, supplier));
-        }
-        else {
+        } else {
             bukkitScheduler.runTaskLater(plugin, () -> tryRun(type, future, supplier), toTicks(delay));
         }
 
@@ -61,8 +61,7 @@ private <T> CompletableFuture<T> supplyAsync(SchedulerPoll type, ThrowingSupplie
 
         if (delay.isZero()) {
             bukkitScheduler.runTaskAsynchronously(plugin, () -> tryRun(type, future, supplier));
-        }
-        else {
+        } else {
             bukkitScheduler.runTaskLaterAsynchronously(plugin, () -> tryRun(type, future, supplier), toTicks(delay));
         }
 
@@ -72,12 +71,10 @@ private <T> CompletableFuture<T> supplyAsync(SchedulerPoll type, ThrowingSupplie
     private <T> CompletableFuture<T> tryRun(SchedulerPoll type, CompletableFuture<T> future, ThrowingSupplier<T, Throwable> supplier) {
         try {
             future.complete(supplier.get());
-        }
-        catch (Throwable throwable) {
+        } catch (Throwable throwable) {
             future.completeExceptionally(throwable);
-
             if (type.isLogging()) {
-                throwable.printStackTrace();
+                plugin.getLogger().severe("Error completing command future: \n" + Throwables.getStackTraceAsString(throwable));
             }
         }
 

litecommands-core/src/dev/rollczi/litecommands/scheduler/SchedulerExecutorPoolBuilder.java

@@ -0,0 +1,90 @@
+package dev.rollczi.litecommands.scheduler;
+
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class SchedulerExecutorPoolBuilder {
+
+    private ScheduledExecutorService mainExecutor;
+    private boolean closeMainExecutorOnShutdown;
+    private String mainExecutorThreadName;
+
+    private ScheduledExecutorService asyncExecutor;
+    private boolean closeAsyncExecutorOnShutdown;
+    private String asyncExecutorThreadName;
+
+    public SchedulerExecutorPoolBuilder setMainExecutor(ScheduledExecutorService mainExecutor, boolean closeOnShutdown) {
+        this.mainExecutor = mainExecutor;
+        this.closeMainExecutorOnShutdown = closeOnShutdown;
+        return this;
+    }
+
+    public SchedulerExecutorPoolBuilder setAsyncExecutor(ScheduledExecutorService asyncExecutor, boolean closeOnShutdown) {
+        this.asyncExecutor = asyncExecutor;
+        this.closeAsyncExecutorOnShutdown = closeOnShutdown;
+        return this;
+    }
+
+    public SchedulerExecutorPoolBuilder setMainExecutorThreadName(String mainExecutorThreadName) {
+        this.mainExecutorThreadName = mainExecutorThreadName;
+        return this;
+    }
+
+    public SchedulerExecutorPoolBuilder setAsyncExecutorThreadName(String asyncExecutorThreadName) {
+        this.asyncExecutorThreadName = asyncExecutorThreadName;
+        return this;
+    }
+
+    public SchedulerExecutorPoolImpl build() {
+        if (mainExecutorThreadName == null) {
+            mainExecutorThreadName = "LiteCommands-scheduler-main";
+        }
+
+        if (asyncExecutorThreadName == null) {
+            asyncExecutorThreadName = "LiteCommands-scheduler-async-%d";
+        }
+
+        if (mainExecutor == null) {
+            /*
+             In some cases, the main executor might never be used. We don't want to have to create a useless thread
+
+             This may look like a premature optimization, but a typical server has 40+ plugins
+             As the framework spreads, more and more of them can use LiteCommands,
+             so we better handle this situation, there are practically no losses anyway
+             */
+            ScheduledThreadPoolExecutor mainExecutor = new ScheduledThreadPoolExecutor(
+                1,
+                runnable -> new Thread(runnable, mainExecutorThreadName)
+            );
+            mainExecutor.setKeepAliveTime(1, TimeUnit.HOURS);
+            mainExecutor.allowCoreThreadTimeOut(true);
+
+            this.mainExecutor = mainExecutor;
+            closeMainExecutorOnShutdown = true;
+        }
+
+        if (asyncExecutor == null) {
+            /*
+            We want to create a thread pool that both is fast and uses as few threads as possible.
+            So we use a fixed core pool size to not hold an unreasonable number of threads on server CPUs
+
+            We could also decrease the max cap, but long-running commands are those that use I/O,
+            so it's fine and meaningful to create a bigger pool in this case
+            */
+            AtomicInteger threadCount = new AtomicInteger();
+            ScheduledThreadPoolExecutor asyncExecutor = new ScheduledThreadPoolExecutor(
+                4,
+                runnable -> new Thread(runnable, String.format(asyncExecutorThreadName, threadCount.get()))
+            );
+            asyncExecutor.setKeepAliveTime(3, TimeUnit.MINUTES);
+            asyncExecutor.setMaximumPoolSize(Runtime.getRuntime().availableProcessors() * 2);
+
+            this.asyncExecutor = asyncExecutor;
+            closeAsyncExecutorOnShutdown = true;
+        }
+
+        return new SchedulerExecutorPoolImpl(mainExecutor, closeMainExecutorOnShutdown, asyncExecutor, closeAsyncExecutorOnShutdown);
+    }
+}