Handler Caching #37

Merged
kske merged 5 commits from f/handler-caching into develop 2022-01-18 17:11:38 +01:00
3 changed files with 82 additions and 32 deletions

View File

@ -90,6 +90,16 @@ public final class EventBus {
*/ */
private final Map<Class<?>, TreeSet<EventHandler>> bindings = new ConcurrentHashMap<>(); private final Map<Class<?>, TreeSet<EventHandler>> 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<Class<?>, TreeSet<EventHandler>> bindingCache = new ConcurrentHashMap<>();
/** /**
* Stores all registered event listeners (which declare event handlers) and prevents them from * Stores all registered event listeners (which declare event handlers) and prevents them from
* being garbage collected. * being garbage collected.
@ -175,11 +185,15 @@ public final class EventBus {
* Searches for the event handlers bound to an event class. This includes polymorphic handlers * Searches for the event handlers bound to an event class. This includes polymorphic handlers
* that are bound to a supertype of the event class. * 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 * @param eventType the event type to use for the search
* @return a navigable set containing the applicable handlers in descending order of priority * @return a navigable set containing the applicable handlers in descending order of priority
* @since 1.2.0 * @since 1.2.0
*/ */
private NavigableSet<EventHandler> getHandlersFor(Class<?> eventType) { private NavigableSet<EventHandler> getHandlersFor(Class<?> eventType) {
return bindingCache.computeIfAbsent(eventType, k -> {
// Get handlers defined for the event class // Get handlers defined for the event class
TreeSet<EventHandler> handlers = TreeSet<EventHandler> handlers =
@ -193,6 +207,7 @@ public final class EventBus {
handlers.add(handler); handlers.add(handler);
return handlers; return handlers;
});
} }
/** /**
@ -369,15 +384,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 * @param handler the handler to bind
* @since 1.2.0 * @since 1.2.0
*/ */
private void bindHandler(EventHandler handler) { private void bindHandler(EventHandler handler) {
bindings.putIfAbsent(handler.getEventType(), new TreeSet<>(byPriority));
// Bind handler
logger.log(Level.DEBUG, "Binding event handler {0}", handler); 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.computeIfAbsent(handler.getEventType(), k -> new TreeSet<>(byPriority))
.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);
} }
/** /**
@ -395,13 +423,25 @@ public final class EventBus {
var it = binding.iterator(); var it = binding.iterator();
while (it.hasNext()) { while (it.hasNext()) {
var handler = it.next(); var handler = it.next();
if (handler.getListener() == listener) { if (handler.getListener().equals(listener)) {
logger.log(Level.DEBUG, "Unbinding event handler {0}", handler); logger.log(Level.DEBUG, "Unbinding event handler {0}", handler);
it.remove(); it.remove();
} }
} }
} }
// Remove bindings from cache
for (var binding : bindingCache.values()) {
var it = binding.iterator();
while (it.hasNext()) {
var handler = it.next();
if (handler.getListener().equals(listener)) {
kske marked this conversation as resolved Outdated
Outdated
Review

Maybe add a comment on why == instead of equals.

Maybe add a comment on why `==` instead of `equals`.
Outdated
Review

As we implicitly test using equals in registerListener(...), I will change this as well.

As we implicitly test using `equals` in `registerListener(...)`, I will change this as well.
logger.log(Level.TRACE, "Removing event handler {0} from cache", handler);
it.remove();
}
}
}
// Remove the listener itself // Remove the listener itself
registeredListeners.remove(listener); registeredListeners.remove(listener);
} }
@ -414,6 +454,7 @@ public final class EventBus {
public void clearListeners() { public void clearListeners() {
logger.log(Level.INFO, "Clearing event listeners"); logger.log(Level.INFO, "Clearing event listeners");
bindings.clear(); bindings.clear();
bindingCache.clear();
registeredListeners.clear(); registeredListeners.clear();
} }

View File

@ -15,7 +15,6 @@ import org.junit.jupiter.api.*;
class DispatchTest { class DispatchTest {
EventBus bus; EventBus bus;
static int hits;
/** /**
* Constructs an event bus and registers this test instance as an event listener. * Constructs an event bus and registers this test instance as an event listener.
@ -27,8 +26,8 @@ class DispatchTest {
bus = new EventBus(); bus = new EventBus();
bus.registerListener(this); bus.registerListener(this);
bus.registerListener(SimpleEvent.class, e -> { bus.registerListener(SimpleEvent.class, e -> {
++hits; e.increment();
assertEquals(4, hits); assertEquals(3, e.getCounter());
}); });
} }
@ -51,29 +50,39 @@ class DispatchTest {
*/ */
@Test @Test
void testDebugExecutionOrder() { void testDebugExecutionOrder() {
String executionOrder = bus.debugExecutionOrder(SimpleEvent.class);
System.out.println(executionOrder);
assertEquals( assertEquals(
"Event handler execution order for class dev.kske.eventbus.core.SimpleEvent (3 handler(s)):\n" "Event handler execution order for class dev.kske.eventbus.core.SimpleEvent (3 handler(s)):\n"
+ "==========================================================================================\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=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(), useParameter=false]\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" + "CallbackEventHandler[eventType=class dev.kske.eventbus.core.SimpleEvent, polymorphic=false, priority=100]\n"
+ "==========================================================================================", + "==========================================================================================",
executionOrder); bus.debugExecutionOrder(SimpleEvent.class));
} }
@Event(SimpleEvent.class) /**
* 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() {
assertEquals(bus.debugExecutionOrder(SimpleEventSub.class),
bus.debugExecutionOrder(SimpleEventSub.class));
}
kske marked this conversation as resolved
Review

Why?

Why?
@Event
@Priority(200) @Priority(200)
void onSimpleEventFirst() { void onSimpleEventFirst(SimpleEvent event) {
++hits; event.increment();
assertTrue(hits == 1 || hits == 2); assertTrue(event.getCounter() == 1 || event.getCounter() == 2);
} }
@Event(SimpleEvent.class) @Event
@Polymorphic(false) @Polymorphic(false)
static void onSimpleEventSecond() { static void onSimpleEventSecond(SimpleEvent event) {
++hits; event.increment();
assertEquals(3, hits); assertEquals(2, event.getCounter());
} }
} }

View File

@ -14,7 +14,7 @@ class SimpleEvent {
@Override @Override
public String toString() { public String toString() {
return String.format("SimpleEvent[%d]", counter); return String.format("%s[%d]", getClass().getSimpleName(), counter);
} }
void increment() { void increment() {