Fabric API

Fabric API

106M Downloads

Codegen for events

yyny opened this issue ยท 10 comments

commented

Writing events is currently very verbose, requiring lots of boilerplate code.

Additionally, many events simply take the form

for (Callback callback : callbacks) {
    callback.onEvent(...);
}

See also this comment in EventFactory

commented

I don't see this happening inside Fabric API. However Architectury API has some invocation handler proxy stuff to generate simple event invokers. Can be of inspiration for anyone who dislikes the current boilerplate.

commented

I agree. It would be very helpful.

commented

We could achieve this now with an annotation processor. It might look something like this; where @GenerateSimpleEvent is applicable to types, source-only, and repeatable.

Foo.java

@GenerateSimpleEvent(name = "foo.bar.Baz.QUX")
// Requirement of GenerateSimpleEvent so the processor doesn't have to enforce the functional interface contract
@FunctionalInterface
public interface Foo
{
    public void doFoo();
}

Baz.java (generated file)

package foo.bar;

public static final Event<Foo> QUX = EventFactory.createArrayBacked(Foo.class, callbacks -> () -> 
{
    for (Foo callback : callbacks)
    {
        callback.doFoo();
    }
})

ASM generated classes are also an option, but first we'd need to add a hook in Loader that allows mods (such as FAPI) to generate classes.

commented

generating classes doesn't strictly need an api

commented

I believe we would end up doing the following in some form:

Store all event instances in an array on the event class when using a generated event. Use the classloader and define a new class for the invoker type which simply just pushes each instance on the stack and pops it off by running the callback method.

The generated code would be something like below but optimized more than the equivalent compiled code would be:

class Foo$InvokerImpl implements Foo {
    Foo$InvokerImpl(Foo foo1, Foo foo2, Foo foo3, Foo ...) { ... set instance fields ... }

    void doFoo(Bar bar) {
        this.foo1.doFoo(bar);
        this.foo2.doFoo(bar);
        this.foo3.doFoo(bar);
        ...
    }
}

That invoker impl would be set as the invoker on the event - we will only generate the invoker using asm in two cases:

First invocation of the event via a sort of factory invoker which generates the invoker class and then calls it
Reason here is to avoid generating the invoker 30 times during mod init.
Although this will make the first invocation slower always - it will be fast every time after.

Adding new callbacks after the first invocation of the event - the class is regenerated and then set
This would allow some later initialization to get in if needed.
I believe we lose only a small bit on init time since the current event impl is pretty much COW.

commented

@Devan-Kerman

generating classes doesn't strictly need an api

I know you can just create an additional classloader, but as far as I'm aware it's desirable to minimise the number of classloaders to avoid issues I don't recall? I've only done class generation once for a toy project, otherwise I've been modifying existing classes, so I'm not sure.

@maityyy

It is not necessary to generate the code, we can simply write this for the event.
InvokerFactoryFunction

for (Callback callback : callbacks) {
    callback.onEvent(...);
}

We can provide an already implemented Function for the invoker factory.

We could do something like this

public static Event<Runnable> createArrayBackedSimple() {
     return EventFactoryImpl.createArrayBacked(Runnable.class, callbacks -> () -> {
          for (Runnable callback : callbacks) {
               callback.run()
          }
     });
}

But that only allows callbacks with no arguments. We can't write a single class that's generic over any number of arguments.

commented

It is not necessary to generate the code, we can simply write this for the event.
InvokerFactoryFunction

for (Callback callback : callbacks) {
    callback.onEvent(...);
}

We can provide an already implemented Function for the invoker factory.

commented

Why can't we just use an java.lang.invoke.Proxy with an InvocationHandler for void ones?
The main concern is the dispatch performance of proxies.

Another problems is events with return types. If the callbacks return different results, you don't know if you should do tristate merging and stop short, take the max or min of int return values, or collect extra returns from the parameters (e.g. you may send in an int[] to collect extra int return values). Your suggestion to "optimize" apparently can handle none of these unfortunately.

commented

An alternative I think of would be a factory method to generate Function<T[], T>, possibly when given Class<T> and/or the method to dispatch, either as a Method or a MethodHandle. However, it is still restricted, since not all callbacks just call sub-callbacks; some have early halt or tristate merging, so the usefulness isn't as much as we would assume.

In addition, we need to check the performance of that Function to make sure it's not slower than the current lambda meta call site. Somewhere it says Proxy makes spring calls about 10% to 20% slower than direct interface calls.

commented

I think whatever form this takes should be optional - there are scenarios where you'd want something more complex than a forEach, like early-exit.