RFR(XS) 8189140 - SystemDictionaryShared::initialize() should be renamed to be more meaningful

[Ioi Lam]

On 5/14/18 11:25 PM, David Holmes wrote:
> Hi Ioi,
>
> On 15/05/2018 4:07 PM, Ioi Lam wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8189140
>> http://cr.openjdk.java.net/~iklam/jdk11/8189140-rename-system-dict-shared-initialize.v01/ 
>>
>>
>> Summary:
>>
>> 1. Removed the forced initialization of a few classes used by AppCDS 
>> at JVM start-up.
>>     Instead, initialize these class on demand by calling 
>> InstanceKlass::initialize, which
>>     is a quick no-op if the class is already initialized.
>
> Any reason not to just add them to the set of pre-loaded and 
> pre-initialized classes used with shared spaces in 
> initialize_preloaded_classes()?
>
Because initialize_preloaded_classes(), despite its name, doesn't 
actually call InstanceKlass::initialize(). It just resolves the klasses. 
If you try to call InstanceKlass::initialize() here, the VM would crash.

A few classes are initialized inside functions like 
Threads::initialize_java_lang_classes(), which seems a bit like black 
magic to me, so I didn't want to muck with them.

> Aside: It looks a little odd to lazy-init a couple of classes but then 
> assume/expect others (like SecureClassLoader) are already initialized.
>
The explicit init is only necessary before calling 
InstanceKlass::allocate_instance_handle(), which, unlike the 'new' 
bytecodes, does not implicitly initialize the class.

The other code in SystemDictionaryShared.cpp access the classes through 
JavaCalls::call_{static,special,virtual}, where the class would be 
initialized implicitly as necessary in exactly the same way as the 
invoke{static,special,virtual} bytecodes.

Perhaps we should add a JavaCalls::new_instance() method that implicitly 
initializes the class, just like the 'new' bytecode? Should I do that in 
this changeset?

> But this seems functionally fine and a good tidy up (the class init 
> stuff really didn't belong as a side-effect). The only thing I'd be 
> double checking is that the same thread (presumably the main thread) 
> is guaranteed to be the one doing the initialization.
>
There's no such requirement on the threads. If another thread is already 
in the middle of initializing the class, the thread that class 
InstanceKlass::initialize() will just wait for the class initialization 
to complete.

Thanks
- Ioi

> Thanks,
> David
>
>> 2. The only initialization left is that of a global lock. So I 
>> renamed the function
>>     to SystemDictionaryShared::initialize_locks().
>>
>> 3. I moved the call of this function from 
>> SystemDictionary::compute_java_loaders() to
>> SystemDictionary::initialize() where it seems to fit.
>>
>> Testing with hs-tiers 1 and 2.
>>
>> Thanks
>> - Ioi