RFR 8199868: Support JNI critical functions in object pinning API

[Roman Kennke]

I am not an expert in this particular code, but know assembly well. The
implementation looks good to me.

What happens when JNI critical functions are encountered on other
platforms? Will it fail gracefully or fall back to something else? Would
(for example) Shenandoah have to turn off JNI criticals on such
platforms until support gets implemented?

Also, as you mentioned, it baked quite a lot in Shenandoah already, and
passes our tests that stress this code.

Thank you!
Roman

> Ping!!!! anyone? please!
> 
> - Zhengyu
> 
> On 05/02/2018 05:12 PM, Per Liden wrote:
>> Hi,
>>
>> On 05/02/2018 09:41 PM, Zhengyu Gu wrote:
>>> Hi,
>>>
>>> Can I have reviews for this RFR?
>>>
>>> This patch completes object pinning for JNI critical section,
>>> provides critical native support.
>>>
>>> The approach is quite straightforward:
>>>
>>> During generating native wrapper for critical native method, it
>>> generates runtime call to pin every array argument, before unpacks them.
>>>
>>> For pinned objects, it also needs to save them for unpinning after
>>> JNI function call completes.
>>>
>>> If argument is passed on stack, it saves pinned object at the
>>> original slot (as pin_object() may move the object). For register
>>> based arguments, it reuses oop handle area (where GCLocker based
>>> implementation saves register based arguments for safepoints).
>>>
>>> Currently, only Shenandoah uses object pinning for JNI critical
>>> section, this patch has been baked quite some time there. However, I
>>> am new to Runtime Stub code, I would appreciate your comments and
>>> suggestions.
>>>
>>> I rebased patch to jdk/jdk repo.
>>>
>>> Webrev: http://cr.openjdk.java.net/~zgu/8199868/webrev.02/
>>
>> Just want to say that I would really like to see this patch go in. As
>> mentioned, it completes the object pinning story and it's useful for
>> other GCs too (at least ZGC and possibly G1). However, I also agree
>> with Aleksey that some one who really knows this code needs to review
>> this. Unfortunately that's not me. Anyone?
>>
>> cheers,
>> Per
>>
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>>
>>> On 04/06/2018 10:35 PM, Zhengyu Gu wrote:
>>>> Offline discussion with Aleksey, he suggested that
>>>> pin/unpin_critical_native_array methods can be made more generic as
>>>> pin/unpin_object.
>>>>
>>>> Updated webrev: http://cr.openjdk.java.net/~zgu/8199868/webrev.01/
>>>>
>>>> Test:
>>>>    Reran all tests, submit-hs tests still clean.
>>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu
>>>>
>>>> On 04/06/2018 08:55 AM, Aleksey Shipilev wrote:
>>>>> On 04/04/2018 07:47 PM, Zhengyu Gu wrote:
>>>>>> Please review this patch that adds JNI critical native support to
>>>>>> object pinning.
>>>>>>
>>>>>> Shenandoah does not block GC while JNI critical session is in
>>>>>> progress. This patch allows it to pin
>>>>>> all incoming array objects before critical native call, and unpin
>>>>>> them after call completes.
>>>>>>
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8199868
>>>>>> Webrev: http://cr.openjdk.java.net/~zgu/8199868/webrev.00/
>>>>>
>>>>> Looks good to me, but somebody more savvy with runtime stub
>>>>> generation should take a closer look.
>>>>>
>>>>> *) Should probably be "Why we are here?"
>>>>>
>>>>> 2867   assert(Universe::heap()->supports_object_pinning(), "Why we
>>>>> here?");
>>>>>
>>>>> 2876   assert(Universe::heap()->supports_object_pinning(), "Why we
>>>>> here?");
>>>>>
>>>>>
>>>>> Thanks,
>>>>> -Aleksey
>>>>>