RFR: 8178813: Add test for G1 pre-barrier on dereference of weak JNI handles

[Kim Barrett]

> On May 9, 2017, at 4:34 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi Kim,
> 
> On Tue, 2017-05-09 at 10:08 +0200, Mikael Gerdin wrote:
>> Hi Kim,
>> [...]
>> On 2017-05-09 07:46, Kim Barrett wrote:
>>> [...]
>>> full: http://cr.openjdk.java.net/~kbarrett/8178813/hotspot.01/
>>> incr: http://cr.openjdk.java.net/~kbarrett/8178813/hotspot.01.inc/
> 
>   some minor comments:

Thanks for looking at this.

> - I would recommend using whitebox api to issue the correct kind of GC
> instead of calling system.gc() in the first place. This would avoid the
> need to disallow ExplicitGCInvokesConcurrent. It also seems to be a
> minor effort as the test already uses whitebox. If you think the loop
> using system.gc is preferable, feel free to keep it.

Good point.  Fixed.

> - TestJNIWeakG1.java:206: the printed text does not correspond to the
> method name like the other methods do ("checkShouldNotClear" vs.
> "checkShouldNotCrash”)

Oops.  Fixed.

> - not sure about the need for the error handling using booleans in the
> main()/check() method. I would think that letting the Exception just
> propagate and the test terminate asap would be sufficient.
> 
> The extra information about the second test failing in case the first
> failed does not seem to be very helpful given that the test is very
> small anyway.

It made it easier to test the test, by backing out the fixes being tested and
verifying both failure modes.  But I agree it does uglify the test itself, and
now that we expected it to always pass, that’s more important.  So fixed.

New webrevs:
full: http://cr.openjdk.java.net/~kbarrett/8178813/hotspot.02/
incr: http://cr.openjdk.java.net/~kbarrett/8178813/hotspot.02.inc/