RFR (S) 8190891: Clean up G1 barrier code in compiler interface (ci)

[coleen.phillimore at oracle.com]

Kim,

Thank you for revewing.

On 11/8/17 2:28 PM, Kim Barrett wrote:
>> On Nov 7, 2017, at 7:38 PM, coleen.phillimore at oracle.com wrote:
>>
>> Summary: consolidate gc barrier code in ci
>>
>> See bug and comments for more details.
>>
>> Tested with mach5 tier1-5 on linux x64 and iwndows x64.  Also tested StressRedefineWithoutBytecodeModification in test/jdk/java/lang/instrument which fails without the g1 barriers.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8190891.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8190891
>>
>> Thanks,
>> Coleen
> ------------------------------------------------------------------------------
> src/hotspot/share/ci/ciInstanceKlass.cpp
>    55 static void ensure_metadata_alive(oop metadata_holder) {
> ...
>    60   if (metadata_holder != NULL) {
>
> When would we ever get a ciInstanceKlass for a Klass with a NULL
> holder?  That seems like a situation that shouldn't ever happen, and
> this should be an assert instead.  And there is an assert here

When it's the NULL class loader, associated with 
ClassLoaderData::the_null_class_loader_data().   This class loader is 
never unloaded.
>
> 75   assert(get_instanceKlass()->is_loaded(), "must be at least loaded");
>
> that dominates the ensure_metadata_alive call that is similarly
> suggestive.  Though I'm not sure whether is_loaded can be true if the
> klass was loaded and then later had its holder die and be cleared.

Not sure I understand this comment.  The InstanceKlass must be in the 
"loaded" _init_state to get to this code, but the marking for 
ensure_metadata_alive and creating an object prevents it from being 
unloaded after this point.
>
> And can the call to get_object at line 99 cope with a NULL holder?

No anonymous class's holders are the mirror which cannot be null.  I can 
add this before line 99:

     assert(holder != NULL, "holder of anonymous class is the mirror 
which is never null");

>
> Basically I'm questioning whether the ik->klass_holder() on line 91
> can ever legitimately return NULL.

Yes it can and does.

Thanks,
Coleen



>
> ------------------------------------------------------------------------------
>