RFR (S): 8213439: Run class initialization for boot loader classes with registered subgraph archiving entry field during CDS dump time

[David Holmes]

Hi Jiangli,

On 8/11/2018 12:29 PM, Jiangli Zhou wrote:
> Hi David,
> 
> 
> On 11/7/18 5:49 PM, David Holmes wrote:
>> Hi Jiangli,
>>
>> On 8/11/2018 11:25 AM, Jiangli Zhou wrote:
>>> Hi David,
>>>
>>> Attached are two logs for class initialization:
>>>
>>>    init_nodumping: bin/java -Xshare:off
>>>
>>>    init_cdsumping: bin/java -Xshare:dump
>>>
>>> The class initialization orderings are identical for 'bin/java 
>>> -Xshare:off' and 'bin/java -Xshare:dump' before the initialization of 
>>> sun.launcher.LauncherHelper.
>>
>> Thanks! That's very interesting. How do you actually initiate 
>> execution of Java code at dump time - ie how far in the VM init 
>> sequence do we go before you do the dump and exit?
> 
> Currently the dumping process is done at the very end of 
> Threads::create_vm() so we can avoid interfering with the initialization 
> order. The module system is initialized in initPhase2, which happens 
> before the dumping process. Module system initialization runs some java 
> code.

Okay I'm much less concerned now as we are doing the normal init 
procedure. :)

> During loading and dumping, we invokes the system loader's loadClass() 
> method explicitly for each class specified in the classes list (please 
> see ClassListParser::load_current_class). That also triggers java code 
> execution.
>>
>> I compared the dump list with what I see currently, before your patch, 
>> and it initializes exactly the same number of classes - 279, but not 
>> in exactly the same order, and I can't readily tell that it is the 
>> same 279 classes (but suspect it is). So I'm a little unsure as to 
>> what exactly your patch does today, and how it might be used in the 
>> future for the Long$LongCache issue ?
> My current patch does not have any visible effect on the class 
> initialization for dump time yet. When the LongCache being adding for 
> archiving, we should see Long and Long$LongCache being initialized at 
> dump time.

Okay. I'll wait for that change to see how it works.

> Just to verify, I just tried dumping out the initialized 
> classes without my patch and I saw the lists is identical (attached 
> init_cdsdumping0). The orderings of the initialization are the same with 
> and without current patch. My repo was synced earlier this afternoon. 
> Maybe there are some differences between our local repos? A library 
> change (or other change) could potentially change the ordering.

Yes my repo is a little out of date.

Thanks, this all looks good to me.

David

> Really appreciate that you are taking a closer look on this!
> 
> Thanks,
> Jiangli
>>
>> Thanks,
>> David
>>
>>> Thanks,
>>>
>>> Jiangli
>>>
>>>
>>> On 11/7/18 4:56 PM, David Holmes wrote:
>>>> Hi Jiangli,
>>>>
>>>> Messing with the established initialization order still rings alarm 
>>>> bells for me. Can you generate the normal class initialization list 
>>>> for VM startup, and one for a CDS dump, so that we can see how they 
>>>> differ. It would very easy for subtle initialization bugs to creep 
>>>> in here. (Even with the existing use of Java code during dump time.)
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 8/11/2018 10:45 AM, Jiangli Zhou wrote:
>>>>> Please review following webrev that explicitly runs class 
>>>>> initialization for classes (for boot loader only) with registered 
>>>>> subgraph archiving entry field(s) during CDS dump time.
>>>>>
>>>>> At CDS dump time, some of the loaded classes are initialized due to 
>>>>> execution of java code. Currently there are 279 classes are 
>>>>> initialized at dump time. The rest of the loaded classes are not 
>>>>> initialized. Running class initialization for the boot classes with 
>>>>> registered subgraph archiving entry field(s) will allow those 
>>>>> static fields (and their directly/indirectly referenced objects) 
>>>>> being populated at CDS dump time and archived. The Long$LongCache 
>>>>> (planned for JDK-8213033 by Claes, thanks!) is such an example.
>>>>>
>>>>>    webrev: http://cr.openjdk.java.net/~jiangli/8213439/webrev.00/
>>>>>
>>>>>    RFE: https://bugs.openjdk.java.net/browse/JDK-8213439
>>>>>
>>>>> The initialization of a class is triggered explicitly when 
>>>>> processing the subgraph entry fields at dump time. For any class 
>>>>> that's already initialized at this point, it's essentially a nop. 
>>>>> That's the case for current existing subgraph entries.
>>>>>
>>>>> Tested with tier1-tier3. Thanks David and Ioi's for discussions in 
>>>>> the bug report.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Jiangli
>>>>>
>>>
>