RFR: JDK-8199620: Support for JNI object pinning

[Per Liden]

Hi Roman,

On 03/29/2018 12:31 AM, Roman Kennke wrote:
> BTW, we do have a working prototype of 'JNI lazy critical functions' or
> whatever it is called in Shenandoah now, using pinning:
> 
> http://hg.openjdk.java.net/shenandoah/jdk/rev/232d819bc452
> 
> It looks like it could use the same API that I proposed (pin_object() /
> unpin_object()), plus an extra method to indicate if the GC would like
> to use it, and generate the corresponding stub code.
> 
> I came to think that it was an unfortunate decision to put the GCLocker
> stuff in pin_object() / unpin_object(). I think I outlined the reasons
> elsewhere in this thread, it basically boils down to what I consider
> unnecessary coupling of the two possible ways to implement JNI critical
> fluff.

The main objection people had with your original patch was that it both 
grabbed the GCLocker _and_ potentially pinned the object. I think that 
was perceived as both unnecessary and confusing. However, the design we 
have now isn't set in stone by any means. How about changing it to this:

http://cr.openjdk.java.net/~pliden/supports_object_pinning/webrev.0

This gives you the ability to ask the collector if pinning is supported, 
and then choose what to do. I think this should cover the need you have, 
right?

I'm currently running it though testing.

/Per

> 
> Roman
> 
> 
>> This looks much better, thanks for fixing. Reviewed.
>>
>> Btw, as Erik mentioned, there are JavaCritical_ libraries out there on
>> many other platforms (I've seen performance critical stuff like OpenGL
>> library bindings using this... anyway, Shenandoah can opt-out from that)
>>
>> cheers,
>> /Per
>>
>> On 03/19/2018 06:07 PM, Roman Kennke wrote:
>>> Hi Erik,
>>>
>>> thanks for reviewing!
>>>
>>> I can do that, but a GC would get the opportunity to do both, or one or
>>> the other in any case. As it stands, the JNI side of the GCLocker
>>> protocol is always executed. If a GC decides to participate in GCLocker
>>> protocol, by calling GCLocker::check_active_before_gc() and related API,
>>> then will get JNI Critical function support, otherwise GCLocker is
>>> basically counting up and down counters.
>>>
>>> If it instead wishes to use pinning and ignore JNI critical functions,
>>> then it only needs to override the pinning methods and not participate
>>> in GCLocker, and everything would be fine.
>>>
>>> However, I don't mind either way, so here's the alternative:
>>> Diff:
>>> http://cr.openjdk.java.net/~rkennke/8199620/webrev.02.diff/
>>> Full:
>>> http://cr.openjdk.java.net/~rkennke/8199620/webrev.02/
>>>
>>> Better?
>>>
>>> Thanks,
>>> Roman
>>>
>>>> Would there be a problem for you to move the GCLocker stuff into the
>>>> default implementation of object pinning?
>>>> I have seen code in the wild (not just Solaris) outside of hotspot that
>>>> exploits critical native. I would feel less worried if a given GC is not
>>>> forced to choose between completely ignoring the GC locking protocol
>>>> (and hence breaking such applications), or only locking.
>>>>
>>>> Thanks,
>>>> /Erik
>>>>
>>>> On 2018-03-19 15:23, Roman Kennke wrote:
>>>>> Am 14.03.2018 um 19:13 schrieb Roman Kennke:
>>>>>> Currently, the Get/Release*Critical() family of functions use the
>>>>>> GCLocker protocol to ensure that no JNI critical arrays are in use
>>>>>> when
>>>>>> a GC pause is entered.
>>>>>>
>>>>>> Some GCs may instead want to use object pinning and guarantee that the
>>>>>> object does not move. For example, this is easy to do with
>>>>>> region-based
>>>>>> GCs (G1, Shenandoah, ZGC) by simply not including regions with pinned
>>>>>> objects in the collection set.
>>>>>>
>>>>>> The implementation/API that I'm proposing is fairly simple: add two
>>>>>> methods oop pin_object(oop) and void unpin_object(oop) to
>>>>>> CollectedHeap,
>>>>>> and call them from JNI's Get/Release*Critical methods.
>>>>>>
>>>>>> This approach has been working perfectly fine since a long time in
>>>>>> Shenandoah.
>>>>>>
>>>>>> Bug:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8199620
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~rkennke/8199620/webrev.00/
>>>>>>
>>>>>>
>>>>> Ping? Any takers for this?
>>>>>
>>>>> Roman
>>>>>
>>>>
>>>
>>>
> 
>