[foreign-preview] RFR: 8278414: Replace binding recipe customization using MH combinators with bytecode spinning

[Jorn Vernee]

On Mon, 31 Jan 2022 19:31:41 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Hi,
>> 
>> This patch removes the binding recipe specialization using MethodHandle combinators in the linker implementation, and replaces it with specialization by spinning a class using the ASM bytecode API.
>> 
>> The main reason for doing this is simplicity of the implementation, as well as finer-grained control over the classes that are generated.
>> 
>> The meat of this PR is in `jdk.internal.foreign.abi.Specializer`, but there are some spin-off changes in this PR as well:
>> - MethodHandles are really good at sharing classes, so to get comparable performance, this PR also adds a cache behind CLinker::downcallHandle. Due to this, I've also created an AbstractLinker base class for all linker implementations, which contains the cache as well as other shared code.
>> - CallingSequence is used to steer the class generation. I've added some predicates to this class to check whether the CS is for downcall or upcall. As well as splitting the methodType() accessor into callerMethodType() and calleeMethodType(), reflecting the 2 method types in play during generation (one of the 2 was being computed ad-hoc during specialization previously). This is also responsible for most of the test changes. I've added some javadoc to CS which hopefully adequately explains these accessors.
>> - I've slightly changed the toString output of ValueLayout to include carrier types as well, in order to be able to use FunctionDescriptor.toString() as a file name in case we want to dump the generated classes to disc. This avoids conflicts between e.g. long and double, which previously were both stringified as b64.
>> 
>> Thanks,
>> Jorn
>
> src/java.base/share/classes/java/lang/foreign/ValueLayout.java line 110:
> 
>> 108:                 order == ByteOrder.BIG_ENDIAN ? "B" : "b",
>> 109:                 bitSize(),
>> 110:                 carrier == MemoryAddress.class ? "MA" : carrier.descriptorString()));
> 
> I have thought separately about that. I think I'd like to change this so that we drop the `b` and we replace it with a one letter descriptor (as per descriptor string), and use `a` for `MemoryAddress`. Then, as for `b` we use upper case for bigg-endian, and lower-case for little endian. That way the toString representation is compact in all cases (while with this patch it blows up on MemoryAddress).

Good suggestion. I went with this scheme since it seemed more extensible if we wanted to add more carriers in the future. But, it's not like the toString format is specified, so I guess we can change it up later if needed.

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

PR: https://git.openjdk.java.net/panama-foreign/pull/635