RFR: 8166188: G1 Needs pre barrier on dereference of weak JNI handles

[Kim Barrett]

> On Feb 13, 2017, at 9:25 AM, Doerr, Martin <martin.doerr at sap.com> wrote:
> 
> Hi Kim,
> 
> I have factored out resolve_jobject on PPC64 and s390.
> 
> Complete webrev is here:
> http://cr.openjdk.java.net/~mdoerr/8166188_s390_ppc64/webrev.06/
> 
> Incremental patch (based on webrev.05):
> http://cr.openjdk.java.net/~mdoerr/8166188_s390_ppc64/webrev.06/8166188_s390_ppc64_incr.patch

Thanks.  I’ll add that to my patch set.

> I prefer masking out the weak tag and only checking it if G1 is used. (The additional check and branch is only needed for G1. Also, see new PPC64 and s390 implementation.)

I considered that, but chose to not penalize the G1 case (the default case now)
to perhaps slightly improve the non-G1 (non-default) case. (Branch prediction
may mitigate the extra branch cost.)  But it’s unlikely to matter either way.

> What is preferable on x86? Btw. the indentation looks inconsistent in resolve_jobject.

Thanks for noticing the mis-indentation.  Will fix.

> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: s390x-port-dev [mailto:s390x-port-dev-bounces at openjdk.java.net] On Behalf Of Kim Barrett
> Sent: Montag, 13. Februar 2017 05:26
> To: hotspot-dev developers <hotspot-dev at openjdk.java.net>
> Cc: s390x-port-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
> Subject: Re: RFR: 8166188: G1 Needs pre barrier on dereference of weak JNI handles
> 
> Here's the updated webrevs, incorporating feedback received so far.
> 
> full: http://cr.openjdk.java.net/~kbarrett/8166188/hotspot.05/
> incr: http://cr.openjdk.java.net/~kbarrett/8166188/hotspot.05.inc/
> 
> For x86 and ARM I added resolve_jobject functions to their respective
> macroAssembler files, and call those to generate the return object
> handling code.  Merging the two cases for Sparc is harder and I'm
> going to leave it to someone else.  I'm also leaving any such merging
> for other (non-Oracle) ports to their respective maintainers.
> 
> Rewrote the error checking in SignatureChekker::check_obj to simplify
> the control flow and provide more information.  Rather than
> conditionally selecting between ReportJNIFatalError or assert to
> report problems, instead just always use guarantee.  Recall that we
> only get here if CheckJNICalls or in a debug build.  This made
> SignatureChekker::_thread unused, so removed it.