My two cents: table this as an interesting exercise. There may be some merit in an immutable list factory, but the payoff for record types is not that much. If you want to try some smaller optimizations in the bytecode generation then you'll start to get some perspective on how easy/hard it can be to translate AST into bytecode. Some of the sub-optimal things we do today are because it makes it simpler to compose certain expression / statement sequences.
There is room for improvement in our bytecode, for sure. It just happens to be the part of the code that is most full of pitfalls and weirdness and requires a very steady hand and a lot of testing and sometimes trial and error. Every time I think I have kept the CompileStack and the OperandStack and some of the other data structures updated correctly, I find I missed something. Just a word of caution, I guess. From: Oscar N <paintninjaproje...@outlook.com> Sent: Monday, January 22, 2024 1:56 PM To: dev@groovy.apache.org Subject: [EXT] Re: Questions regarding implementation approach for GROOVY-11294 External Email: Use caution with links and attachments. 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)`. You can find my current implementation here: PaintNinja/groovy at GROOVY-11294-record-immutable-tolist (github.com)<https://urldefense.com/v3/__https:/github.com/PaintNinja/groovy/tree/GROOVY-11294-record-immutable-tolist__;!!GFN0sa3rsbfR8OLyAw!aQDiu_1w2KbRw1H6JgBGUNQqFDFp5OFwGC_-Ucg93kMD7llsCq77Q-Sl-IK46K1yLM_Xl-rg55WpvazueOknF_skqjwvI2kQLRk$> 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? 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. ________________________________ From: Milles, Eric (TR Technology) via dev <dev@groovy.apache.org<mailto:dev@groovy.apache.org>> Sent: 22 January 2024 18:04 To: dev@groovy.apache.org<mailto:dev@groovy.apache.org> <dev@groovy.apache.org<mailto:dev@groovy.apache.org>> Subject: RE: Questions regarding implementation approach for GROOVY-11294 The exercise is an interesting one: how to add support for something like `List.of(...)` for Java 8-21 when the method exists only for 9+ and some behavior you seek in only in 16+. However, is it worthwhile in this case to add new `ScriptBytecodeAdapter` methods to support this for records instead of just writing the bytecode you need based on the compile-time target bytecode version?
