[13] RFR (M): 8223213: Implement fast class initialization checks on x86-64

[Vladimir Ivanov]

Thanks, Coleen.

Updated webrev:
   http://cr.openjdk.java.net/~vlivanov/8223213/webrev.03

Incremental webrev:
   http://cr.openjdk.java.net/~vlivanov/8223213/webrev.03_02/

> I reviewed mostly the interpreter and shared change.  As someone else 
> commented, I don't like the addition of a develop flag because some 
> platforms don't support class initialization barriers.  Isn't it normal 
> to do this with the misnamed VM_Version, like adding 
> VM_Version::supports_class_initialization_barriers() with x86 returning 
> true until the other platforms false until they implement the feature. 
> Then there isn't another flag configuration to test (or not test).

I like your suggestion. Didn't know VM_Version is used in such a way.

Replaced UseFastClassInitChecks with 
VM_Version::supports_fast_class_init_checks().

> http://cr.openjdk.java.net/~vlivanov/8223213/webrev.02/src/hotspot/cpu/x86/interp_masm_x86.cpp.udiff.html 
> 
> 
> I have to admit that the relationship between resolved bytecode in 
> bytecode_1/bytecode_2 and which _f1/_f2 held the Method* was actually a 
> surprise to me.  There's nothing structurally in cpCache other than 
> reading the code that enforces this and it wasn't always a Method* in f2 
> for invokeinterface, for example, so it's sort of an accident.
> 
> But this code is correct and I think as a follow up we should make 
> load_invoke_cp_cache_entry() call load_resolved_method_at_index() too 
> and have some assert in cpCache that this is true, or rewrite the 
> cpCache completely.

I went ahead and changed load_invoke_cp_cache_entry() to call 
load_resolved_method_at_index() (along with some other minor 
refactorings). If you have any ideas/suggestions about the assert, I can 
add it as well.

Best regards,
Vladimir Ivanov

> The code to do the initialization barrier in the interpreter looks good.
> 
> Thanks,
> Coleen
> 
> On 5/28/19 7:40 AM, Vladimir Ivanov wrote:
>> Thanks, Martin.
>>
>> Updated webrev:
>>   http://cr.openjdk.java.net/~vlivanov/8223213/webrev.02/
>>
>>> Are these assertions safe?
>>> +   assert(method()->needs_clinit_barrier(), "barrier not needed");
>>> +   assert(method()->holder()->is_being_initialized(), "barrier not 
>>> needed");
>>> Can it happen that initialization concurrently completes before they 
>>> are evaluated?
>>
>> Good point. Even though ciInstanceKlass caches initialization state of 
>> the corresponding InstanceKlass, it seems there's a possibility that 
>> the state is updated during the compilation (see 
>> ciInstanceKlass::update_if_shared). I enhanced the asserts to check 
>> that initialization has been stated.
> 
> Ok, this makes sense.
>>
>>> A small suggestion for x86 TemplateTable::invokeinterface:
>>> It'd be nice to replace load of interface klass by your new 
>>> load_method_holder.
>>
>> Agree. Updated.
> 
> This is nice.
> 
> 
>>
>> Best regards,
>> Vladimir Ivanov
>