GH-355 Enhance LiteCommands execution and argument resolving performa…

…nce. (#355)

* Enhance LiteCommands execution and argument resolving performance.

* Fix after cleanup

* Fix unit test.

* Add better delay and margin for CI.

* Add cache for argument key and use class implementation instead lambda.

* Remove jmh

* Tests and then build CI

* Fix scheduled chain.

* Remove invalid assertions.

.github/workflows/build.yml

@@ -27,7 +27,7 @@ jobs:
         run: chmod +x gradlew
 
       - name: Build with Gradle
-        run: ./gradlew clean build
+        run: ./gradlew clean test clean build
         env:
           GIT_BRANCH: ${{ github.ref }}
           GIT_COMMIT: ${{ github.sha }}

buildSrc/build.gradle.kts

@@ -7,7 +7,6 @@ repositories {
 }
 
 dependencies {
-    implementation("me.champeau.jmh:jmh-gradle-plugin:0.7.2")
     implementation("net.kyori:indra-git:3.1.3")
     implementation("com.google.guava:guava:30.1.1-jre")
 }

litecommands-annotations/build.gradle.kts

@@ -4,12 +4,10 @@ plugins {
     `litecommands-java-unit-test`
     `litecommands-repositories`
     `litecommands-publish`
-    `litecommands-java-benchmark`
 }
 
 dependencies {
     api(project(":litecommands-core"))
-    jmhImplementation(project(":litecommands-unit"))
 }
 
 litecommandsUnit {

litecommands-annotations/src/dev/rollczi/litecommands/annotations/MethodCommandExecutor.java

@@ -18,10 +18,12 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
+import java.util.function.Supplier;
 
 class MethodCommandExecutor<SENDER> extends AbstractCommandExecutor<SENDER> {
 
     private final Method method;
+    private final Parameter[] parameters;
     private final Object instance;
     private final MethodDefinition definition;
     private final MethodValidatorService<SENDER> validatorService;
@@ -37,6 +39,7 @@ class MethodCommandExecutor<SENDER> extends AbstractCommandExecutor<SENDER> {
         super(parent, definition.getArguments(), definition.getContextRequirements(), definition.getBindRequirements());
         this.method = method;
         this.method.setAccessible(true);
+        this.parameters = method.getParameters();
         this.instance = instance;
         this.definition = definition;
         this.validatorService = validatorService;
@@ -47,17 +50,18 @@ class MethodCommandExecutor<SENDER> extends AbstractCommandExecutor<SENDER> {
     public CommandExecutorMatchResult match(RequirementsResult<SENDER> results) {
         Object[] objects = new Object[method.getParameterCount()];
 
-        for (int parameterIndex = 0; parameterIndex < method.getParameterCount(); parameterIndex++) {
-            Requirement<?> requirement = definition.getRequirement(parameterIndex);
+        int parameterIndex = 0;
+        for (Requirement<?> requirement : definition.getRequirements()) {
             String name = requirement.getName();
 
-            if (!results.has(name)) {
+            RequirementMatch requirementMatch = results.get(name);
+
+            if (requirementMatch == null) {
                 return CommandExecutorMatchResult.failed(new IllegalStateException("Not all parameters are resolved, missing " + name));
             }
 
-            RequirementMatch<?, ?> requirementMatch = results.get(name);
             Object unwrapped = requirementMatch.getResult().unwrap();
-            Parameter parameter = method.getParameters()[parameterIndex];
+            Parameter parameter = parameters[parameterIndex];
             Class<?> type = parameter.getType();
 
             if (unwrapped == null && type.isPrimitive()) {
@@ -71,6 +75,7 @@ public CommandExecutorMatchResult match(RequirementsResult<SENDER> results) {
             }
 
             objects[parameterIndex] = unwrapped;
+            parameterIndex++;
         }
 
         ValidatorResult result = validatorService.validate(new MethodValidatorContext<>(results, definition, instance, method, objects));
@@ -79,23 +84,31 @@ public CommandExecutorMatchResult match(RequirementsResult<SENDER> results) {
             return CommandExecutorMatchResult.failed(result.getInvalidResult());
         }
 
-        return CommandExecutorMatchResult.success(() -> {
+        return CommandExecutorMatchResult.success(new FinalCommandExecutor(objects));
+    }
+
+    private class FinalCommandExecutor implements Supplier<CommandExecuteResult> {
+        private final Object[] objects;
+
+        public FinalCommandExecutor(Object[] objects) {
+            this.objects = objects;
+        }
+
+        @Override
+        public CommandExecuteResult get() {
             try {
-                return CommandExecuteResult.success(this, this.method.invoke(this.instance, objects));
-            }
-            catch (IllegalAccessException exception) {
-                throw new LiteCommandsReflectInvocationException(this.method, "Cannot access method", exception);
-            }
-            catch (InvocationTargetException exception) {
+                return CommandExecuteResult.success(MethodCommandExecutor.this, MethodCommandExecutor.this.method.invoke(MethodCommandExecutor.this.instance, objects));
+            } catch (IllegalAccessException exception) {
+                throw new LiteCommandsReflectInvocationException(MethodCommandExecutor.this.method, "Cannot access method", exception);
+            } catch (InvocationTargetException exception) {
                 Throwable targetException = exception.getTargetException();
 
                 if (targetException instanceof InvalidUsageException) { //TODO: Use invalid usage handler (when InvalidUsage.Cause is mapped to InvalidUsage)
-                    return CommandExecuteResult.failed(this, ((InvalidUsageException) targetException).getErrorResult());
+                    return CommandExecuteResult.failed(MethodCommandExecutor.this, ((InvalidUsageException) targetException).getErrorResult());
                 }
 
-                throw new LiteCommandsReflectInvocationException(this.method, "Command method threw " + targetException.getClass().getSimpleName(), targetException);
+                throw new LiteCommandsReflectInvocationException(MethodCommandExecutor.this.method, "Command method threw " + targetException.getClass().getSimpleName(), targetException);
             }
-        });
+        }
     }
-
 }

litecommands-annotations/src/dev/rollczi/litecommands/annotations/MethodDefinition.java

@@ -9,8 +9,10 @@
 
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 public class MethodDefinition {
@@ -20,11 +22,32 @@ public class MethodDefinition {
     private final Map<Integer, ContextRequirement<?>> contextRequirements = new HashMap<>();
     private final Map<Integer, BindRequirement<?>> bindRequirements = new HashMap<>();
 
+    private List<Requirement<?>> indexedRequirements;
+
     MethodDefinition(Method method) {
         this.method = method;
     }
 
     public Requirement<?> getRequirement(int parameterIndex) {
+        return getRequirements().get(parameterIndex);
+    }
+
+    public List<Requirement<?>> getRequirements() {
+        if (indexedRequirements == null) {
+            List<Requirement<?>> indexedRequirements = new ArrayList<>();
+
+            for (int i = 0; i < method.getParameterCount(); i++) {
+                indexedRequirements.add(getRequirement0(i));
+            }
+
+            this.indexedRequirements = indexedRequirements;
+            return indexedRequirements;
+        }
+
+        return indexedRequirements;
+    }
+
+    private Requirement<?> getRequirement0(int parameterIndex) {
         Argument<?> argument = arguments.get(parameterIndex);
 
         if (argument != null) {

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

@@ -14,6 +14,7 @@
 import dev.rollczi.litecommands.scheduler.SchedulerExecutorPoolImpl;
 import dev.rollczi.litecommands.unit.AssertExecute;
 import dev.rollczi.litecommands.unit.TestSender;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 import org.junit.jupiter.api.Test;
 
 import java.util.Date;
@@ -24,11 +25,14 @@
 
 class AsyncCommandTest extends LiteTestSpec {
 
+    private static final int DELAY = 500;
+    private static final int MARGIN = 200;
+
     static LiteConfig config = builder -> builder
         .scheduler(new SchedulerExecutorPoolImpl("test", 1))
         .context(Date.class, invocation -> ContextResult.ok(() -> {
             try {
-                Thread.sleep(800);
+                Thread.sleep(DELAY);
             } catch (InterruptedException e) {
                 throw new RuntimeException(e);
             }
@@ -104,7 +108,8 @@ void testAsyncArgs() {
         CompletableFuture<AssertExecute> result = platform.executeAsync("test async-args first second");
 
         await()
-            .atLeast(300, TimeUnit.MILLISECONDS)
+            .atLeast(DELAY - MARGIN, TimeUnit.MILLISECONDS)
+            .atMost(DELAY + MARGIN, TimeUnit.MILLISECONDS)
             .until(() -> result.isDone());
 
         result.join()
@@ -116,7 +121,8 @@ void testAsyncArgsAndMethod() {
         CompletableFuture<AssertExecute> result = platform.executeAsync("test async-args-and-method first second");
 
         await()
-            .atLeast(300, TimeUnit.MILLISECONDS)
+            .atLeast(DELAY - MARGIN, TimeUnit.MILLISECONDS)
+            .atMost(DELAY + MARGIN, TimeUnit.MILLISECONDS)
             .until(() -> result.isDone());
 
         result.join()

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

@@ -0,0 +1,79 @@
+package dev.rollczi.litecommands.annotations.async;
+
+import dev.rollczi.litecommands.annotations.LiteConfig;
+import dev.rollczi.litecommands.annotations.LiteTestSpec;
+import dev.rollczi.litecommands.annotations.argument.Arg;
+import dev.rollczi.litecommands.annotations.command.Command;
+import dev.rollczi.litecommands.annotations.context.Context;
+import dev.rollczi.litecommands.annotations.execute.Execute;
+import dev.rollczi.litecommands.argument.Argument;
+import dev.rollczi.litecommands.argument.parser.ParseResult;
+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.unit.AssertExecute;
+import dev.rollczi.litecommands.unit.TestSender;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import org.junit.jupiter.api.Test;
+
+public class ParallelAsyncCommandTest extends LiteTestSpec {
+
+    private static final int DELAY = 100;
+
+    static LiteConfig config = builder -> builder
+        .scheduler(new SchedulerExecutorPoolImpl("test", 50))
+        .context(Date.class, invocation -> ContextResult.ok(() -> {
+            try {
+                Thread.sleep(DELAY);
+            } catch (InterruptedException e) {
+                throw new RuntimeException(e);
+            }
+
+            return new Date();
+        }))
+        .argumentParser(String.class, new StringArgumentResolver());
+
+    static class StringArgumentResolver extends ArgumentResolver<TestSender, String> {
+        @Override
+        protected ParseResult<String> parse(Invocation<TestSender> invocation, Argument<String> context, String argument) {
+            try {
+                Thread.sleep(DELAY);
+            } catch (InterruptedException e) {
+                throw new RuntimeException(e);
+            }
+
+            return ParseResult.success(argument);
+        }
+    }
+
+    @Command(name = "async")
+    static class TestCommand {
+
+        @Async
+        @Execute
+        public String testAsyncArgs2(@Async @Context Date date, @Async @Arg String first, @Async @Arg String second) {
+            return first + " " + second;
+        }
+
+    }
+
+    @Test
+    void testAsyncArgsAndMethod() {
+        List<CompletableFuture<AssertExecute>> results = new ArrayList<>();
+
+        for (int i = 0; i < 500; i++) {
+            results.add(platform.executeAsync("async first second"));
+        }
+
+        for (CompletableFuture<AssertExecute> result : results) {
+            result.join()
+                .assertSuccess("first second");
+        }
+    }
+
+
+}

litecommands-chatgpt/src/dev/rollczi/litecommands/chatgpt/ChatGptStringSuggester.java

@@ -7,7 +7,6 @@
 import dev.rollczi.litecommands.argument.suggester.Suggester;
 import dev.rollczi.litecommands.identifier.Identifier;
 import dev.rollczi.litecommands.input.raw.RawCommand;
-import dev.rollczi.litecommands.input.raw.RawInput;
 import dev.rollczi.litecommands.invocation.Invocation;
 import dev.rollczi.litecommands.join.JoinStringArgumentResolver;
 import dev.rollczi.litecommands.scheduler.Scheduler;
@@ -16,7 +15,6 @@
 import dev.rollczi.litecommands.suggestion.SuggestionContext;
 import dev.rollczi.litecommands.suggestion.SuggestionResult;
 
-import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.Collection;
 import java.util.HashMap;
@@ -29,9 +27,8 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
-import java.util.logging.Logger;
 
-class ChatGptStringSuggester<SENDER> extends JoinStringArgumentResolver<SENDER> implements Suggester<SENDER, String>, ArgumentResolverBase<SENDER, RawInput, String> {
+class ChatGptStringSuggester<SENDER> extends JoinStringArgumentResolver<SENDER> implements Suggester<SENDER, String>, ArgumentResolverBase<SENDER, String> {
 
     private final Scheduler scheduler;
     private final ChatGptClient chatGptClient;