[foreign-memaccess+abi] RFR: 8308293: A linker should expose the layouts it supports

[Maurizio Cimadamore]

On Thu, 15 Jun 2023 14:11:02 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> This is another stab at https://github.com/openjdk/jdk/pull/14037
>> 
>> I believe, after some offline discussion, that we have found a more satisfying solution to the problem of JAVA_CHAR being exposed. Jorn suggested that linkers should also provide mappings for JNI types such as `jint`, `jshort` and such (which are aliases for our layout constants anyway). I think that's a great way to bring `JAVA_CHAR` back into the fold.
>> 
>> For now, I decided not to specify support for JNI canonical layouts (but I could do so, if that's preferred). I think the highest priority is to provide some stable mappings for C builtin types (+ `size_t`) as that's what 99% of developers will be struggling with.
>> 
>> API-wise, we just expose a map. In the preovious PR there were questions as to whether the map should be split into two methods. In general I see the following options:
>> 
>> 1. Just expose a map (that's the primitive, other things can derived from it)
>> 2. Expose a map, plus a method to get a canonical layout from a type name (that's the `Charset` approach, which has both `availableCharsets` *and* `forName`)
>> 3. Expose a method to get canonical layout from name, plus a method that returns the set of supported canonical layout names
>> 
>> My (not so strong) preference would be for either (1) or (2).
>
> src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java line 142:
> 
>> 140:     @Override
>> 141:     public Map<String, MemoryLayout> canonicalLayouts() {
>> 142:         return CANONICAL_LAYOUTS_MAP;
> 
> I would prefer each linker to define it's own `static final` map, and implement canonicalLayouts() individually, rather than having a bunch of selection logic here in `AbstractLinker`. Since the mappings are ABI specific, and the linkers implement the ABI specific code, I think that is where this belongs.
> 
> Since most of the layouts are the same, I suggest adding a common factory method to SharedUtils with parameters for the layouts that change. (In practice we only use a single ABI per process, so we should only create the map once, and C2 should be able to fold away the virtual call).

I'm not too sure on what this shared factories would look like. In the general case (e.g. fallback) it would have at least to specify the size of short/int/long/long long. Plus possibly, size_t. Basically everything besides float/double/char/bool. I'm not sure that would end up simpler than just having duplicated code with the layout creation?

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

PR Review Comment: https://git.openjdk.org/panama-foreign/pull/839#discussion_r1231240413