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

Reply via email to