@Inject without object allocation

[mega12345mega]

When injecting into methods that get called a lot (in my case hundreds of thousands of times per second), a large number of CallbackInfo or CallbackInfoReturnable instances get created. Here is an example of this issue getting reported to a different project I found when researching this: FTBTeam/FTB-Mods-Issues#2012

Ideally, it would be possible to inject code that can return without allocating objects in the process. I have a couple of ideas on how this could be implemented:

Cache CallbackInfo[Returnable] objects for each thread

Mixin:

@Inject(method = "targetMethod", at = @At("HEAD"), cancellable = true, highTraffic = true) // highTraffic indicates caching should be used
private void injected(int arg, CallbackInfoReturnable<String> info) {
    if (Math.random() < 0.5)
        info.setReturnValue("foo");
}

Target:

+ private static final ThreadLocal cache_targetMethod_injected = ThreadLocal.withInitial(() -> new CallbackInfoReturnable<>("targetMethod", true));
  public String targetMethod(int arg) {
+     CallbackInfoReturnable<String> info = cache_targetMethod_injected.get();
+     info.reset(); // Sets cancelled to false, return value to null
+     injected(arg, info);
+     if (info.isCancelled())
+         return info.getReturnValue();
      ...
  }

Actually return from mixin method

Mixin:

@InjectInline(method = "targetMethod", at = @At("HEAD"))
private String injected(int arg) {
    if (Math.random() < 0.5)
        return "foo";

    // This should be stripped out of the bytecode before being injected
    // Can only show up at the end of the mixin method
    throw InjectInline.DONT_RETURN;

    // Another option
    return InjectInline.DontReturn.forReference(); // non-void methods
    InjectInline.DontReturn.forVoid(); // void methods
}

Target:

  public String targetMethod(int arg) {
+     if (Math.random() < 0.5) // Bytecode is injected into the target method directly
+         return "foo";
+     // throw is removed
      ...
  }

InjectInline:

public @interface InjectInline {
    public static final RuntimeException DONT_RETURN = null;

    // Alternatively
    public static class DontReturn {
        public static <T> T forReference() {
            return null;
        }
        public static void forVoid() {}
    }

    ...
}

[LlamaLad7]

• In many cases the JIT is able to elide the CallbackInfo allocations. You need to be careful about what kind of profilers you're using to measure allocations because many approaches can interfere with this elision.
• Even in cases where this is not possible allocation is likely to be faster than any sort of ThreadLocal lookup (allocation is normally a single pointer bump, ThreadLocals require hash table lookups).
• Cancellation is rarely needed to begin with and can cause compatibility issues, for example the mixin referenced in the linked issue should be a ModifyReturnValue which would provide better compat and avoid any sort of CallbackInfo

[LlamaLad7]

If we were to introduce a non-allocating form of cancellable injection it would be using the handler's return value, but it is impossible to solve the general case of @Cancellable CallbackInfo(Returnable) in a reasonable manner. Regardless, I do not believe it is needed as above.

[mega12345mega]

For the initial issue that was reported to me (mega12345mega/NBT-Editor#173) it mentioned a different object getting allocated a lot which I can fix. It was in the testing for that where I discovered the CallbackInfoReturnable allocations, but it is possible they are indeed optimized out, and that is why they weren't mentioned in the original issue. Note that I do use the ability to change the return value so it seems surprising to me that it would be optimizable - is there a recommend way to confirm this?

In my case, I always want to change the return values for some instances of RegistryEntry.Reference, and some of the methods I want to inject into can throw before returning, so for performance reasons (not requiring multiple mixins to avoid the throwing parts, skipping the method early) I would like to still use @Inject if possible. (I have just discovered a possible alternative I need to test that takes advantage of the fact that RegistryEntry.Reference happens to be only create-able in a few places, but not everyone may be so lucky.)

I'm curious what the underlying issues are with the second method, but there is a related solution that I forgot to include that may fix it as it moves the injected code into a separate method like normal (while being slightly grosser):

Mixin:

@InjectInline(method = "targetMethod", at = @At("HEAD"))
private Object injected(int arg) { // Returns Object instead of String; not sure if primitive boxing is a concern here
    if (Math.random() < 0.5)
        return "foo";
    return InjectInline.DONT_RETURN;
}

Target:

  public String targetMethod(int arg) {
+     Object returnValue = injected(arg);
+     if (returnValue != InjectInline.DONT_RETURN)
+         return (String) returnValue;
      ...
  }

InjectInline:

public @interface InjectInline {
    public static final Object DONT_RETURN = new Object();

    ...
}