RFR: 7903649: Field and global variables of array type should have indexed accessors

[Maurizio Cimadamore]

On Tue, 30 Jan 2024 15:42:03 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>>> we could settle by having a single extra layout accessor/descriptor accessor per var/field/function
>> 
>> Although, I think I like the idea of exposing multiple related constants under a single umbrella - given that in most cases we generate the holder class anyway. In many cases there seem to be multiple interesting constants for a given symbol - and if we were to expose different constant accessors, the constant accessor would quickly outnumber the _real_ API point, which would then lead us back in a situation of poor API discoverability.
>> 
>> In other words, while reserving the "good name" to the common API points remains a good move, if such good names are buried under several "less than good names", the API quickly becomes bloated, which is still problematic, unfortunately.
>
> I've put together another branch where I fully explored the idea of exposing constants (for global vars, functions and struct fields) in a separate holder class:
> 
> https://github.com/openjdk/jextract/compare/panama...mcimadamore:jextract:public_holders?expand=1
> 
> For globals and functions, not much changes (as we needed an holder class even before this).
> 
> For structs, additional holder classes are generated (one per field). For instance, here's what `Point` looks like:
> 
> 
> /**
>  * {@snippet lang=c :
>  * struct Point {
>  *     int x;
>  *     int y;
>  * }
>  * }
>  */
> public class Point {
> 
>     Point() {
>         // Should not be called directly
>     }
> 
>     private static final GroupLayout $LAYOUT = MemoryLayout.structLayout(
>         foo_h.C_INT.withName("x"),
>         foo_h.C_INT.withName("y")
>     ).withName("Point");
> 
>     /**
>      * The layout of this struct
>      */
>     public static final GroupLayout layout() {
>         return $LAYOUT;
>     }
> 
>     public static class x$constants {
>         public static final OfInt LAYOUT = (OfInt)layout().select(groupElement("x"));
>         public static final long OFFSET = layout().byteOffset(groupElement("x"));
>     }
> 
>     /**
>      * Getter for field:
>      * {@snippet lang=c :
>      * int x
>      * }
>      */
>     public static int x(MemorySegment struct) {
>         return struct.get(x$constants.LAYOUT, x$constants.OFFSET);
>     }
> 
>     /**
>      * Setter for field:
>      * {@snippet lang=c :
>      * int x
>      * }
>      */
>     public static void x(MemorySegment struct, int fieldValue) {
>         struct.set(x$constants.LAYOUT, x$constants.OFFSET, fieldValue);
>     }
> 
>     public static class y$constants {
>         public static final OfInt LAYOUT = (OfInt)layout().select(groupElement("y"));
>         public static final long OFFSET = layout().byteOffset(groupElement("y"));
>     }
> 
>     /**
>      * Getter for field:
>      * {@snippet lang=c :
>      * int y
>      * }
>      */
>     public static int y(MemorySegment struct) {
>         return struct.get(y$constants.LAYOUT, y$constants.OFFSET);
>     }
> 
>     /**
>      * Setter for field:
>      * {@snippet lang=c :
>      * int y
>      * }
>      */
>     public static void y(MemorySegment struct, int fieldValue) {
>         struct.set(y$constants.LAYOUT, y$constants.OFFSET, fieldValue);
>     }
> 
>     /**
>      * Obtains a slice of {@code arrayParam} which selects the array element at {@code index}.
>      * The returned segment has address {@code arrayParam.address() + index * layout().byteSize()}
>      */
>     public static MemorySegment asSl...

The above approach can be dialled down in a number of ways:
* maybe a `jextract` option can make all these holder classes "private", so that they don't show up in the API
* for structs, we might only generate holder classes for array fields (on the basis that other info is quickly obtainable when looking at the struct layout).

Stepping back - I think what we need to ask ourselves is whether this incremental complexity of the generated code is worth the added benefits (e.g. of providing more direct access to constants that might be required from the generated code). It might also be useful to take some footprint measurements before/after. It might well be that the extra holder class doesn't matter much in terms of overall bytes of the compiled code. While it's true that there's more classes to load, it's also true that this approach makes everything more lazy - e.g. so that an offset, or a var handle is only computed if the corresponding field is accessed by a client. The two things might well cancel each other out.

-------------

PR Review Comment: https://git.openjdk.org/jextract/pull/198#discussion_r1471485065