From 89f47fdd8a3e93ca72dcb61342540be9291bacd6 Mon Sep 17 00:00:00 2001 From: nathan Date: Sun, 7 Feb 2016 00:10:42 +0100 Subject: [PATCH] Code polish/cleanup --- src/dorkbox/util/messagebus/IMessageBus.java | 2 +- .../util/messagebus/PubSubSupport.java | 2 +- .../util/messagebus/common/ClassTree.java | 22 ++------------ src/dorkbox/util/messagebus/common/Entry.java | 2 +- .../messagebus/common/MessageHandler.java | 5 +++- .../messagebus/common/NamedThreadFactory.java | 2 +- .../error/IPublicationErrorHandler.java | 13 ++++----- .../messagebus/subscription/Subscription.java | 20 +++++-------- .../subscription/SubscriptionManager.java | 24 ++------------- .../subscription/asm/AsmInvocation.java | 2 +- .../asm/AsmSynchronizedInvocation.java | 2 +- .../subscription/asm/SubscriptionAsm.java | 9 ------ .../reflection/ReflectionInvocation.java | 2 +- .../ReflectionSynchronizedInvocation.java | 2 +- .../reflection/SubscriptionReflection.java | 9 ------ .../util/messagebus/synchrony/AsyncABQ.java | 2 +- .../messagebus/synchrony/AsyncABQ_noGc.java | 2 +- .../messagebus/synchrony/AsyncDisruptor.java | 8 ++--- .../messagebus/synchrony/MessageHolder.java | 25 ---------------- .../util/messagebus/utils/ClassUtils.java | 17 +++++++---- .../messagebus/utils/ReflectionUtils.java | 29 +++++-------------- .../util/messagebus/MultiTreeTest.java | 4 --- 22 files changed, 58 insertions(+), 147 deletions(-) diff --git a/src/dorkbox/util/messagebus/IMessageBus.java b/src/dorkbox/util/messagebus/IMessageBus.java index a253e0e..8121ff4 100644 --- a/src/dorkbox/util/messagebus/IMessageBus.java +++ b/src/dorkbox/util/messagebus/IMessageBus.java @@ -93,7 +93,7 @@ import dorkbox.util.messagebus.error.ErrorHandlingSupport; * NOTE: Generic type parameters of messages will not be taken into account, e.g. a List will * publish dispatched to all message handlers that take an instance of List as their parameter * - * @Author bennidi + * @author bennidi * Date: 2/8/12 * @author dorkbox, llc * Date: 2/2/15 diff --git a/src/dorkbox/util/messagebus/PubSubSupport.java b/src/dorkbox/util/messagebus/PubSubSupport.java index 36bb93c..0c30523 100644 --- a/src/dorkbox/util/messagebus/PubSubSupport.java +++ b/src/dorkbox/util/messagebus/PubSubSupport.java @@ -43,7 +43,7 @@ package dorkbox.util.messagebus; * Listeners can be subscribed and unsubscribed using the corresponding methods. When a listener is subscribed its * handlers will be registered and start to receive matching message publications. * - * @Author bennidi + * @author bennidi * @author dorkbox, llc * Date: 2/2/15 */ diff --git a/src/dorkbox/util/messagebus/common/ClassTree.java b/src/dorkbox/util/messagebus/common/ClassTree.java index 966e7f5..35c5941 100644 --- a/src/dorkbox/util/messagebus/common/ClassTree.java +++ b/src/dorkbox/util/messagebus/common/ClassTree.java @@ -56,7 +56,8 @@ public class ClassTree { @SuppressWarnings("unchecked") - public static IdentityMap> cast(Object o) { + private static + IdentityMap> cast(Object o) { return (IdentityMap>) o; } @@ -109,28 +110,11 @@ public class ClassTree { return getOrCreateValue(leaf); } - public final - MultiClass get(KEY... keys) { - if (keys == null) { - throw new NullPointerException("keys"); - } - - int length = keys.length; - - // have to put value into our children - ClassTree leaf = getOrCreateLeaf(keys[0]); - for (int i=1;i messageClass) { @@ -99,7 +102,7 @@ class MessageHandler { } } - return finalMethods.toArray(new MessageHandler[0]); + return finalMethods.toArray(EMPTY_MESSAGEHANDLERS); } private final Method method; diff --git a/src/dorkbox/util/messagebus/common/NamedThreadFactory.java b/src/dorkbox/util/messagebus/common/NamedThreadFactory.java index 1aad502..8c8e1d3 100644 --- a/src/dorkbox/util/messagebus/common/NamedThreadFactory.java +++ b/src/dorkbox/util/messagebus/common/NamedThreadFactory.java @@ -101,7 +101,7 @@ class NamedThreadFactory implements ThreadFactory { return newThread(stringBuilder.toString(), r); } - public + private Thread newThread(String name, Runnable r) { // stack size is arbitrary based on JVM implementation. Default is 0 // 8k is the size of the android stack. Depending on the version of android, this can either change, or will always be 8k diff --git a/src/dorkbox/util/messagebus/error/IPublicationErrorHandler.java b/src/dorkbox/util/messagebus/error/IPublicationErrorHandler.java index b70ad1e..9167853 100644 --- a/src/dorkbox/util/messagebus/error/IPublicationErrorHandler.java +++ b/src/dorkbox/util/messagebus/error/IPublicationErrorHandler.java @@ -23,10 +23,10 @@ package dorkbox.util.messagebus.error; /** - * Publication error handlers are provided with a publication error every time an - * error occurs during message publication. - * A handler might fail with an exception, not be accessible because of the presence - * of a security manager or other reasons might lead to failures during the message publication process. + * Publication error handlers are provided with a publication error every time an error occurs during message publication. + *

+ * A handler might fail with an exception, not be accessible because of the presence of a security manager or other reasons might + * lead to failures during the message publication process. *

* * @author bennidi @@ -46,14 +46,13 @@ interface IPublicationErrorHandler { * Handle the given publication error. * * @param error The PublicationError to handle. - * @param listenerClass + * @param listenerClass The class that caused the error to occur */ void handleError(String error, final Class listenerClass); /** - * The default error handler will simply log to standard out and - * print the stack trace if available. + * The default error handler will simply log to standard out and print the stack trace if available. */ final class ConsoleLogger implements IPublicationErrorHandler { diff --git a/src/dorkbox/util/messagebus/subscription/Subscription.java b/src/dorkbox/util/messagebus/subscription/Subscription.java index 3e8970b..98b68ce 100644 --- a/src/dorkbox/util/messagebus/subscription/Subscription.java +++ b/src/dorkbox/util/messagebus/subscription/Subscription.java @@ -39,7 +39,7 @@ import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; public abstract class Subscription { private static final AtomicInteger ID_COUNTER = new AtomicInteger(); - public final int ID = ID_COUNTER.getAndIncrement(); + private final int ID = ID_COUNTER.getAndIncrement(); // this is the listener class that created this subscription @@ -58,7 +58,8 @@ class Subscription { private final IdentityMap entries; // maintain a map of entries for FAST lookup during unsubscribe. // this is still inside the single-writer, and can use the same techniques as subscription manager (for thread safe publication) - public volatile Entry head; // reference to the first element + // accessed from within SYNCHRONIZE + private Entry head; // reference to the first element protected Subscription(final Class listenerClass, final MessageHandler handler) { @@ -90,6 +91,7 @@ class Subscription { public final void subscribe(final Object listener) { // single writer principle! + // called from within SYNCHRONIZE Entry head = headREF.get(this); if (!entries.containsKey(listener)) { @@ -100,18 +102,13 @@ class Subscription { } } - /** - * @return TRUE if the element was removed - */ public final - boolean unsubscribe(final Object listener) { + void unsubscribe(final Object listener) { Entry entry = entries.get(listener); - if (entry == null || entry.getValue() == null) { - // fast exit - return false; - } - else { + + if (entry != null && entry.getValue() != null) { // single writer principle! + // called from within SYNCHRONIZE Entry head = headREF.get(this); if (entry != head) { @@ -125,7 +122,6 @@ class Subscription { this.entries.remove(listener); headREF.lazySet(this, head); - return true; } } diff --git a/src/dorkbox/util/messagebus/subscription/SubscriptionManager.java b/src/dorkbox/util/messagebus/subscription/SubscriptionManager.java index 53b8ad9..bcc9321 100644 --- a/src/dorkbox/util/messagebus/subscription/SubscriptionManager.java +++ b/src/dorkbox/util/messagebus/subscription/SubscriptionManager.java @@ -43,7 +43,7 @@ import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; public final class SubscriptionManager { public static final float LOAD_FACTOR = 0.8F; - public static final Subscription[] EMPTY_SUBS = new Subscription[0]; + private static final Subscription[] EMPTY_SUBS = new Subscription[0]; // controls if we use java reflection or ASM to access methods during publication private final SubMaker subMaker; @@ -112,7 +112,7 @@ class SubscriptionManager { this.subMaker = new SubMakerReflection(); } - classUtils = new ClassUtils(SubscriptionManager.LOAD_FACTOR); + classUtils = new ClassUtils(); classTree = new ClassTree>(); @@ -321,25 +321,7 @@ class SubscriptionManager { } default: { - multiClass = classTree.get(messageHandlerTypes); - - // makes this subscription visible for publication - final Subscription[] newSubs; - Subscription[] currentSubs = multiSubs.get(multiClass); - - if (currentSubs != null) { - final int currentLength = currentSubs.length; - - // add the new subscription to the array - newSubs = Arrays.copyOf(currentSubs, currentLength + 1, Subscription[].class); - newSubs[currentLength] = subscription; - } else { - newSubs = new Subscription[1]; - newSubs[0] = subscription; - } - - multiSubs.put(multiClass, newSubs); - break; + throw new RuntimeException("Unsupported number of parameters during subscribe. Acceptable max is 3"); } } } diff --git a/src/dorkbox/util/messagebus/subscription/asm/AsmInvocation.java b/src/dorkbox/util/messagebus/subscription/asm/AsmInvocation.java index f69f604..dde3b0f 100644 --- a/src/dorkbox/util/messagebus/subscription/asm/AsmInvocation.java +++ b/src/dorkbox/util/messagebus/subscription/asm/AsmInvocation.java @@ -54,7 +54,7 @@ import com.esotericsoftware.reflectasm.MethodAccess; * @author dorkbox, llc * Date: 2/2/15 */ -public interface AsmInvocation { +interface AsmInvocation { /** * Invoke the message delivery logic of this handler diff --git a/src/dorkbox/util/messagebus/subscription/asm/AsmSynchronizedInvocation.java b/src/dorkbox/util/messagebus/subscription/asm/AsmSynchronizedInvocation.java index 82e59c5..59b2e15 100644 --- a/src/dorkbox/util/messagebus/subscription/asm/AsmSynchronizedInvocation.java +++ b/src/dorkbox/util/messagebus/subscription/asm/AsmSynchronizedInvocation.java @@ -50,7 +50,7 @@ import com.esotericsoftware.reflectasm.MethodAccess; public class AsmSynchronizedInvocation implements AsmInvocation { - private AsmInvocation delegate; + private final AsmInvocation delegate; public AsmSynchronizedInvocation(AsmInvocation delegate) { diff --git a/src/dorkbox/util/messagebus/subscription/asm/SubscriptionAsm.java b/src/dorkbox/util/messagebus/subscription/asm/SubscriptionAsm.java index 81ad34c..b750d72 100644 --- a/src/dorkbox/util/messagebus/subscription/asm/SubscriptionAsm.java +++ b/src/dorkbox/util/messagebus/subscription/asm/SubscriptionAsm.java @@ -86,9 +86,6 @@ class SubscriptionAsm extends Subscription { } - /** - * @return true if messages were published - */ public void publish(final Object message) throws Throwable { final MethodAccess handler = this.handlerAccess; @@ -105,9 +102,6 @@ class SubscriptionAsm extends Subscription { } } - /** - * @return true if messages were published - */ public void publish(final Object message1, final Object message2) throws Throwable { final MethodAccess handler = this.handlerAccess; @@ -124,9 +118,6 @@ class SubscriptionAsm extends Subscription { } } - /** - * @return true if messages were published - */ public void publish(final Object message1, final Object message2, final Object message3) throws Throwable { final MethodAccess handler = this.handlerAccess; diff --git a/src/dorkbox/util/messagebus/subscription/reflection/ReflectionInvocation.java b/src/dorkbox/util/messagebus/subscription/reflection/ReflectionInvocation.java index 08553e6..1280c06 100644 --- a/src/dorkbox/util/messagebus/subscription/reflection/ReflectionInvocation.java +++ b/src/dorkbox/util/messagebus/subscription/reflection/ReflectionInvocation.java @@ -54,7 +54,7 @@ import java.lang.reflect.Method; * @author dorkbox, llc * Date: 2/2/15 */ -public interface ReflectionInvocation { +interface ReflectionInvocation { /** * Invoke the message delivery logic of this handler diff --git a/src/dorkbox/util/messagebus/subscription/reflection/ReflectionSynchronizedInvocation.java b/src/dorkbox/util/messagebus/subscription/reflection/ReflectionSynchronizedInvocation.java index 81890c0..dc46080 100644 --- a/src/dorkbox/util/messagebus/subscription/reflection/ReflectionSynchronizedInvocation.java +++ b/src/dorkbox/util/messagebus/subscription/reflection/ReflectionSynchronizedInvocation.java @@ -50,7 +50,7 @@ import java.lang.reflect.Method; public class ReflectionSynchronizedInvocation implements ReflectionInvocation { - private ReflectionInvocation delegate; + private final ReflectionInvocation delegate; public ReflectionSynchronizedInvocation(ReflectionInvocation delegate) { diff --git a/src/dorkbox/util/messagebus/subscription/reflection/SubscriptionReflection.java b/src/dorkbox/util/messagebus/subscription/reflection/SubscriptionReflection.java index 7858319..2c13452 100644 --- a/src/dorkbox/util/messagebus/subscription/reflection/SubscriptionReflection.java +++ b/src/dorkbox/util/messagebus/subscription/reflection/SubscriptionReflection.java @@ -79,9 +79,6 @@ class SubscriptionReflection extends Subscription { } - /** - * @return true if messages were published - */ public void publish(final Object message) throws Throwable { final Method method = this.method; @@ -97,9 +94,6 @@ class SubscriptionReflection extends Subscription { } } - /** - * @return true if messages were published - */ public void publish(final Object message1, final Object message2) throws Throwable { final Method method = this.method; @@ -115,9 +109,6 @@ class SubscriptionReflection extends Subscription { } } - /** - * @return true if messages were published - */ public void publish(final Object message1, final Object message2, final Object message3) throws Throwable { final Method method = this.method; diff --git a/src/dorkbox/util/messagebus/synchrony/AsyncABQ.java b/src/dorkbox/util/messagebus/synchrony/AsyncABQ.java index 2e75d6b..4205680 100644 --- a/src/dorkbox/util/messagebus/synchrony/AsyncABQ.java +++ b/src/dorkbox/util/messagebus/synchrony/AsyncABQ.java @@ -52,7 +52,7 @@ class AsyncABQ implements Synchrony { // each thread will run forever and process incoming message publication requests Runnable runnable = new Runnable() { - @SuppressWarnings("ConstantConditions") + @SuppressWarnings({"ConstantConditions", "UnnecessaryLocalVariable"}) @Override public void run() { diff --git a/src/dorkbox/util/messagebus/synchrony/AsyncABQ_noGc.java b/src/dorkbox/util/messagebus/synchrony/AsyncABQ_noGc.java index 404bfd4..400d48e 100644 --- a/src/dorkbox/util/messagebus/synchrony/AsyncABQ_noGc.java +++ b/src/dorkbox/util/messagebus/synchrony/AsyncABQ_noGc.java @@ -62,7 +62,7 @@ class AsyncABQ_noGc implements Synchrony { // each thread will run forever and process incoming message publication requests Runnable runnable = new Runnable() { - @SuppressWarnings("ConstantConditions") + @SuppressWarnings({"ConstantConditions", "UnnecessaryLocalVariable"}) @Override public void run() { diff --git a/src/dorkbox/util/messagebus/synchrony/AsyncDisruptor.java b/src/dorkbox/util/messagebus/synchrony/AsyncDisruptor.java index 1099952..9149c97 100644 --- a/src/dorkbox/util/messagebus/synchrony/AsyncDisruptor.java +++ b/src/dorkbox/util/messagebus/synchrony/AsyncDisruptor.java @@ -35,10 +35,10 @@ import java.util.concurrent.locks.LockSupport; public final class AsyncDisruptor implements Synchrony { - private WorkProcessor[] workProcessors; - private MessageHandler[] handlers; - private RingBuffer ringBuffer; - private Sequence workSequence; + private final WorkProcessor[] workProcessors; + private final MessageHandler[] handlers; + private final RingBuffer ringBuffer; + private final Sequence workSequence; public AsyncDisruptor(final int numberOfThreads, final ErrorHandlingSupport errorHandler, final Synchrony syncPublication) { diff --git a/src/dorkbox/util/messagebus/synchrony/MessageHolder.java b/src/dorkbox/util/messagebus/synchrony/MessageHolder.java index 6ade7ad..a46ede7 100644 --- a/src/dorkbox/util/messagebus/synchrony/MessageHolder.java +++ b/src/dorkbox/util/messagebus/synchrony/MessageHolder.java @@ -32,29 +32,4 @@ class MessageHolder { public MessageHolder() {} - - public - MessageHolder(Subscription[] subscriptions, Object message1) { - this.subscriptions = subscriptions; - this.message1 = message1; - } - - public - MessageHolder(Subscription[] subscriptions, Object message1, Object message2) { - type = MessageType.TWO; - - this.subscriptions = subscriptions; - this.message1 = message1; - this.message2 = message2; - } - - public - MessageHolder(Subscription[] subscriptions, Object message1, Object message2, Object message3) { - type = MessageType.THREE; - - this.subscriptions = subscriptions; - this.message1 = message1; - this.message2 = message2; - this.message3 = message3; - } } diff --git a/src/dorkbox/util/messagebus/utils/ClassUtils.java b/src/dorkbox/util/messagebus/utils/ClassUtils.java index 307045a..bfccac6 100644 --- a/src/dorkbox/util/messagebus/utils/ClassUtils.java +++ b/src/dorkbox/util/messagebus/utils/ClassUtils.java @@ -16,6 +16,7 @@ package dorkbox.util.messagebus.utils; import com.esotericsoftware.kryo.util.IdentityMap; +import dorkbox.util.messagebus.subscription.SubscriptionManager; import java.lang.reflect.Array; import java.util.ArrayList; @@ -43,9 +44,9 @@ class ClassUtils { * principle" for storing data, EVEN THOUGH it's not accessed by a single writer. This DOES NOT MATTER because duplicates DO NOT matter */ public - ClassUtils(final float loadFactor) { - this.arrayCache = new IdentityMap, Class>(32, loadFactor); - this.superClassesCache = new IdentityMap, Class[]>(32, loadFactor); + ClassUtils() { + this.arrayCache = new IdentityMap, Class>(32, SubscriptionManager.LOAD_FACTOR); + this.superClassesCache = new IdentityMap, Class[]>(32, SubscriptionManager.LOAD_FACTOR); } /** @@ -58,7 +59,7 @@ class ClassUtils { public Class[] getSuperClasses(final Class clazz) { // access a snapshot of the subscriptions (single-writer-principle) - final IdentityMap, Class[]> cache = superClassesREF.get(this); + final IdentityMap, Class[]> cache = cast(superClassesREF.get(this)); Class[] classes = cache.get(clazz); @@ -115,7 +116,7 @@ class ClassUtils { public Class getArrayClass(final Class c) { // access a snapshot of the subscriptions (single-writer-principle) - final IdentityMap, Class> cache = arrayREF.get(this); + final IdentityMap, Class> cache = cast(arrayREF.get(this)); Class clazz = cache.get(c); @@ -141,4 +142,10 @@ class ClassUtils { this.arrayCache.clear(); this.superClassesCache.clear(); } + + @SuppressWarnings("unchecked") + private static + T cast(Object obj) { + return (T) obj; + } } diff --git a/src/dorkbox/util/messagebus/utils/ReflectionUtils.java b/src/dorkbox/util/messagebus/utils/ReflectionUtils.java index 7f6964f..e1e3af2 100644 --- a/src/dorkbox/util/messagebus/utils/ReflectionUtils.java +++ b/src/dorkbox/util/messagebus/utils/ReflectionUtils.java @@ -56,6 +56,9 @@ import java.util.Collection; public final class ReflectionUtils { + private static final Method[] EMPTY_METHODS = new Method[0]; + private static final Class[] EMPTY_CLASSES = new Class[0]; + private ReflectionUtils() { } @@ -65,7 +68,7 @@ class ReflectionUtils { ArrayList methods = new ArrayList(); getMethods(target, methods); - return methods.toArray(new Method[0]); + return methods.toArray(EMPTY_METHODS); } private static @@ -111,7 +114,7 @@ class ReflectionUtils { * @return A set of classes, each representing a super type of the root class */ public static - Class[] getSuperTypes2(Class from) { + Class[] getSuperTypes(Class from) { ArrayList> superclasses = new ArrayList>(); collectInterfaces(from, superclasses); @@ -122,25 +125,10 @@ class ReflectionUtils { collectInterfaces(from, superclasses); } - return superclasses.toArray(new Class[0]); + return superclasses.toArray(EMPTY_CLASSES); } - public static Class[] getSuperTypes(Class from) { - ArrayList> superclasses = new ArrayList>(); - - collectInterfaces( from, superclasses ); - while ( !from.equals( Object.class ) && !from.isInterface() ) { - superclasses.add( from.getSuperclass() ); - from = from.getSuperclass(); - collectInterfaces( from, superclasses ); - } - - return superclasses.toArray(new Class[0]); - } - - - - public static + private static void collectInterfaces(Class from, Collection> accumulator) { for (Class intface : from.getInterfaces()) { accumulator.add(intface); @@ -193,8 +181,7 @@ class ReflectionUtils { public static A getAnnotation(AnnotatedElement from, Class annotationType) { - A annotation = getAnnotation(from, annotationType, new IdentityMap()); - return annotation; + return getAnnotation(from, annotationType, new IdentityMap()); } // diff --git a/test/dorkbox/util/messagebus/MultiTreeTest.java b/test/dorkbox/util/messagebus/MultiTreeTest.java index bdcab7a..be2c67f 100644 --- a/test/dorkbox/util/messagebus/MultiTreeTest.java +++ b/test/dorkbox/util/messagebus/MultiTreeTest.java @@ -32,8 +32,6 @@ public class MultiTreeTest extends AssertSupport { final MultiClass d = tree.get(Object.class, Object.class); final MultiClass e = tree.get(String.class, String.class, String.class); - final MultiClass f = tree.get(String.class, String.class, String.class, String.class); - final MultiClass g = tree.get(Object.class, Object.class, String.class, Integer.class, Float.class); // we never can remove elements, unless we CLEAR the entire thing (usually at shutdown) assertNotNull(a); @@ -41,7 +39,5 @@ public class MultiTreeTest extends AssertSupport { assertNotNull(c); assertNotNull(d); assertNotNull(e); - assertNotNull(f); - assertNotNull(g); } }