From 9ed705ce5ab9b0256dafbf46e768b827920f2e09 Mon Sep 17 00:00:00 2001 From: alanlit Date: Tue, 1 Oct 2024 13:27:13 -0700 Subject: [PATCH] * Fix egregious bug with Actor creation * Actor parameters use reference types and not primitive types (minor optimization) --- build.gradle | 9 +- .../dust/core/actors/Actor.java | 78 +++++++++++++++- .../dust/core/actors/ActorContext.java | 92 ++++++++++++++++++- .../dust/core/actors/ActorRef.java | 2 +- .../lib/PersistentServiceManagerActor.java | 8 +- .../core/actors/lib/ServiceManagerActor.java | 4 +- .../system/CompletionServiceManagerActor.java | 2 +- .../dust/core/system/DeadLetterActor.java | 4 +- .../dust/core/system/SystemActor.java | 4 +- src/test/groovy/TraitTest.groovy | 16 +++- src/test/java/TestTrait.java | 11 --- 11 files changed, 195 insertions(+), 35 deletions(-) delete mode 100644 src/test/java/TestTrait.java diff --git a/build.gradle b/build.gradle index e2f2603..7fe239a 100644 --- a/build.gradle +++ b/build.gradle @@ -6,7 +6,7 @@ plugins { } group 'com.mentalresonance' -version '1.0.1' +version '1.0.2' repositories { mavenLocal() @@ -43,7 +43,7 @@ publishing { } def testArgs = [ - '-Xmx16g', + '-Xmx20g', '--add-opens=java.base/java.lang=ALL-UNNAMED', '--add-opens=java.base/java.math=ALL-UNNAMED', '--add-opens=java.base/java.util=ALL-UNNAMED', @@ -65,8 +65,9 @@ java { } tasks.withType(JavaCompile) { - options.debug = false - options.compilerArgs << '-g:none' // Ensures no debug information is included + options.debug = true + options.debugOptions.debugLevel = 'source,lines,vars' + // options.compilerArgs << '-g:none' // Ensures no debug information is included } javadoc.options { diff --git a/src/main/java/com/mentalresonance/dust/core/actors/Actor.java b/src/main/java/com/mentalresonance/dust/core/actors/Actor.java index 600f5cb..a39dfe4 100644 --- a/src/main/java/com/mentalresonance/dust/core/actors/Actor.java +++ b/src/main/java/com/mentalresonance/dust/core/actors/Actor.java @@ -27,6 +27,7 @@ import java.io.Serializable; import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; import java.util.*; import java.util.concurrent.LinkedBlockingQueue; import static com.mentalresonance.dust.core.actors.SupervisionStrategy.*; @@ -699,8 +700,7 @@ protected ActorRef actorOf(Props props, String name) throws ActorInstantiationEx log.warn("Child %s of %s already exists !!".formatted(name, self.path)); return exists; } - var cons = context.actorConstructors.get(props.actorClass); - Actor actor = (Actor)cons.newInstance(props.actorArgs); + Actor actor = createInstanceWithParameters(props.actorClass, props.actorArgs); ActorRef ref = new ActorRef(self.path, name, context); ref.props = props; @@ -717,11 +717,83 @@ protected ActorRef actorOf(Props props, String name) throws ActorInstantiationEx return actor.self; } catch (Exception e) { - e.printStackTrace(); + log.error("Could not create Actor {} error: {}", props.actorClass, e.getMessage()); throw new ActorInstantiationException(e.getMessage()); } } + Actor createInstanceWithParameters(Class actorClass, Object[] args) throws Exception { + ArrayList> classList = new ArrayList<>(); + classList.add(actorClass); + for (Object obj : args) { + if (obj != null) { + Class clazz = obj.getClass(); + if (clazz.isPrimitive()) { + if (clazz == int.class) { + classList.add(Integer.class); + } else if (clazz == boolean.class) { + classList.add(Boolean.class); + } else if (clazz == byte.class) { + classList.add(Byte.class); + } else if (clazz == char.class) { + classList.add(Character.class); + } else if (clazz == double.class) { + classList.add(Double.class); + } else if (clazz == float.class) { + classList.add(Float.class); + } else if (clazz == long.class) { + classList.add(Long.class); + } else if (clazz == short.class) { + classList.add(Short.class); + } else if (clazz == void.class) { + classList.add(Void.class); + } + } else { + classList.add(clazz); + } + } else { + classList.add(null); + } + } + Constructor cons = context.actorConstructors.get(classList); + return (Actor) cons.newInstance(args); + } + + private List> autoboxPrimitiveTypes(List> classList) { + List> autoboxedList = new ArrayList<>(); + + for (Class clazz : classList) { + // Check if the class is a primitive type, and if so, add the corresponding wrapper class + if (clazz.isPrimitive()) { + if (clazz == int.class) { + autoboxedList.add(Integer.class); + } else if (clazz == boolean.class) { + autoboxedList.add(Boolean.class); + } else if (clazz == byte.class) { + autoboxedList.add(Byte.class); + } else if (clazz == char.class) { + autoboxedList.add(Character.class); + } else if (clazz == double.class) { + autoboxedList.add(Double.class); + } else if (clazz == float.class) { + autoboxedList.add(Float.class); + } else if (clazz == long.class) { + autoboxedList.add(Long.class); + } else if (clazz == short.class) { + autoboxedList.add(Short.class); + } else if (clazz == void.class) { + autoboxedList.add(Void.class); + } + } else { + // If it's not a primitive, just add the class as-is + autoboxedList.add(clazz); + } + } + + return autoboxedList; + } + + /** * Convert a path into an ActorRef. Path can be of the form .. * /.... in which case is absolute diff --git a/src/main/java/com/mentalresonance/dust/core/actors/ActorContext.java b/src/main/java/com/mentalresonance/dust/core/actors/ActorContext.java index df8a733..4836d4b 100644 --- a/src/main/java/com/mentalresonance/dust/core/actors/ActorContext.java +++ b/src/main/java/com/mentalresonance/dust/core/actors/ActorContext.java @@ -72,14 +72,100 @@ public ActorContext(ActorSystem system) { this.system = system; } - // Actor class -> Constructor for the Actor /** * Cache of Actor constructors. This is self loading and will get constructors on demand. Purely for * speed. + * Key List is [ActorClass, Arg1Class, Arg2Class ..... ] + * Value is the constructor matching this signature */ - public final LoadingCache actorConstructors = Caffeine.newBuilder() + public final LoadingCache>, Constructor> actorConstructors = Caffeine.newBuilder() .maximumSize(4096) - .build(k -> k.getConstructors()[0]); + .build(k -> constructorFor(k)); + + /** + * Appropriate constructor was not cached, so take list of [ActorClass, Arg1Class, Arg2Class..] and + * find the appropriate constructor. + * @param classAndArgs List of (already boxed) classes defining the call + * @return appropriate constructor + * @throws Exception if we cannot do this + */ + private Constructor constructorFor(List> classAndArgs) throws Exception { + try { + List> constructors = Arrays.stream(classAndArgs.get(0).getConstructors()).toList(); + List> argClasses = classAndArgs.size() > 1 ? classAndArgs.subList(1, classAndArgs.size()) : List.of(); + List matchingConstructors = new ArrayList<>(); + + for (Constructor cons : constructors ) { + List> parameterTypes = autoboxPrimitiveTypes(cons.getParameterTypes()); + if (areParametersMatching(parameterTypes, argClasses)) { + return cons; + } + } + + throw new Exception("No constructor found for " + classAndArgs.get(0)); + } + catch (Exception e) { + e.printStackTrace(); + throw e; + } + } + + /** + * Do parameter types in constructor match the classes of the arguments + * @param parameterTypes constructor types + * @param argsClasses argument types + * @return true or false + */ + private static boolean areParametersMatching(List> parameterTypes, List> argsClasses) { + if (parameterTypes.size() != argsClasses.size()) { + return false; + } + for (int i = 0; i < parameterTypes.size(); i++) { + Class clazz = argsClasses.get(i); + if (clazz != null && !(parameterTypes.get(i).isAssignableFrom(clazz))) { + return false; + } + } + return true; + } + + /** + * Primitive types (!) cannot be checked by isAssignableFrom, so we box them here if necessary + * @param classList array of types - possible some primitive + * @return list of types - boxed where necessary + */ + private List> autoboxPrimitiveTypes(Class[] classList) { + List> autoboxedList = new ArrayList<>(); + + for (Class clazz : classList) { + if (clazz.isPrimitive()) { + if (clazz == int.class) { + autoboxedList.add(Integer.class); + } else if (clazz == boolean.class) { + autoboxedList.add(Boolean.class); + } else if (clazz == byte.class) { + autoboxedList.add(Byte.class); + } else if (clazz == char.class) { + autoboxedList.add(Character.class); + } else if (clazz == double.class) { + autoboxedList.add(Double.class); + } else if (clazz == float.class) { + autoboxedList.add(Float.class); + } else if (clazz == long.class) { + autoboxedList.add(Long.class); + } else if (clazz == short.class) { + autoboxedList.add(Short.class); + } else if (clazz == void.class) { + autoboxedList.add(Void.class); + } + } else { + // If it's not a primitive, just add the class as-is + autoboxedList.add(clazz); + } + } + + return autoboxedList; + } /** * Resolve a path to an ActorRef. The path must be rooted at '/' diff --git a/src/main/java/com/mentalresonance/dust/core/actors/ActorRef.java b/src/main/java/com/mentalresonance/dust/core/actors/ActorRef.java index 925e933..5db0ae9 100644 --- a/src/main/java/com/mentalresonance/dust/core/actors/ActorRef.java +++ b/src/main/java/com/mentalresonance/dust/core/actors/ActorRef.java @@ -260,7 +260,7 @@ public void waitForDeath() throws Exception { @Override public String toString() { - return (null == mailBox) ? path + " [Remote]" : (null != props ? props.actorClass.getSimpleName() : "NULL") + ": " + path + " @" + Integer.toHexString(this.hashCode()) +" [Local]"; + return path; } /** diff --git a/src/main/java/com/mentalresonance/dust/core/actors/lib/PersistentServiceManagerActor.java b/src/main/java/com/mentalresonance/dust/core/actors/lib/PersistentServiceManagerActor.java index 4d43258..78e02d6 100644 --- a/src/main/java/com/mentalresonance/dust/core/actors/lib/PersistentServiceManagerActor.java +++ b/src/main/java/com/mentalresonance/dust/core/actors/lib/PersistentServiceManagerActor.java @@ -55,7 +55,7 @@ public abstract class PersistentServiceManagerActor extends PersistentActor { * @param maxWorkers - max number of workers * @return Props */ - public static Props props(Props serviceProps, int maxWorkers) { + public static Props props(Props serviceProps, Integer maxWorkers) { return Props.create(PersistentServiceManagerActor.class, serviceProps, maxWorkers, 5000L); } @@ -66,7 +66,7 @@ public static Props props(Props serviceProps, int maxWorkers) { * @param delayMS - interval in ms between snapshot saves * @return Props */ - public static Props props(Props serviceProps, int maxWorkers, Long delayMS) { + public static Props props(Props serviceProps, Integer maxWorkers, Long delayMS) { return Props.create(PersistentServiceManagerActor.class, serviceProps, maxWorkers, delayMS); } @@ -76,7 +76,7 @@ public static Props props(Props serviceProps, int maxWorkers, Long delayMS) { * @param maxWorkers - max number of workers * @param delayMS - interval in ms between snapshot saves */ - public PersistentServiceManagerActor(Props serviceProps, int maxWorkers, Long delayMS) { + public PersistentServiceManagerActor(Props serviceProps, Integer maxWorkers, Long delayMS) { this.serviceProps = serviceProps; this.maxWorkers = maxWorkers; this.delayMS = delayMS; @@ -89,7 +89,7 @@ public PersistentServiceManagerActor(Props serviceProps, int maxWorkers, Long de * @param serviceProps - Props for the service workers * @param maxWorkers - max number of workers */ - public PersistentServiceManagerActor(Props serviceProps, int maxWorkers) { + public PersistentServiceManagerActor(Props serviceProps, Integer maxWorkers) { this(serviceProps, maxWorkers, 5000L); } diff --git a/src/main/java/com/mentalresonance/dust/core/actors/lib/ServiceManagerActor.java b/src/main/java/com/mentalresonance/dust/core/actors/lib/ServiceManagerActor.java index 29ef170..1e03f6c 100644 --- a/src/main/java/com/mentalresonance/dust/core/actors/lib/ServiceManagerActor.java +++ b/src/main/java/com/mentalresonance/dust/core/actors/lib/ServiceManagerActor.java @@ -55,7 +55,7 @@ public class ServiceManagerActor extends Actor { * @param maxWorkers max slots available * @return Props */ - public static Props props(Props serviceProps, int maxWorkers) { + public static Props props(Props serviceProps, Integer maxWorkers) { return Props.create(ServiceManagerActor.class, serviceProps, maxWorkers); } @@ -64,7 +64,7 @@ public static Props props(Props serviceProps, int maxWorkers) { * @param serviceProps the ServiceActor props * @param maxWorkers max slots available */ - public ServiceManagerActor(Props serviceProps, int maxWorkers) { + public ServiceManagerActor(Props serviceProps, Integer maxWorkers) { this.serviceProps = serviceProps; this.maxWorkers = maxWorkers; currentWorkers = 0; diff --git a/src/main/java/com/mentalresonance/dust/core/system/CompletionServiceManagerActor.java b/src/main/java/com/mentalresonance/dust/core/system/CompletionServiceManagerActor.java index 5dc1f6b..4ca78ef 100644 --- a/src/main/java/com/mentalresonance/dust/core/system/CompletionServiceManagerActor.java +++ b/src/main/java/com/mentalresonance/dust/core/system/CompletionServiceManagerActor.java @@ -33,7 +33,7 @@ public class CompletionServiceManagerActor extends ServiceManagerActor { * Constructor * @param maxWorkers max workers for this service */ - public CompletionServiceManagerActor(int maxWorkers) { + public CompletionServiceManagerActor(Integer maxWorkers) { super(CompletionServiceActor.props(), maxWorkers); } } diff --git a/src/main/java/com/mentalresonance/dust/core/system/DeadLetterActor.java b/src/main/java/com/mentalresonance/dust/core/system/DeadLetterActor.java index 6763a16..a6365f6 100644 --- a/src/main/java/com/mentalresonance/dust/core/system/DeadLetterActor.java +++ b/src/main/java/com/mentalresonance/dust/core/system/DeadLetterActor.java @@ -50,7 +50,7 @@ public class DeadLetterActor extends PubSubActor { * @param logDeadLetters if true log dead letters * @return Props */ - public static Props props(boolean logDeadLetters) { + public static Props props(Boolean logDeadLetters) { return Props.create(DeadLetterActor.class, logDeadLetters); } @@ -58,7 +58,7 @@ public static Props props(boolean logDeadLetters) { * Constructor * @param logDeadLetters if true log dead letters */ - public DeadLetterActor(boolean logDeadLetters) { + public DeadLetterActor(Boolean logDeadLetters) { this.logDeadLetters = logDeadLetters; } diff --git a/src/main/java/com/mentalresonance/dust/core/system/SystemActor.java b/src/main/java/com/mentalresonance/dust/core/system/SystemActor.java index fc04f6a..b12b7bb 100644 --- a/src/main/java/com/mentalresonance/dust/core/system/SystemActor.java +++ b/src/main/java/com/mentalresonance/dust/core/system/SystemActor.java @@ -49,7 +49,7 @@ public class SystemActor extends Actor { * @param logDeadLetters if true dead letters are to be logged * @return Props */ - public static Props props(boolean logDeadLetters) { + public static Props props(Boolean logDeadLetters) { return Props.create(SystemActor.class, logDeadLetters); } @@ -57,7 +57,7 @@ public static Props props(boolean logDeadLetters) { * Constructor * @param logDeadLetters if true dead letters are to be logged */ - public SystemActor(boolean logDeadLetters) { + public SystemActor(Boolean logDeadLetters) { this.logDeadLetters = logDeadLetters; } diff --git a/src/test/groovy/TraitTest.groovy b/src/test/groovy/TraitTest.groovy index fb3f856..cb7e10f 100644 --- a/src/test/groovy/TraitTest.groovy +++ b/src/test/groovy/TraitTest.groovy @@ -1,4 +1,5 @@ import com.mentalresonance.dust.core.actors.Actor +import com.mentalresonance.dust.core.actors.ActorTrait import com.mentalresonance.dust.core.actors.Props import com.mentalresonance.dust.core.actors.ActorSystem import groovy.util.logging.Slf4j @@ -10,6 +11,8 @@ import spock.lang.Specification @Slf4j class TraitTest extends Specification { + static boolean called = false + @Slf4j static class Supervisor extends Actor implements TestTrait { @@ -23,13 +26,22 @@ class TraitTest extends Specification { } } + static interface TestTrait extends ActorTrait { + + default void callMe() { + System.out.println(String.format("MyActorRef - %s - MyActor %s", getSelf(), this)) + called = true + } + } + def "Trait Test"() { when: ActorSystem system = new ActorSystem("TraitTest") system.context.actorOf( Supervisor.props(), "trait") + Thread.sleep(500) system.stop() - then: - true + then: + called } } diff --git a/src/test/java/TestTrait.java b/src/test/java/TestTrait.java deleted file mode 100644 index 5758216..0000000 --- a/src/test/java/TestTrait.java +++ /dev/null @@ -1,11 +0,0 @@ -import com.mentalresonance.dust.core.actors.ActorTrait; - -/** - * Adds a simple method via the interface/trait that the implementing Actor will call. Part of tests - */ -public interface TestTrait extends ActorTrait { - - default void callMe() { - System.out.println(String.format("MyActorRef - %s - MyActor %s", getSelf(), this)); - } -}