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

[coleen.phillimore at oracle.com]

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).

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.

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