Skip to content

Commit e22d8c1

Browse files
authored
Strip redundant wrapped post-time listener checks (#85)
Further improves the performance of posting events that are final, non-`@Cancelable` and non-generic.
1 parent c110475 commit e22d8c1

6 files changed

Lines changed: 52 additions & 11 deletions

File tree

eventbus-test-jar/src/main/java/net/minecraftforge/eventbus/testjar/events/CancelableEvent.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@
99
import net.minecraftforge.eventbus.api.Event;
1010

1111
@Cancelable
12-
public class CancelableEvent extends Event { }
12+
public final class CancelableEvent extends Event {}

eventbus-test-jar/src/main/java/net/minecraftforge/eventbus/testjar/events/EventWithData.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import net.minecraftforge.eventbus.api.Event;
99

10-
public class EventWithData extends Event {
10+
public final class EventWithData extends Event {
1111
private final String data;
1212
private final int foo;
1313
private final boolean bar;

eventbus-test-jar/src/main/java/net/minecraftforge/eventbus/testjar/events/ResultEvent.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@
88
import net.minecraftforge.eventbus.api.Event;
99

1010
@Event.HasResult
11-
public class ResultEvent extends Event { }
11+
public final class ResultEvent extends Event {}

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

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,23 @@
1010
import static org.objectweb.asm.Type.getMethodDescriptor;
1111

1212
public class ASMEventHandler implements IEventListener {
13-
private final IEventListener handler;
13+
protected final IEventListener handler;
1414
private final SubscribeEvent subInfo;
1515
private final String readable;
1616
private final Type filter;
1717

18+
/**
19+
* @deprecated Use {@link #of(IEventListenerFactory, Object, Method, boolean)} instead for better performance.
20+
*/
21+
@Deprecated
1822
public ASMEventHandler(IEventListenerFactory factory, Object target, Method method, boolean isGeneric) throws IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException, ClassNotFoundException {
23+
this(factory, target, method, isGeneric, method.getAnnotation(SubscribeEvent.class));
24+
}
25+
26+
private ASMEventHandler(IEventListenerFactory factory, Object target, Method method, boolean isGeneric, SubscribeEvent subInfo) throws IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException, ClassNotFoundException {
1927
handler = factory.create(method, target);
2028

21-
subInfo = method.getAnnotation(SubscribeEvent.class);
29+
this.subInfo = subInfo;
2230
readable = "ASM: " + target + " " + method.getName() + getMethodDescriptor(method);
2331
Type filter = null;
2432
if (isGeneric) {
@@ -54,4 +62,30 @@ public EventPriority getPriority() {
5462
public String toString() {
5563
return readable;
5664
}
65+
66+
/**
67+
* Creates a new ASMEventHandler instance, factoring in a time-shifting optimisation.
68+
*
69+
* <p>In the case that no post-time checks are needed, an anonymous subclass instance will be returned that calls
70+
* the listener without additional redundant checks.</p>
71+
*
72+
* @implNote The 'all or nothing' nature of the post-time checks is to reduce the likelihood of megamorphic method
73+
* invocation, which isn't as performant as monomorphic or bimorphic calls in Java 16
74+
* (what EventBus 6.2.x targets).
75+
*/
76+
public static ASMEventHandler of(IEventListenerFactory factory, Object target, Method method, boolean isGeneric) throws IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException, ClassNotFoundException {
77+
var subInfo = method.getAnnotation(SubscribeEvent.class);
78+
assert subInfo != null;
79+
var eventType = method.getParameterTypes()[0];
80+
if (isGeneric || !Modifier.isFinal(eventType.getModifiers()) || EventListenerHelper.isCancelable(eventType))
81+
return new ASMEventHandler(factory, target, method, isGeneric, subInfo);
82+
83+
// If we get to this point, no post-time checks are needed, so strip them out
84+
return new ASMEventHandler(factory, target, method, false, subInfo) {
85+
@Override
86+
public void invoke(Event event) {
87+
handler.invoke(event);
88+
}
89+
};
90+
}
5791
}

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ else if (method.getDeclaringClass() != Object.class) // No need to check Object,
110110
}
111111
}
112112

113-
private void parentTypes(Set<Class<?>> classes, Stack<Class<?>> stack, Class<?> cls) {
113+
private static void parentTypes(Set<Class<?>> classes, Stack<Class<?>> stack, Class<?> cls) {
114114
for (var inf : cls.getInterfaces()) {
115115
if (classes.add(inf))
116116
stack.push(inf);
@@ -166,7 +166,7 @@ private void registerListener(final Object target, final Method method, final Me
166166
private static final Predicate<Event> checkCancelled = e -> !e.isCanceled();
167167
@SuppressWarnings("unchecked")
168168
private static <T extends Event> Predicate<T> passCancelled(boolean ignored) {
169-
return ignored ? null : (Predicate<T>)checkCancelled;
169+
return ignored ? null : (Predicate<T>) checkCancelled;
170170
}
171171

172172
private static <T extends GenericEvent<? extends F>, F> Predicate<T> passGenericFilter(Class<F> type, boolean ignored) {
@@ -250,7 +250,13 @@ private <T extends Event> void addListener(final EventPriority priority, final P
250250
throw new IllegalArgumentException(
251251
"Listener for event " + eventClass + " takes an argument that is not a subtype of the base type " + baseType);
252252
}
253-
addToListeners(consumer, eventClass, NamedEventListener.namedWrapper(e-> doCastFilter(filter, eventClass, consumer, e), consumer.getClass()::getName), priority);
253+
254+
@SuppressWarnings("unchecked")
255+
IEventListener listener = Modifier.isFinal(eventClass.getModifiers()) && (filter == checkCancelled || filter == null) && !EventListenerHelper.isCancelable(eventClass)
256+
? e -> consumer.accept((T) e)
257+
: e -> doCastFilter(filter, eventClass, consumer, e);
258+
259+
addToListeners(consumer, eventClass, NamedEventListener.namedWrapper(listener, consumer.getClass()::getName), priority);
254260
}
255261

256262
@SuppressWarnings("unchecked")
@@ -262,8 +268,7 @@ private static <T extends Event> void doCastFilter(final Predicate<? super T> fi
262268

263269
private void register(Class<?> eventType, Object target, Method method) {
264270
try {
265-
final ASMEventHandler asm = new ASMEventHandler(this.factory, target, method, IGenericEvent.class.isAssignableFrom(eventType));
266-
271+
ASMEventHandler asm = ASMEventHandler.of(this.factory, target, method, IGenericEvent.class.isAssignableFrom(eventType));
267272
addToListeners(target, eventType, asm, asm.getPriority());
268273
} catch (IllegalAccessException | InstantiationException | NoSuchMethodException | InvocationTargetException | ClassNotFoundException e) {
269274
LOGGER.error(EVENTBUS,"Error registering event handler: {} {}", eventType, method, e);

src/main/java/net/minecraftforge/eventbus/api/EventListenerHelper.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import net.minecraftforge.eventbus.ListenerList;
88
import net.minecraftforge.eventbus.api.Event.HasResult;
99
import net.minecraftforge.eventbus.internal.Cache;
10+
import org.jetbrains.annotations.ApiStatus;
1011

1112
import java.lang.annotation.Annotation;
1213
import java.lang.reflect.Constructor;
@@ -63,7 +64,8 @@ private static ListenerList computeListenerList(Class<?> eventClass, boolean fro
6364
}
6465
}
6566

66-
static boolean isCancelable(Class<?> eventClass) {
67+
@ApiStatus.Internal
68+
public static boolean isCancelable(Class<?> eventClass) {
6769
return hasAnnotation(eventClass, Cancelable.class, cancelable);
6870
}
6971

0 commit comments

Comments
 (0)