[6.2] New aproach to optimizing cancelable events

eventbus-test/src/test/java/net/minecraftforge/eventbus/test/TestModLauncher.java

@@ -17,6 +17,7 @@
 import net.minecraftforge.eventbus.test.general.ParallelEventTest;
 import net.minecraftforge.eventbus.test.general.ParentHandlersGetInvokedTest;
 import net.minecraftforge.eventbus.test.general.ParentHandlersGetInvokedTestDummy;
+import net.minecraftforge.eventbus.test.general.ParentInheritsCancelableTest;
 import net.minecraftforge.eventbus.test.general.ThreadedListenerExceptionTest;
 
 import org.junit.jupiter.api.RepeatedTest;
@@ -82,6 +83,11 @@ public void parentHandlerGetsInvokedDummy() {
         doTest(new ParentHandlersGetInvokedTestDummy() {});
     }
 
+    @Test
+    public void parentInheritsCanceableTest() {
+    	doTest(new ParentInheritsCancelableTest() {} );
+    }
+
     @RepeatedTest(100)
     public void testThreadedEventFiring() {
         doTest(new ThreadedListenerExceptionTest() {});

eventbus-test/src/test/java/net/minecraftforge/eventbus/test/TestNoLoader.java

@@ -17,6 +17,7 @@
 import net.minecraftforge.eventbus.test.general.ParallelEventTest;
 import net.minecraftforge.eventbus.test.general.ParentHandlersGetInvokedTest;
 import net.minecraftforge.eventbus.test.general.ParentHandlersGetInvokedTestDummy;
+import net.minecraftforge.eventbus.test.general.ParentInheritsCancelableTest;
 import net.minecraftforge.eventbus.test.general.ThreadedListenerExceptionTest;
 
 import org.junit.jupiter.api.Disabled;
@@ -89,6 +90,11 @@ public void parentHandlerGetsInvokedDummy() {
         doTest(new ParentHandlersGetInvokedTestDummy() {});
     }
 
+    @Test
+    public void parentInheritsCanceableTest() {
+    	doTest(new ParentInheritsCancelableTest() {} );
+    }
+
     @RepeatedTest(100)
     public void testThreadedEventFiring() {
         doTest(new ThreadedListenerExceptionTest() {});

eventbus-test/src/test/java/net/minecraftforge/eventbus/test/general/ParallelEventTest.java

@@ -53,7 +53,7 @@ public void test(Consumer<Class<?>> validator, Supplier<BusBuilder> builder) {
             busSet.forEach(bus -> {
                 int busid = Whitebox.getInternalState(bus, "busID");
                 ListenerList afterAdd = Whitebox.invokeMethod(new DummyEvent.GoodEvent(), "getListenerList");
-                assertEquals(LISTENER_COUNT, afterAdd.getListeners(busid).length - 1, "Failed to register all event handlers");
+                assertEquals(LISTENER_COUNT, afterAdd.getListeners(busid).length, "Failed to register all event handlers");
             });
 
             busSet.parallelStream().forEach(iEventBus -> { //post events parallel
@@ -80,7 +80,7 @@ public void test(Consumer<Class<?>> validator, Supplier<BusBuilder> builder) {
             // Make sure it tracked them all
             int busid = Whitebox.getInternalState(bus, "busID");
             ListenerList afterAdd = Whitebox.invokeMethod(new DummyEvent.GoodEvent(), "getListenerList");
-            assertEquals(LISTENER_COUNT, afterAdd.getListeners(busid).length - 1, "Failed to register all event handlers");
+            assertEquals(LISTENER_COUNT, afterAdd.getListeners(busid).length, "Failed to register all event handlers");
 
             toAdd = new HashSet<>();
             for (int i = 0; i < RUN_ITERATIONS; i++) //prepare parallel event posting

eventbus-test/src/test/java/net/minecraftforge/eventbus/test/general/ParentInheritsCancelableTest.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) Forge Development LLC
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.eventbus.test.general;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import net.minecraftforge.eventbus.api.BusBuilder;
+import net.minecraftforge.eventbus.api.Cancelable;
+import net.minecraftforge.eventbus.api.Event;
+import net.minecraftforge.eventbus.api.EventPriority;
+import net.minecraftforge.eventbus.api.IEventBus;
+import net.minecraftforge.eventbus.api.SubscribeEvent;
+import net.minecraftforge.eventbus.test.ITestHandler;
+
+public class ParentInheritsCancelableTest implements ITestHandler {
+    @Override
+    public void test(Consumer<Class<?>> validator, Supplier<BusBuilder> builder) {
+        validator.accept(SuperEvent.class);
+
+        IEventBus bus = builder.get().build();
+
+        // Register a Non-Cancelable ASMEventHandler to the SuperEvent. Which should result in an Unchecked handler being registered
+        Listener listener = new Listener();
+        bus.register(listener);
+
+         // Test that it gets invoked
+        bus.post(new SuperEvent());
+        assertTrue(listener.invoked, "Handler was not invoked for SuperEvent");
+        listener.invoked = false;
+
+        // Now we classload the Cancelable event
+        validator.accept(SubEvent.class);
+
+        // Lambda handler should be invoked, and so should the normal listener
+        AtomicBoolean handled = new AtomicBoolean(false);
+        bus.addListener(EventPriority.NORMAL, false, SubEvent.class, (SubEvent event) -> {
+        	handled.set(true);
+        });
+
+        bus.post(new SubEvent());
+        assertTrue(listener.invoked, "Handler was not invoked for SubEvent");
+        listener.invoked = false;
+        assertTrue(handled.getAndSet(false), "Lambda Handler was not invoked for SubEvent");
+
+        // And finally lets add a listener that cancels the event, its registered after the first lambda, so that should be called, but it cancels so the ASMEventHandler in Super shouldn't be
+        AtomicBoolean canceled = new AtomicBoolean(false);
+        bus.addListener(EventPriority.NORMAL, false, SubEvent.class, (SubEvent event) -> {
+        	canceled.set(true);
+        	event.setCanceled(true);
+        });
+
+
+        bus.post(new SubEvent());
+        assertTrue(handled.get(), "Lambda Handler was not invoked for SubEvent");
+        assertTrue(canceled.get(), "Canceling Handler was not invoked for SubEvent");
+        assertFalse(listener.invoked, "Handler was invoked for canceled SubEvent");
+    }
+
+    public static class SuperEvent extends Event {}
+
+    @Cancelable
+    public static class SubEvent extends SuperEvent {}
+
+    public static class Listener {
+    	private boolean invoked = false;
+
+    	@SubscribeEvent
+    	public void listener(SuperEvent event) {
+    		invoked = true;
+    	}
+    }
+}

src/main/java/net/minecraftforge/eventbus/ASMEventHandler.java

@@ -20,14 +20,26 @@ public class ASMEventHandler implements IEventListener {
      */
     @Deprecated
     public ASMEventHandler(IEventListenerFactory factory, Object target, Method method, boolean isGeneric) throws IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException, ClassNotFoundException {
-        this(factory, target, method, isGeneric, method.getAnnotation(SubscribeEvent.class));
+        this(
+            factory.create(method, target),
+            method.getAnnotation(SubscribeEvent.class),
+            makeReadable(target, method),
+            getFilter(isGeneric, method)
+        );
     }
 
-    private ASMEventHandler(IEventListenerFactory factory, Object target, Method method, boolean isGeneric, SubscribeEvent subInfo) throws IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException, ClassNotFoundException {
-        handler = factory.create(method, target);
-
+    private ASMEventHandler(IEventListener handler, SubscribeEvent subInfo, String readable, Type filter) {
+        this.handler = handler;
         this.subInfo = subInfo;
-        readable = "ASM: " + target + " " + method.getName() + getMethodDescriptor(method);
+        this.readable = readable;
+        this.filter = filter;
+    }
+
+    private static String makeReadable(Object target, Method method) {
+        return "ASM: " + target + " " + method.getName() + getMethodDescriptor(method);
+    }
+
+    private static Type getFilter(boolean isGeneric, Method method) {
         Type filter = null;
         if (isGeneric) {
             Type type = method.getGenericParameterTypes()[0];
@@ -43,7 +55,7 @@ else if (filter instanceof WildcardType wfilter) {
                 }
             }
         }
-        this.filter = filter;
+        return filter;
     }
 
     @SuppressWarnings("rawtypes")
@@ -59,33 +71,65 @@ public EventPriority getPriority() {
         return subInfo.priority();
     }
 
+    @Override
     public String toString() {
         return readable;
     }
 
     /**
      * Creates a new ASMEventHandler instance, factoring in a time-shifting optimisation.
      *
-     * <p>In the case that no post-time checks are needed, an anonymous subclass instance will be returned that calls
-     * the listener without additional redundant checks.</p>
+     * <p>In the case that no post-time checks are needed, an subclass instance will be returned that calls
+     * the listener without additional redundant checks.</p>     *
      *
-     * @implNote The 'all or nothing' nature of the post-time checks is to reduce the likelihood of megamorphic method
-     *           invocation, which isn't as performant as monomorphic or bimorphic calls in Java 16
-     *           (what EventBus 6.2.x targets).
+     * @deprecated Use {@link #of(IEventListenerFactory, Object, Method, boolean, boolean)} instead to prevent wrapping in ASMEventHandler type, for better performance
      */
+    @Deprecated
     public static ASMEventHandler of(IEventListenerFactory factory, Object target, Method method, boolean isGeneric) throws IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException, ClassNotFoundException {
         var subInfo = method.getAnnotation(SubscribeEvent.class);
         assert subInfo != null;
         var eventType = method.getParameterTypes()[0];
-        if (isGeneric || !Modifier.isFinal(eventType.getModifiers()) || EventListenerHelper.isCancelable(eventType))
-            return new ASMEventHandler(factory, target, method, isGeneric, subInfo);
+        var filter = getFilter(isGeneric, method);
+        var readable = makeReadable(target, method);
+        var cancelable = EventListenerHelper.isCancelable(eventType);
 
-        // If we get to this point, no post-time checks are needed, so strip them out
-        return new ASMEventHandler(factory, target, method, false, subInfo) {
-            @Override
-            public void invoke(Event event) {
-                handler.invoke(event);
-            }
-        };
+        var handler = ReactiveEventListener.of(factory.create(method, target), readable, filter, subInfo.receiveCanceled(), cancelable);
+        if (handler instanceof IReactiveEventListener)
+            return new Reactive(handler, subInfo, readable, filter);
+        return new ASMEventHandler(handler, subInfo, readable, filter);
+    }
+
+
+    /**
+     * Creates a new ASMEventHandler instance, factoring in a time-shifting optimisation.
+     *
+     * <p>In the case that no post-time checks are needed, an subclass instance will be returned that calls
+     * the listener without additional redundant checks.</p>
+     */
+    public static IEventListener of(IEventListenerFactory factory, Object target, Method method, boolean isGeneric, boolean forceCancelable) throws IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException, ClassNotFoundException {
+        var subInfo = method.getAnnotation(SubscribeEvent.class);
+        assert subInfo != null;
+        var eventType = method.getParameterTypes()[0];
+        var filter = getFilter(isGeneric, method);
+        var readable = makeReadable(target, method);
+        var cancelable = forceCancelable || EventListenerHelper.isCancelable(eventType);
+
+        return ReactiveEventListener.of(factory.create(method, target), readable, filter, subInfo.receiveCanceled(), cancelable);
+    }
+
+    private static class Reactive extends ASMEventHandler implements IReactiveEventListener {
+        private Reactive(IEventListener handler, SubscribeEvent subInfo, String readable, Type filter) {
+            super(handler, subInfo, readable, filter); //Filter can never be null in any paths we call it. But actually don't want to add a null check here because i don't want to re-do the if.
+        }
+
+        @Override
+        public void invoke(Event event) {
+            handler.invoke(event);
+        }
+
+        @Override
+        public IEventListener toCancelable() {
+            return ((IReactiveEventListener)handler).toCancelable();
+        }
     }
 }

src/main/java/net/minecraftforge/eventbus/EventBus.java

@@ -17,8 +17,6 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
-import java.util.function.Predicate;
-
 import static net.minecraftforge.eventbus.LogMarkers.EVENTBUS;
 
 public class EventBus implements IEventExceptionHandler, IEventBus {
@@ -163,18 +161,6 @@ private void registerListener(final Object target, final Method method, final Me
         register(eventType, target, real);
     }
 
-    private static final Predicate<Event> checkCancelled = e -> !e.isCanceled();
-    @SuppressWarnings("unchecked")
-    private static <T extends Event> Predicate<T> passCancelled(boolean ignored) {
-        return ignored ? null : (Predicate<T>) checkCancelled;
-    }
-
-    private static <T extends GenericEvent<? extends F>, F> Predicate<T> passGenericFilter(Class<F> type, boolean ignored) {
-        if (ignored)
-            return e -> e.getGenericType() == type;
-        return e -> e.getGenericType() == type && !e.isCanceled();
-    }
-
     private static void checkNotGeneric(final Consumer<? extends Event> consumer) {
         checkNotGeneric(getEventClass(consumer));
     }
@@ -197,13 +183,13 @@ public <T extends Event> void addListener(final EventPriority priority, final Co
     @Override
     public <T extends Event> void addListener(final EventPriority priority, final boolean receiveCancelled, final Consumer<T> consumer) {
         checkNotGeneric(consumer);
-        addListener(priority, passCancelled(receiveCancelled), consumer);
+        addListener(priority, consumer, null, receiveCancelled);
     }
 
     @Override
     public <T extends Event> void addListener(final EventPriority priority, final boolean receiveCancelled, final Class<T> eventType, final Consumer<T> consumer) {
         checkNotGeneric(eventType);
-        addListener(priority, passCancelled(receiveCancelled), eventType, consumer);
+        addListener(priority, eventType, consumer, null, receiveCancelled);
     }
 
     @Override
@@ -218,12 +204,12 @@ public <T extends GenericEvent<? extends F>, F> void addGenericListener(final Cl
 
     @Override
     public <T extends GenericEvent<? extends F>, F> void addGenericListener(final Class<F> genericClassFilter, final EventPriority priority, final boolean receiveCancelled, final Consumer<T> consumer) {
-        addListener(priority, passGenericFilter(genericClassFilter, receiveCancelled), consumer);
+        addListener(priority, consumer, genericClassFilter, receiveCancelled);
     }
 
     @Override
     public <T extends GenericEvent<? extends F>, F> void addGenericListener(final Class<F> genericClassFilter, final EventPriority priority, final boolean receiveCancelled, final Class<T> eventType, final Consumer<T> consumer) {
-        addListener(priority, passGenericFilter(genericClassFilter, receiveCancelled), eventType, consumer);
+        addListener(priority, eventType, consumer, genericClassFilter, receiveCancelled);
     }
 
     @SuppressWarnings("unchecked")
@@ -236,47 +222,61 @@ private static <T extends Event> Class<T> getEventClass(Consumer<T> consumer) {
         return eventClass;
     }
 
-    private <T extends Event> void addListener(final EventPriority priority, final Predicate<? super T> filter, final Consumer<T> consumer) {
+    private <T extends Event> void addListener(final EventPriority priority, final Consumer<T> consumer, final Class<?> genericFilter, boolean receiveCancelled) {
         Class<T> eventClass = getEventClass(consumer);
         if (Objects.equals(eventClass, Event.class))
             LOGGER.warn(EVENTBUS,"Attempting to add a Lambda listener with computed generic type of Event. " +
                     "Are you sure this is what you meant? NOTE : there are complex lambda forms where " +
                     "the generic type information is erased and cannot be recovered at runtime.");
-        addListener(priority, filter, eventClass, consumer);
+        addListener(priority, eventClass, consumer, genericFilter, receiveCancelled);
     }
 
-    private <T extends Event> void addListener(final EventPriority priority, final Predicate<? super T> filter, final Class<T> eventClass, final Consumer<T> consumer) {
+    private <T extends Event> void addListener(final EventPriority priority, final Class<T> eventClass, final Consumer<T> consumer, final Class<?> genericFilter, boolean receiveCancelled) {
         if (baseType != Event.class && !baseType.isAssignableFrom(eventClass)) {
             throw new IllegalArgumentException(
                     "Listener for event " + eventClass + " takes an argument that is not a subtype of the base type " + baseType);
         }
 
         @SuppressWarnings("unchecked")
-        IEventListener listener = Modifier.isFinal(eventClass.getModifiers()) && (filter == checkCancelled || filter == null) && !EventListenerHelper.isCancelable(eventClass)
-                ? e -> consumer.accept((T) e)
-                : e -> doCastFilter(filter, eventClass, consumer, e);
+        IEventListener listener = new IEventListener() {
+            @Override
+            public void invoke(Event event) {
+                consumer.accept((T)event);
+            }
 
-        addToListeners(consumer, eventClass, NamedEventListener.namedWrapper(listener, consumer.getClass()::getName), priority);
-    }
+            @Override
+            public String toString() {
+                return "Lambda Handler: " + consumer.toString();
+            }
+        };
 
-    @SuppressWarnings("unchecked")
-    private static <T extends Event> void doCastFilter(final Predicate<? super T> filter, final Class<T> eventClass, final Consumer<T> consumer, final Event e) {
-        T cast = (T)e;
-        if (filter == null || filter.test(cast))
-            consumer.accept(cast);
+        ListenerList listenerList = getListenerList(eventClass);
+        var cancelable = listenerList.isCancelable() || EventListenerHelper.isCancelable(eventClass);
+
+        IEventListener finalListener = ReactiveEventListener.of(listener, listener.toString(), genericFilter, receiveCancelled, cancelable);
+        addToListeners(listenerList, consumer, finalListener, priority);
     }
 
     private void register(Class<?> eventType, Object target, Method method) {
         try {
-            ASMEventHandler asm = ASMEventHandler.of(this.factory, target, method, IGenericEvent.class.isAssignableFrom(eventType));
-            addToListeners(target, eventType, asm, asm.getPriority());
+            EventPriority priority = method.getAnnotation(SubscribeEvent.class).priority();
+            ListenerList listenerList = getListenerList(eventType);
+            IEventListener asm = ASMEventHandler.of(this.factory, target, method, IGenericEvent.class.isAssignableFrom(eventType), listenerList.isCancelable());
+            addToListeners(listenerList, target, asm, priority);
         } catch (IllegalAccessException | InstantiationException | NoSuchMethodException | InvocationTargetException | ClassNotFoundException e) {
             LOGGER.error(EVENTBUS,"Error registering event handler: {} {}", eventType, method, e);
         }
     }
 
-    private void addToListeners(final Object target, final Class<?> eventType, final IEventListener listener, final EventPriority priority) {
+    private ListenerList getListenerList(Class<?> eventType) {
         ListenerList listenerList = EventListenerHelper.getListenerList(eventType);
+        if (!listenerList.isCancelable() && EventListenerHelper.isCancelable(eventType))
+            listenerList.setCancelable();
+
+        return listenerList;
+    }
+
+    private void addToListeners(final ListenerList listenerList, final Object target, final IEventListener listener, final EventPriority priority) {
         listenerList.register(busID, this, priority, listener);
         List<IEventListener> others = listeners.computeIfAbsent(target, k -> Collections.synchronizedList(new ArrayList<>()));
         others.add(listener);