[foreign-abi] Intrinsify down calls

[Vladimir Ivanov]

> For now I've reimplemented the stub lifetime management without using a 
> cleaner. As mentioned, the stubs are now attached to the nmethod, and 
> cleaned up when the nmethod is. This also means we can defer the stub 
> generation until intrinsification happens, which in itself reduces 
> pressure on the code cache.

Looks good. It solves the immediate problem with the resource management 
and I'm fine with leaving it as is for now.

But in a longer term, IMO it is still worth to consider an alternative 
approach. The best solution will depend on some implementation decisions.

Some thoughts follow.

Having a stub associated with a nmethod have some drawbacks/limitations.

For example, if you look at other MethodHandle linkers (linkToVirtual et 
al), there are 2 variants: used by compiled code and used by 
interpreter. If interpreter needs a stub, then a different solution for 
resource management will be needed for it.

Sharing stubs between nmethods/native functions also becomes more 
complicated: it requires some sort of reference counting to track usages.

Also, if sharing is not required (and all the native stubs have unique 
nmethod users), then the stubs can be embedded into the nmethod itself. 
Then the stubs will be freed along with the nmethod. (It can also 
improve situation with code cache fragmentation unless the stub is 
allocated right next to the user nmethod.)

At the moment, the most promising solution to me is to associate the JVM 
stubs with native library and free them all at once when the library is 
unloaded.

> The diff can still be found here: 
> https://github.com/openjdk/panama-foreign/compare/foreign-abi...JornVernee:Call_Intrinsics_Stubs 

I only briefly browsed the code, but overall it looks very good!

Quick question: can you remind me why is the following line is needed 
(from NativeMethodHandle.make()):

   fallback.customize(); // needed

Best regards,
Vladimir Ivanov


> On 29/04/2020 13:58, Maurizio Cimadamore wrote:
>>
>> On 29/04/2020 12:43, Jorn Vernee wrote:
>>> There might be some options for attaching the stub to the nmethod 
>>> itself, and then freeing it when the nmethod is freed. But maybe for 
>>> now we should go back to emitting the thread state transition inline, 
>>> and then improve upon this later. 
>>
>> +1
>>
>> After chatting with Jorn I'm not overly convinced of the wider 
>> advantages to split the nmethod from the thread transition code. They 
>> will have to be co-generated anyway, so I don't see an advantage in 
>> terms of reduce the amount of generated code; I also don't see an 
>> (obvious) performance edge, since in the common case the nmethod will 
>> have to call the transition stub code.
>>
>> Using a Cleaner to do this seems like the wrong tool for the job - 
>> since the Cleaner is tuned for heap allocations - and here we have to 
>> rely on it to manage the code cache, whose size is typically much 
>> smaller, and totally opaque to the cleaner anyway. So this presents 
>> all the thorny problems that direct ByteBuffer deallocation via 
>> Cleaner also presents: objects that are not deemed "very important" by 
>> the cleaner are left around hanging, and you eventually run out of 
>> space. ByteBuffers did some heroic attempts to try to kick Cleaner, 
>> and add retries with exponential backoff - but I don't think we wanna 
>> go there for such a low level API as this one?
>>
>> Cheers
>> Maurizio
>>