RFR: 8254231: Implementation of Foreign Linker API (Incubator) [v4]

[David Holmes]

Hi Jorn,

I'm not reviewing this but this exchange caught my attention ...

On 16/10/2020 9:15 pm, Jorn Vernee wrote:
> On Thu, 15 Oct 2020 22:42:49 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
> 
>>> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
>>>
>>>    Re-add file erroneously deleted (detected as rename)
>>
>> src/hotspot/cpu/x86/foreign_globals_x86.cpp line 56:
>>
>>> 54: }
>>> 55:
>>> 56: const ABIDescriptor parseABIDescriptor(JNIEnv* env, jobject jabi) {
>>
>> I don't know if you care about performance but of these env->calls transition into the VM and back out again.  You
>> should prefix all the code that comes from java to native with JNI_ENTRY and just use native JVM code to implement
>> these.
> 
> Currently this is prefixed with `JVM_ENTRY` e.g. like:
> JVM_ENTRY(jlong, PI_generateAdapter(JNIEnv* env, jclass _unused, jobject abi, jobject layout))
>    {
>      ThreadToNativeFromVM ttnfvm(thread);
>      return ProgrammableInvoker::generate_adapter(env, abi, layout);
>    }
> JVM_END
> (where `generate_adapter` ends up calling `parseABIDescriptor`)
> 
> JVM_ENTYRY seems to be mostly the same except for JNI_ENTRY having a `WeakPreserverExceptionMark` as well. Do we need
> to switch these? Also, I guess if we want to use VM code directly, we should get rid of the `ThreadToNativeFromVM` RAII
> handle.

Why are you going from native to VM to native again with this code? You 
would use a JNI/JVM_ENTRY because you have to execute VM runtime code. 
But your code immediately switches back to native and doesn't execute 
any VM runtime code (other than that involved in the transition logic 
itself). ??

Cheers,
David

> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/634
>