[13] RFR (M): 8223216: C2: Unify class initialization checks between new, getstatic, and putstatic

[Vladimir Ivanov]

Thanks for the feedback, Dean.

> Do you want to have a Runtime reviewer take a look at the new logic?

I'm definitely looking for feedback on 8223213 from Runtime team. But 
8223216 is C2-specific and incrementally builds on top of it, so I don't 
think there's anything new for Runtime team to look at.

> Can you explain why Parse::clinit_deopt() changed from testing for
> 
> InstanceKlass::fully_initialized
> 
> to testing for
> 
> InstanceKlass::being_initialized
> 
> instead?  How do we know we it is the initializing thread?

Initializing thread is irrelevant here. The check is solely about the 
current state of the holder class.

Parse::clinit_deopt() is not mandatory (nmethod clinit barrier on entry 
cover all important cases), but an optimization. It is added by 8223213 
specifically for C2 to trigger recompilation once the holder class is 
fully initialized. The motivation is to get better code when a class is 
fully initialized.

The change in 8223216 is intended as a refactoring: since there are only 
2 states allowed here (being_initialized and fully_initialized), it 
doesn't matter what state is checked (== being initialized vs != 
fully_initialized).

Best regards,
Vladimir Ivanov

> On 5/1/19 4:37 PM, Vladimir Ivanov wrote:
>> http://cr.openjdk.java.net/~vlivanov/8223216/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8223216
>>
>> (The patch has minor dependencies on 8223213 [1] I sent out for review 
>> earlier.)
>>
>> C2 implements class initialization checks for new and 
>> getstatic/putstatic differently: while "new" supports fast class 
>> initialization checks, static field accesses rely on uncommon traps 
>> which may lead to deoptimization/recompilation storms during 
>> long-running class initialisation.
>>
>> Proposed patch unifies implementation between them and uses the 
>> following barrier:
>>    if (holder->is_initialized()) {
>>      uncommon_trap(initialized, reinterpret);
>>    }
>>    if (!holder->is_reentrant_initialization(current_thread)) {
>>      uncommon_trap(uninitialized, none);
>>    }
>>
>> It also enhances checks for not-yet-initialized classes 
>> (Compile::needs_clinit_barrier) and unifies the implementation between 
>> new, invokestatic, and getfield/putfield.
>>
>> Testing: tier1-5, targeted microbenchmarks, new test from 8223213
>>
>> Thanks!
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> [1] http://cr.openjdk.java.net/~vlivanov/8223213/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8223213
>>
>