Hi Oscar, sorry for replying so late, I was very busy this week.
On 22.01.24 20:55, Oscar N wrote:
Indeed, that is the crux of my earlier questions. I've got it working at runtime, however I'm unsure of which approach to take for compile-time. Adding new `ScriptBytecodeAdapter`/`InvokerHelper` methods that call a `VMPlugin` method allows for optimising other scenarios, such as `DefaultGroovyMethods.asImmutable(List)`.
InvokerHelper normally contains methods for the users of the language to help the with mostly invocation related issues like invoking a method in a dynamic way or such. There is more in there, but usually we only add if really really needed. ScriptBytecodeAdapter (SBA) is for helper code directly called from the bytecode and tailored to the needs of those calls only. Used mostly if too complex to compile directly to bytecode or if further helper structures like the VMPlugin are required. Here again the rule is trying to avoid using it, but the VMPlugin is not for direct calling, so if you do like you did in your started implementation, then going through SBA looks to me like the right way. DefaultGroovyMethods are methods made available by the runtime for method calls. DefaultGroovyMethods.asImmutable(List) would mean List#asImmutable() ah yes.. and VMPlugin should not call into SBA or InvokerHelper.
You can find my current implementation here: PaintNinja/groovy at GROOVY-11294-record-immutable-tolist (github.com) <https://github.com/PaintNinja/groovy/tree/GROOVY-11294-record-immutable-tolist>
well.. I suggested some changes above already. In the end though I second Miles and wonder if it is worth it.
As for compile-time optimisations, I'm not very familiar with Groovy's compiler internals. What would be the most suitable place to do JVM version-specific optimisations?
For bytecode generation we ask the supported version in the compiler config. For the runtime VMPlugin is the right place.
Note that the exact type of immutable list used also varies on the nullity of items and the length of the array, so for optimal performance it's unfortunately not as simple as one method for all lists on one Java version and another method on a different version.
I actually now wonder why you do Arrays.stream(values).toList(); in the Java16 version. Would List.of be better if all elements are not null? If yes, then why not check them? And why go with List#of and catch-NPE instead of stream before? bye Jochen