RFR 8199868: Support JNI critical functions in object pinning API

[Zhengyu Gu]

Hi,

Epsilon just became the first GC to support object pinning!
(https://bugs.openjdk.java.net/browse/JDK-8207056)

I added test cases for Epsilon to exercise this patch.

Webrev: http://cr.openjdk.java.net/~zgu/8199868/webrev.03/index.html

Test:

   gc/epsilon on Linux 64 (fastdebug + release)

Thanks,

-Zhengyu


On 07/05/2018 02:37 PM, Roman Kennke wrote:
> 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
>>>>>>
> 
>