Fix EventBus Performance issues

editor/src/main/com/mbrlabs/mundus/editor/events/EventBus.java

@@ -16,70 +16,116 @@
 
 package com.mbrlabs.mundus.editor.events;
 
+import com.mbrlabs.mundus.editor.utils.ReflectionUtils;
+
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.util.LinkedList;
+import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
-
-import com.mbrlabs.mundus.editor.utils.ReflectionUtils;
+import java.util.Map;
 
 /**
  * Simple Event bus via reflection.
- *
+ * <p>
  * Subscribers need to provide a public method, annotated with @Subscribe and 1
  * parameter as event type.
- *
+ * <p>
  * Inspired by the Otto Event Bus for Android.
  *
  * @author Marcus Brummer
  * @version 12-12-2015
  */
-// TODO improve/test performance might not be that great
 public class EventBus {
 
-    private class EventBusExcetion extends RuntimeException {
-        private EventBusExcetion(String s) {
+    private static class EventBusException extends RuntimeException {
+        private EventBusException(String s) {
             super(s);
         }
     }
 
-    private List<Object> subscribers;
+    /**
+     * Tracks the subscriber methods for each event type.
+     */
+    private static class SubscriberMethod {
+        final Object instance;
+        final Method method;
+
+        SubscriberMethod(Object instance, Method method) {
+            this.instance = instance;
+            this.method = method;
+        }
+    }
+
+    // Maps the event class to subscriber methods, cached for performance
+    private final Map<Class<?>, List<SubscriberMethod>> subscribersMap;
 
     public EventBus() {
-        subscribers = new LinkedList<>();
+        subscribersMap = new HashMap<>();
     }
 
+    /**
+     * Registers all subscriber methods of the given object.
+     * For performance reasons we cache the subscriber methods for each event on register.
+     *
+     * @param subscriber the object with subscriber methods
+     */
     public void register(Object subscriber) {
-        subscribers.add(subscriber);
+        // Loop over each method in the subscriber
+        for (Method method : subscriber.getClass().getDeclaredMethods()) {
+            if (!isSubscriber(method)) continue;
+
+            if (method.getParameterTypes().length != 1) {
+                throw new EventBusException("Size of parameter list of method " + method.getName() + " in "
+                        + subscriber.getClass().getName() + " must be 1");
+            }
+
+            Class<?> eventType = method.getParameterTypes()[0];
+
+            // Get the list of subscribers for this event
+            List<SubscriberMethod> subscribers = subscribersMap.computeIfAbsent(eventType, k -> new ArrayList<>());
+            subscribers.add(new SubscriberMethod(subscriber, method));
+        }
     }
 
+    /**
+     * Unregisters all subscriber methods of the given object.
+     * @param subscriber
+     */
     public void unregister(Object subscriber) {
-        subscribers.remove(subscriber);
+        // Iterate over each event type in the map, remove if matched
+        for (List<SubscriberMethod> methods : subscribersMap.values()) {
+            methods.removeIf(subscriberMethod -> subscriberMethod.instance.equals(subscriber));
+        }
     }
 
+    /**
+     * Posts an event to all subscribers of this event type.
+     * @param event the event to post
+     */
     public void post(Object event) {
-        try {
-            final Class eventType = event.getClass();
-            for (Object subscriber : subscribers.toArray()) {
-                for (Method method : subscriber.getClass().getDeclaredMethods()) {
-                    if (isSubscriber(method)) {
-                        if (method.getParameterTypes().length != 1) {
-                            throw new EventBusExcetion("Size of parameter list of method " + method.getName() + " in "
-                                    + subscriber.getClass().getName() + " must be 1");
-                        }
-
-                        if (method.getParameterTypes()[0].equals(eventType)) {
-                            // System.out.println(subscriber.getClass().getName());
-                            method.invoke(subscriber, eventType.cast(event));
-                        }
-                    }
-                }
+        // Get the list of subscribers for this event
+        List<SubscriberMethod> subscribers = subscribersMap.get(event.getClass());
+
+        if (subscribers == null) return;
+
+        // Call each subscriber method
+        for (SubscriberMethod subscriberMethod : subscribers) {
+            try {
+                subscriberMethod.method.invoke(subscriberMethod.instance, event);
+            } catch (IllegalAccessException | InvocationTargetException e) {
+                e.printStackTrace();
             }
-        } catch (IllegalAccessException | InvocationTargetException e) {
-            e.printStackTrace();
         }
     }
 
+    /**
+     * Checks if the given method is a subscriber.
+     * Slow with reflection, but we only check it on register.
+     *
+     * @param method the method to check
+     * @return true if the method is a subscriber
+     */
     private boolean isSubscriber(Method method) {
         // check if @Subscribe is directly used in class
         boolean isSub = ReflectionUtils.hasMethodAnnotation(method, Subscribe.class);

editor/src/test/java/com/mbrlabs/mundus/editor/events/EventBusTest.java

@@ -0,0 +1,72 @@
+package com.mbrlabs.mundus.editor.events;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * @author JamesTKhan
+ * @version June 27, 2023
+ */
+public class EventBusTest {
+
+    @Test
+    public void eventsReceivedTest() {
+        EventBus eventBus = new EventBus();
+        final int[] sceneChangedEventsReceived = {0};
+        final int[] sceneAddedEventsReceived = {0};
+
+        eventBus.register(new Object() {
+            @Subscribe
+            public void onEvent(SceneChangedEvent event) {
+                sceneChangedEventsReceived[0]++;
+            }
+        });
+
+        eventBus.register(new Object() {
+            @Subscribe
+            public void onEvent(SceneAddedEvent event) {
+                sceneAddedEventsReceived[0]++;
+            }
+        });
+
+        int sceneChangedCount = 10;
+        int sceneAddedCount = 5;
+
+        for (int i = 0; i < sceneChangedCount; i++) {
+            eventBus.post(new SceneChangedEvent());
+        }
+
+        for (int i = 0; i < sceneAddedCount; i++) {
+            eventBus.post(new SceneAddedEvent(null));
+        }
+
+        // check if events were received
+        Assert.assertEquals(sceneChangedEventsReceived[0], sceneChangedCount);
+        Assert.assertEquals(sceneAddedEventsReceived[0], sceneAddedCount);
+    }
+
+    @Test
+    public void unregisterTest() {
+        EventBus eventBus = new EventBus();
+        final int[] sceneChangedEventsReceived = {0};
+
+        Object subscriber = new Object() {
+            @Subscribe
+            public void onEvent(SceneChangedEvent event) {
+                sceneChangedEventsReceived[0]++;
+            }
+        };
+
+        // register and post, should receive 1 event
+        eventBus.register(subscriber);
+        eventBus.post(new SceneChangedEvent());
+
+        // unregister and post, should not receive any events
+        eventBus.unregister(subscriber);
+        eventBus.post(new SceneChangedEvent());
+
+        // should still be 1
+        Assert.assertEquals(sceneChangedEventsReceived[0], 1);
+    }
+
+}