From 5468bddb3568422c623d2ee08728cc268877bd1c Mon Sep 17 00:00:00 2001 From: kske Date: Fri, 14 Jan 2022 15:44:21 +0100 Subject: [PATCH 1/5] Add handler cache The cache has the same structure as the bindings and is updated accordingly. To ensure the correctness and efficiency of the cache, more testing has to be conducted. --- .../java/dev/kske/eventbus/core/EventBus.java | 65 +++++++++++++++---- 1 file changed, 54 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/dev/kske/eventbus/core/EventBus.java b/core/src/main/java/dev/kske/eventbus/core/EventBus.java index 334dd52..9f66c0c 100644 --- a/core/src/main/java/dev/kske/eventbus/core/EventBus.java +++ b/core/src/main/java/dev/kske/eventbus/core/EventBus.java @@ -90,6 +90,16 @@ public final class EventBus { */ private final Map, TreeSet> bindings = new ConcurrentHashMap<>(); + /** + * A cache mapping an event class to all handlers the event should be dispatched to. This + * includes polymorphic handlers that don't reference the event class explicitly. If an event + * class is not contained inside this cache, the {@link #bindings} have to be traversed manually + * in search of applicable handlers. + * + * @since 1.3.0 + */ + private final Map, TreeSet> bindingCache = new ConcurrentHashMap<>(); + /** * Stores all registered event listeners (which declare event handlers) and prevents them from * being garbage collected. @@ -175,24 +185,32 @@ public final class EventBus { * Searches for the event handlers bound to an event class. This includes polymorphic handlers * that are bound to a supertype of the event class. * + * @implNote If the given event type was requested in the past, the handlers are retrieved from + * the {@link #bindingCache}. If not, the entire {@link #bindings} are traversed in + * search of polymorphic handlers compatible with the event type. * @param eventType the event type to use for the search * @return a navigable set containing the applicable handlers in descending order of priority * @since 1.2.0 */ private NavigableSet getHandlersFor(Class eventType) { + if (bindingCache.containsKey(eventType)) { + return bindingCache.get(eventType); + } else { - // Get handlers defined for the event class - TreeSet handlers = - bindings.getOrDefault(eventType, new TreeSet<>(byPriority)); + // Get handlers defined for the event class + TreeSet handlers = + bindings.getOrDefault(eventType, new TreeSet<>(byPriority)); - // Get polymorphic handlers - for (var binding : bindings.entrySet()) - if (binding.getKey().isAssignableFrom(eventType)) - for (var handler : binding.getValue()) - if (handler.isPolymorphic()) - handlers.add(handler); + // Get polymorphic handlers + for (var binding : bindings.entrySet()) + if (binding.getKey().isAssignableFrom(eventType)) + for (var handler : binding.getValue()) + if (handler.isPolymorphic()) + handlers.add(handler); - return handlers; + bindingCache.put(eventType, handlers); + return handlers; + } } /** @@ -369,15 +387,28 @@ public final class EventBus { } /** - * Inserts a new handler into the {@link #bindings} map. + * Inserts a new handler into the {@link #bindings} map. Additionally, the handler is placed + * inside the {@link #bindingCache} where applicable. * * @param handler the handler to bind * @since 1.2.0 */ private void bindHandler(EventHandler handler) { + + // Bind handler bindings.putIfAbsent(handler.getEventType(), new TreeSet<>(byPriority)); logger.log(Level.DEBUG, "Binding event handler {0}", handler); bindings.get(handler.getEventType()).add(handler); + + // Insert handler into cache + bindingCache.putIfAbsent(handler.getEventType(), new TreeSet<>(byPriority)); + bindingCache.get(handler.getEventType()).add(handler); + + // Handler is polymorphic => insert where applicable + if (handler.isPolymorphic()) + for (var binding : bindingCache.entrySet()) + if (binding.getKey().isAssignableFrom(handler.getEventType())) + binding.getValue().add(handler); } /** @@ -402,6 +433,18 @@ public final class EventBus { } } + // Remove bindings from cache + for (var binding : bindingCache.values()) { + var it = binding.iterator(); + while (it.hasNext()) { + var handler = it.next(); + if (handler.getListener() == listener) { + logger.log(Level.TRACE, "Removing event handler {0} from cache", handler); + it.remove(); + } + } + } + // Remove the listener itself registeredListeners.remove(listener); } From ee9d08b2b8d6ef6be8f3216481f4a011b1cdc2f6 Mon Sep 17 00:00:00 2001 From: kske Date: Tue, 18 Jan 2022 13:44:33 +0100 Subject: [PATCH 2/5] Test binding cache --- .../java/dev/kske/eventbus/core/EventBus.java | 1 + .../dev/kske/eventbus/core/DispatchTest.java | 42 ++++++++++++------- .../dev/kske/eventbus/core/SimpleEvent.java | 2 +- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/dev/kske/eventbus/core/EventBus.java b/core/src/main/java/dev/kske/eventbus/core/EventBus.java index 9f66c0c..1d39e1e 100644 --- a/core/src/main/java/dev/kske/eventbus/core/EventBus.java +++ b/core/src/main/java/dev/kske/eventbus/core/EventBus.java @@ -457,6 +457,7 @@ public final class EventBus { public void clearListeners() { logger.log(Level.INFO, "Clearing event listeners"); bindings.clear(); + bindingCache.clear(); registeredListeners.clear(); } diff --git a/core/src/test/java/dev/kske/eventbus/core/DispatchTest.java b/core/src/test/java/dev/kske/eventbus/core/DispatchTest.java index cfd4de1..b126475 100644 --- a/core/src/test/java/dev/kske/eventbus/core/DispatchTest.java +++ b/core/src/test/java/dev/kske/eventbus/core/DispatchTest.java @@ -14,8 +14,7 @@ import org.junit.jupiter.api.*; @Priority(150) class DispatchTest { - EventBus bus; - static int hits; + EventBus bus; /** * Constructs an event bus and registers this test instance as an event listener. @@ -27,8 +26,8 @@ class DispatchTest { bus = new EventBus(); bus.registerListener(this); bus.registerListener(SimpleEvent.class, e -> { - ++hits; - assertEquals(4, hits); + e.increment(); + assertEquals(3, e.getCounter()); }); } @@ -56,24 +55,37 @@ class DispatchTest { assertEquals( "Event handler execution order for class dev.kske.eventbus.core.SimpleEvent (3 handler(s)):\n" + "==========================================================================================\n" - + "ReflectiveEventHandler[eventType=class dev.kske.eventbus.core.SimpleEvent, polymorphic=true, priority=200, method=void dev.kske.eventbus.core.DispatchTest.onSimpleEventFirst(), useParameter=false]\n" - + "ReflectiveEventHandler[eventType=class dev.kske.eventbus.core.SimpleEvent, polymorphic=false, priority=150, method=static void dev.kske.eventbus.core.DispatchTest.onSimpleEventSecond(), useParameter=false]\n" + + "ReflectiveEventHandler[eventType=class dev.kske.eventbus.core.SimpleEvent, polymorphic=true, priority=200, method=void dev.kske.eventbus.core.DispatchTest.onSimpleEventFirst(dev.kske.eventbus.core.SimpleEvent), useParameter=true]\n" + + "ReflectiveEventHandler[eventType=class dev.kske.eventbus.core.SimpleEvent, polymorphic=false, priority=150, method=static void dev.kske.eventbus.core.DispatchTest.onSimpleEventSecond(dev.kske.eventbus.core.SimpleEvent), useParameter=true]\n" + "CallbackEventHandler[eventType=class dev.kske.eventbus.core.SimpleEvent, polymorphic=false, priority=100]\n" + "==========================================================================================", executionOrder); } - @Event(SimpleEvent.class) - @Priority(200) - void onSimpleEventFirst() { - ++hits; - assertTrue(hits == 1 || hits == 2); + /** + * Tests whether the handlers bound to an event type are correct when retrieved from the binding + * cache. On the second call of {@link EventBus#debugExecutionOrder(Class)} the cache is used. + * + * @since 1.3.0 + */ + @Test + void testBindingCache() { + String executionOrder = bus.debugExecutionOrder(SimpleEventSub.class); + System.out.println(executionOrder); + assertEquals(executionOrder, bus.debugExecutionOrder(SimpleEventSub.class)); } - @Event(SimpleEvent.class) + @Event + @Priority(200) + void onSimpleEventFirst(SimpleEvent event) { + event.increment(); + assertTrue(event.getCounter() == 1 || event.getCounter() == 2); + } + + @Event @Polymorphic(false) - static void onSimpleEventSecond() { - ++hits; - assertEquals(3, hits); + static void onSimpleEventSecond(SimpleEvent event) { + event.increment(); + assertEquals(2, event.getCounter()); } } diff --git a/core/src/test/java/dev/kske/eventbus/core/SimpleEvent.java b/core/src/test/java/dev/kske/eventbus/core/SimpleEvent.java index 1512107..5536698 100644 --- a/core/src/test/java/dev/kske/eventbus/core/SimpleEvent.java +++ b/core/src/test/java/dev/kske/eventbus/core/SimpleEvent.java @@ -14,7 +14,7 @@ class SimpleEvent { @Override public String toString() { - return String.format("SimpleEvent[%d]", counter); + return String.format("%s[%d]", getClass().getSimpleName(), counter); } void increment() { From 8609c6a90c2ae655058fdb09ec3d5a0f00c685ee Mon Sep 17 00:00:00 2001 From: kske Date: Tue, 18 Jan 2022 15:00:18 +0100 Subject: [PATCH 3/5] Simplify binding cache usage --- .../java/dev/kske/eventbus/core/EventBus.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/dev/kske/eventbus/core/EventBus.java b/core/src/main/java/dev/kske/eventbus/core/EventBus.java index 1d39e1e..dc94a7f 100644 --- a/core/src/main/java/dev/kske/eventbus/core/EventBus.java +++ b/core/src/main/java/dev/kske/eventbus/core/EventBus.java @@ -193,9 +193,7 @@ public final class EventBus { * @since 1.2.0 */ private NavigableSet getHandlersFor(Class eventType) { - if (bindingCache.containsKey(eventType)) { - return bindingCache.get(eventType); - } else { + return bindingCache.computeIfAbsent(eventType, k -> { // Get handlers defined for the event class TreeSet handlers = @@ -208,9 +206,8 @@ public final class EventBus { if (handler.isPolymorphic()) handlers.add(handler); - bindingCache.put(eventType, handlers); return handlers; - } + }); } /** @@ -396,13 +393,13 @@ public final class EventBus { private void bindHandler(EventHandler handler) { // Bind handler - bindings.putIfAbsent(handler.getEventType(), new TreeSet<>(byPriority)); logger.log(Level.DEBUG, "Binding event handler {0}", handler); - bindings.get(handler.getEventType()).add(handler); + bindings.computeIfAbsent(handler.getEventType(), k -> new TreeSet<>(byPriority)) + .add(handler); // Insert handler into cache - bindingCache.putIfAbsent(handler.getEventType(), new TreeSet<>(byPriority)); - bindingCache.get(handler.getEventType()).add(handler); + bindingCache.computeIfAbsent(handler.getEventType(), k -> new TreeSet<>(byPriority)) + .add(handler); // Handler is polymorphic => insert where applicable if (handler.isPolymorphic()) From 2d276a1d749a83a09469b34b5de921c911797402 Mon Sep 17 00:00:00 2001 From: kske Date: Tue, 18 Jan 2022 17:09:05 +0100 Subject: [PATCH 4/5] Compare listener using equals() during removal --- core/src/main/java/dev/kske/eventbus/core/EventBus.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/dev/kske/eventbus/core/EventBus.java b/core/src/main/java/dev/kske/eventbus/core/EventBus.java index dc94a7f..957da57 100644 --- a/core/src/main/java/dev/kske/eventbus/core/EventBus.java +++ b/core/src/main/java/dev/kske/eventbus/core/EventBus.java @@ -423,7 +423,7 @@ public final class EventBus { var it = binding.iterator(); while (it.hasNext()) { var handler = it.next(); - if (handler.getListener() == listener) { + if (handler.getListener().equals(listener)) { logger.log(Level.DEBUG, "Unbinding event handler {0}", handler); it.remove(); } @@ -435,7 +435,7 @@ public final class EventBus { var it = binding.iterator(); while (it.hasNext()) { var handler = it.next(); - if (handler.getListener() == listener) { + if (handler.getListener().equals(listener)) { logger.log(Level.TRACE, "Removing event handler {0} from cache", handler); it.remove(); } From 8fae4f6d763822933ef5ee437895c88a22eeac80 Mon Sep 17 00:00:00 2001 From: kske Date: Tue, 18 Jan 2022 17:09:21 +0100 Subject: [PATCH 5/5] Remove print statements from test --- .../test/java/dev/kske/eventbus/core/DispatchTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/core/src/test/java/dev/kske/eventbus/core/DispatchTest.java b/core/src/test/java/dev/kske/eventbus/core/DispatchTest.java index b126475..919bdfc 100644 --- a/core/src/test/java/dev/kske/eventbus/core/DispatchTest.java +++ b/core/src/test/java/dev/kske/eventbus/core/DispatchTest.java @@ -50,8 +50,6 @@ class DispatchTest { */ @Test void testDebugExecutionOrder() { - String executionOrder = bus.debugExecutionOrder(SimpleEvent.class); - System.out.println(executionOrder); assertEquals( "Event handler execution order for class dev.kske.eventbus.core.SimpleEvent (3 handler(s)):\n" + "==========================================================================================\n" @@ -59,7 +57,7 @@ class DispatchTest { + "ReflectiveEventHandler[eventType=class dev.kske.eventbus.core.SimpleEvent, polymorphic=false, priority=150, method=static void dev.kske.eventbus.core.DispatchTest.onSimpleEventSecond(dev.kske.eventbus.core.SimpleEvent), useParameter=true]\n" + "CallbackEventHandler[eventType=class dev.kske.eventbus.core.SimpleEvent, polymorphic=false, priority=100]\n" + "==========================================================================================", - executionOrder); + bus.debugExecutionOrder(SimpleEvent.class)); } /** @@ -70,9 +68,8 @@ class DispatchTest { */ @Test void testBindingCache() { - String executionOrder = bus.debugExecutionOrder(SimpleEventSub.class); - System.out.println(executionOrder); - assertEquals(executionOrder, bus.debugExecutionOrder(SimpleEventSub.class)); + assertEquals(bus.debugExecutionOrder(SimpleEventSub.class), + bus.debugExecutionOrder(SimpleEventSub.class)); } @Event