Enhance alwaysCancelling listener awareness in invoker optimisation strategy (#82)

If the event is cancellable but all listeners are consumers, the invoker optimisation strategy would previously bail out to using the wrapped predicates if alwaysCancelling listeners were present.

Now, alwaysCancelling listeners are properly accounted for in this case, combining the benefits of eliminating non-monitoring listeners after them with also unwrapping to branchless consumer calls in the built invoker.

Author: PaintNinja

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

@@ -10,12 +10,15 @@
 import net.minecraftforge.eventbus.api.event.characteristic.Cancellable;
 import net.minecraftforge.eventbus.api.event.characteristic.SelfPosting;
 import net.minecraftforge.eventbus.api.listener.EventListener;
+import net.minecraftforge.eventbus.api.listener.Priority;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Consumer;
 
 public class IndividualEventListenerTests {
     /**
@@ -105,6 +108,30 @@ record AlwaysCancellingTestEvent() implements Cancellable, RecordEvent {
         Assertions.assertFalse(wasCancelled, "The event should not have been cancelled without any listeners");
     }
 
+    @Test
+    public void testAlwaysCancellingListenerOptimisation() throws Exception{
+        record CancellableTestEvent() implements Cancellable, RecordEvent {
+            static final CancellableEventBus<CancellableTestEvent> BUS = CancellableEventBus.create(CancellableTestEvent.class);
+        }
+
+        var cancellingListener = CancellableTestEvent.BUS.addListener(Priority.HIGHEST, true, event -> {});
+        var ordinaryListener = CancellableTestEvent.BUS.addListener(event -> {});
+
+        var listeners = List.of(cancellingListener, ordinaryListener);
+
+        // create a MH to net.minecraftforge.eventbus.internal.InvokerFactoryUtils.unwrapAlwaysCancellingConsumers
+        var invokerFactoryUtilsClass = Class.forName("net.minecraftforge.eventbus.internal.InvokerFactoryUtils");
+        var method = invokerFactoryUtilsClass.getDeclaredMethod("unwrapAlwaysCancellingConsumers", List.class);
+        method.setAccessible(true);
+        @SuppressWarnings("unchecked")
+        var result = (List<Consumer<?>>) method.invoke(null, listeners);
+
+        Assertions.assertEquals(1, result.size(), "The ordinary listener should have been removed from the list");
+
+        CancellableTestEvent.BUS.removeListener(cancellingListener);
+        CancellableTestEvent.BUS.removeListener(ordinaryListener);
+    }
+
     /**
      * Tests that exceptions thrown by listeners are propagated to the poster.
      */

src/main/java/net/minecraftforge/eventbus/internal/Constants.java

@@ -19,6 +19,7 @@ private Constants() {}
 
     static final Consumer<Event> NO_OP_CONSUMER = event -> {};
     static final Predicate<Event> NO_OP_PREDICATE = event -> false;
+    static final Predicate<Event> ALWAYS_TRUE_PREDICATE = event -> true;
 
     static final MethodHandle MH_NULL_CONSUMER = MethodHandles.constant(Consumer.class, null);
     static final MethodHandle MH_NO_OP_CONSUMER = MethodHandles.constant(Consumer.class, NO_OP_CONSUMER);
@@ -60,8 +61,8 @@ static <T> Consumer<T> getNoOpConsumer() {
     }
 
     @SuppressWarnings("unchecked")
-    static <T> Predicate<T> getNoOpPredicate() {
-        return (Predicate<T>) NO_OP_PREDICATE;
+    static <T> Predicate<T> getNoOpPredicate(boolean alwaysCancelling) {
+        return (Predicate<T>) (alwaysCancelling ? ALWAYS_TRUE_PREDICATE : NO_OP_PREDICATE);
     }
 
     static boolean isSelfDestructing(int characteristics) {

src/main/java/net/minecraftforge/eventbus/internal/InvokerFactory.java

@@ -194,9 +194,12 @@ private static <T extends Event & Cancellable> Predicate<T> createCancellableInv
             // ...so annoyingly, we need to duplicate the code of createInvoker() here, but with a different primitive return type (boolean instead of void)
             // Maybe JEP 402 can save us from this workaround in the future? https://openjdk.java.net/jeps/402
 
-            // Todo: [EB][Invoker] Support alwaysCancelling listeners in cancellation check-free Consumer invokers
-            if (listeners.stream().map(EventListenerImpl.WrappedConsumerListener.class::cast).noneMatch(EventListenerImpl.WrappedConsumerListener::alwaysCancelling))
-                return createCancellableInvokerFromUnwrappedNoChecks((List<Consumer<T>>) (List) InvokerFactoryUtils.unwrapConsumers(listeners));
+            return createCancellableInvokerFromUnwrappedNoChecks(
+                    (List<Consumer<T>>) (List) InvokerFactoryUtils.unwrapAlwaysCancellingConsumers(listeners),
+                    listeners.stream()
+                            .map(EventListenerImpl.WrappedConsumerListener.class::cast)
+                            .anyMatch(EventListenerImpl.WrappedConsumerListener::alwaysCancelling)
+            );
         }
 
         return createCancellableInvokerFromUnwrapped((List<Predicate<T>>) (List) InvokerFactoryUtils.unwrapPredicates(listeners));
@@ -250,68 +253,127 @@ private static <T extends Event> Consumer<T> createInvokerFromUnwrapped(List<Con
     /**
      * Same as {@link #createInvokerFromUnwrapped(List)} but returns a {@link Predicate} instead of a {@link Consumer}.
      * <p>See the code comments inside {@link #createCancellableInvoker(List)} for an explainer as to why this exists.</p>
+     * <p>Also see {@link EventListenerImpl.WrappedConsumerListener#wrap(boolean, Consumer)} for an explainer as to why capturing the return value is avoided.</p>
      */
-    private static <T extends Event & Cancellable> Predicate<T> createCancellableInvokerFromUnwrappedNoChecks(List<Consumer<T>> listeners) {
-        return switch (listeners.size()) {
-            case 0 -> Constants.getNoOpPredicate();
-            case 1 -> {
-                var first = listeners.getFirst();
-                yield event -> {
-                    first.accept(event);
-                    return false;
-                };
-            }
-            case 2 -> {
-                var first = listeners.getFirst();
-                var second = listeners.getLast();
-                yield event -> {
-                    first.accept(event);
-                    second.accept(event);
-                    return false;
-                };
-            }
+    private static <T extends Event & Cancellable> Predicate<T> createCancellableInvokerFromUnwrappedNoChecks(List<Consumer<T>> listeners, boolean alwaysCancelling) {
+        if (alwaysCancelling) {
+            return switch (listeners.size()) {
+                case 0 -> Constants.getNoOpPredicate(true);
+                case 1 -> {
+                    var first = listeners.getFirst();
+                    yield event -> {
+                        first.accept(event);
+                        return true;
+                    };
+                }
+                case 2 -> {
+                    var first = listeners.getFirst();
+                    var second = listeners.getLast();
+                    yield event -> {
+                        first.accept(event);
+                        second.accept(event);
+                        return true;
+                    };
+                }
 
-            case 3 -> {
-                var first = listeners.getFirst(); // 0
-                var second = listeners.get(1);
-                var third = listeners.getLast(); // 2
-                yield event -> {
-                    first.accept(event);
-                    second.accept(event);
-                    third.accept(event);
-                    return false;
-                };
-            }
-            case 4 -> {
-                var first = listeners.getFirst(); // 0
-                var second = listeners.get(1);
-                var third = listeners.get(2);
-                var fourth = listeners.getLast(); // 3
-                yield event -> {
-                    first.accept(event);
-                    second.accept(event);
-                    third.accept(event);
-                    fourth.accept(event);
-                    return false;
-                };
-            }
+                case 3 -> {
+                    var first = listeners.getFirst(); // 0
+                    var second = listeners.get(1);
+                    var third = listeners.getLast(); // 2
+                    yield event -> {
+                        first.accept(event);
+                        second.accept(event);
+                        third.accept(event);
+                        return true;
+                    };
+                }
+                case 4 -> {
+                    var first = listeners.getFirst(); // 0
+                    var second = listeners.get(1);
+                    var third = listeners.get(2);
+                    var fourth = listeners.getLast(); // 3
+                    yield event -> {
+                        first.accept(event);
+                        second.accept(event);
+                        third.accept(event);
+                        fourth.accept(event);
+                        return true;
+                    };
+                }
 
-            default -> {
-                @SuppressWarnings("unchecked")
-                Consumer<T>[] listenersArray = listeners.toArray(new Consumer[0]);
-                yield event -> {
-                    for (Consumer<T> listener : listenersArray) {
-                        listener.accept(event);
-                    }
-                    return false;
-                };
-            }
-        };
+                default -> {
+                    @SuppressWarnings("unchecked")
+                    Consumer<T>[] listenersArray = listeners.toArray(new Consumer[0]);
+                    yield event -> {
+                        for (Consumer<T> listener : listenersArray) {
+                            listener.accept(event);
+                        }
+                        return true;
+                    };
+                }
+            };
+        } else {
+            return switch (listeners.size()) {
+                case 0 -> Constants.getNoOpPredicate(false);
+                case 1 -> {
+                    var first = listeners.getFirst();
+                    yield event -> {
+                        first.accept(event);
+                        return false;
+                    };
+                }
+                case 2 -> {
+                    var first = listeners.getFirst();
+                    var second = listeners.getLast();
+                    yield event -> {
+                        first.accept(event);
+                        second.accept(event);
+                        return false;
+                    };
+                }
+
+                case 3 -> {
+                    var first = listeners.getFirst(); // 0
+                    var second = listeners.get(1);
+                    var third = listeners.getLast(); // 2
+                    yield event -> {
+                        first.accept(event);
+                        second.accept(event);
+                        third.accept(event);
+                        return false;
+                    };
+                }
+                case 4 -> {
+                    var first = listeners.getFirst(); // 0
+                    var second = listeners.get(1);
+                    var third = listeners.get(2);
+                    var fourth = listeners.getLast(); // 3
+                    yield event -> {
+                        first.accept(event);
+                        second.accept(event);
+                        third.accept(event);
+                        fourth.accept(event);
+                        return false;
+                    };
+                }
+
+                default -> {
+                    @SuppressWarnings("unchecked")
+                    Consumer<T>[] listenersArray = listeners.toArray(new Consumer[0]);
+                    yield event -> {
+                        for (Consumer<T> listener : listenersArray) {
+                            listener.accept(event);
+                        }
+                        return false;
+                    };
+                }
+            };
+        }
     }
 
     private static <T extends Event & Cancellable> Predicate<T> createCancellableInvokerFromUnwrapped(List<Predicate<T>> listeners) {
         return switch (listeners.size()) {
-            case 0 -> Constants.getNoOpPredicate();
+            case 0 -> Constants.getNoOpPredicate(false);
             case 1 -> listeners.getFirst(); // Direct call
             case 2 -> {
                 var first = listeners.getFirst();

src/main/java/net/minecraftforge/eventbus/internal/InvokerFactoryUtils.java

@@ -9,10 +9,8 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
-import java.util.stream.Collectors;
 
 final class InvokerFactoryUtils {
     private InvokerFactoryUtils() {}
@@ -30,11 +28,29 @@ static <T> List<Consumer<T>> unwrapConsumers(List<EventListener> listeners) {
                 .toList();
     }
 
+    static <T> List<Consumer<T>> unwrapAlwaysCancellingConsumers(List<EventListener> listeners) {
+        var unwrappedConsumers = new ArrayList<Consumer<T>>(listeners.size());
+        for (var listener : listeners) {
+            if (listener instanceof EventListenerImpl.HasConsumer<?> consumerListener) {
+                unwrappedConsumers.add(uncheckedCast(consumerListener.consumer()));
+            } else {
+                throw new IllegalStateException("Unexpected listener type: " + listener.getClass());
+            }
+
+            if (listener instanceof EventListenerImpl.WrappedConsumerListener wrappedConsumerListener
+                    && wrappedConsumerListener.alwaysCancelling()) {
+                unwrappedConsumers.trimToSize();
+                break;
+            }
+        }
+        return unwrappedConsumers;
+    }
+
     static <T> List<Predicate<T>> unwrapPredicates(List<EventListener> listeners) {
         var unwrappedPredicates = new ArrayList<Predicate<T>>(listeners.size());
         for (var listener : listeners) {
             if (listener instanceof EventListenerImpl.HasPredicate<?> predicateListener) {
-                unwrappedPredicates.add(InvokerFactoryUtils.uncheckedCast(predicateListener.predicate()));
+                unwrappedPredicates.add(uncheckedCast(predicateListener.predicate()));
             } else {
                 throw new IllegalStateException("Unexpected listener type: " + listener.getClass());
             }