RFR 8199868: Support JNI critical functions in object pinning API

[Ian Rogers]

Re JDK-8199868 - should pinning be limited to just critical APIs? The
non-critical variants could also pin to avoid copies.
I see this as related to JDK-8199919. It'd be nice to just move the
critical APIs to use the same logic as the non-critical APIs, then pinning
just becomes a performance optimization to avoid copying. Re JDK-8199919's
comment that there is no deprecation mechanism, compilers like GCC allow
function attributes that warn when a deprecated function is used.

Thanks,
Ian



On Wed, May 2, 2018 at 2:12 PM Per Liden <per.liden at oracle.com> 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
> >>>
>