RFR 8212958: Allow Klass::_subklass and _next_sibling to have unloaded classes

[coleen.phillimore at oracle.com]

Hi Dean,  Thank you for reviewing this.

On 10/26/18 2:17 AM, dean.long at oracle.com wrote:
> Hi Coleen.  Most of this looks good to me, but I have a few questions:
>
> Is the need for Compile_lock around accesses new in 12, because the 
> implementor and subclass links can now be cleaned without being at a 
> safepoint?

Yes.  Erik's going to take Compile_lock to clean these during concurrent 
class unloading.  There were some unlocked accesses though that seemed 
suspicious before though.
>
> Style typo: In ciInstanceKlass::compute_has_subklass(), I think it 
> should be return "ik->subklass() != NULL".

Fixed.
>
> I see that you removed the memoizing of _has_subklass but left 
> _implementor.  I'm concerned that this might have a performance impact 
> and change behavior.  It seems to allow has_subklass() to change from 
> true to false (and is_leaf_type() to change from false to true) where 
> previously that should have been impossible.  With some fiddling with 
> the locking, I was able to add back _has_subklass.  Do you remember 
> why you removed it?

Yes, I removed the memoizing for _has_subklass because I couldn't take 
the Compile_lock in the ciInstanceKlass constructor, because some 
callers had it.   If you have a change that will make that work, I'd be 
happy to use yours.  I was trying to not change anything semantically, 
just grab fresh results from InstanceKlass which I didn't think would 
affect performance (although it grabs Compile_lock so maybe it could).   
We can revert this part and add locking to ciInstanceKlass constructor.  
I've been trying to avoid conditional locking.

Thanks,
Coleen
>
> dl
>
> On 10/25/18 1:42 PM, coleen.phillimore at oracle.com wrote:
>> Summary: Don't return unloaded klasses. Make sure access is protected 
>> by Compile_lock.
>>
>> See bug for more description.  This test has compiler changes since 
>> the weak klass links in Klass (and implementor in InstanceKlass are 
>> used by the compiler).  All the compiler jtreg tests passed.
>>
>> Tested with mach5 tier1-7 and test added.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8212958.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8212958
>>
>> Thanks,
>> Coleen
>