LlamaLad7 / MixinExtras

Companion library to SpongePowered Mixin with many custom injectors for a more expressive experience.
MIT License
293 stars 16 forks source link

[Suggestion] Model Operation passed to WrapOperation with MethodHandle as well #86

Closed liach closed 3 weeks ago

liach commented 3 weeks ago

An advantage of MethodHandle is that java compiler can generate any invocation method descriptor for invokeExact and invoke when they are surrounded by casts. This allows the call type (as from the NameAndType CP entry) to be preserved in the bytecode so it's easier for bytecode transfromation to substitute a call to MethodHandle (just call and then drop MH on stack), or the bytecode generator can pass actual MethodHandles loaded from constant pool, which is still more friendly to JIT compilation than Operation objects.

In contrast, the existing Operation takes a varargs array; though this looks simple in usage, it actually incurs extra costs in actual calls with varargs array allocations and type conversions, which is also complex and error-prone on the implementation side. https://github.com/LlamaLad7/MixinExtras/blob/b186f229cf76b69148d4e27902f9c4472cedb9af/src/main/java/com/llamalad7/mixinextras/utils/OperationUtils.java#L56

LlamaLad7 commented 3 weeks ago

I have already confirmed that in hot code the JIT can see through many layers of wrappers and remove the boxing, arrays and lambdas entirely. As such, handles would offer no performance benefit and would be much less ergonomic to use.

liach commented 3 weeks ago

Well, if you write if (1 == 2) or an empty loop, JIT profiling can see through it too as long as the bytecode paths aren't shared and your methods are below FreqInlineSize. (The operation object is constant from a callsite, which plays a role too) The actual benefits here is the simpler generation.

  1. If handles are actually created, they can be created as easy as an ldc instruction.
  2. If they serve as compiler hint shadows, the invokevirtual invoke/invokeExact calls in injectors are much easier to transform to actual method calls: the type is pretty close, everything on the stack can be passed to the call as-is; you just need to pop the handle from the stack after returning.
  3. These operation calls' types can be checked at compile time instead of only at runtime, which is faster feedback for developers. (Note that the argument type are already specified in the wrap handler parameters, so passing them to the handle without casting would infer the correct parameter types for the java compiler)

Of course, since you still have to support the existing Operation interface, I would totally understand if you think the maintenance cost of the new feature outweighs the benefit.

LlamaLad7 commented 3 weeks ago

All in all, I don't really see any benefits whatsoever, along with many drawbacks.