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

[Jiangli Zhou]

On 11/8/18 11:08 AM, Ioi Lam wrote:
>
>
> On 11/8/2018 9:57 AM, Jiangli Zhou wrote:
>> Hi Ioi,
>>
>> Thanks for the review!
>>
>> On 11/7/18 10:37 PM, Ioi Lam wrote:
>>> The changes look OK for me, but they don't seem to do anything now, 
>>> as all the classes that we have in the table today are already 
>>> initialized.
>> Yes, the current classes in the subgraph entry table are already 
>> initialized during VM initialization. The change is aimed for the 
>> ones that are not used during VM initialization, but are commonly 
>> used by applications (and safe to be archived).
>>
>> I also want to clarify that the change does not have any impact on 
>> the runtime initialization order. For example, archiving the Long 
>> cache will not cause premature initialization of Long and Long$Cache 
>> at runtime.
>>>
>>> Maybe this should wait until we implement the caching of 
>>> Long$LongCache?
>>
>> Claes has already started the work for Long cache, but is blocked by 
>> this issue. I've tested with his Long cache change using this patch 
>> and verified that it enabled the cache being archived properly. I'd 
>> recommend integrating this to unblock the work for JDK-8213033 
>> (https://bugs.openjdk.java.net/browse/JDK-8213033).
>>
>
> Hi Jiangli,
>
> I think it's OK to push this change separately from JDK-8213033, if 
> that's the way you and Claes want to handle it.

Thanks!

Jiangli
>
> Thanks
> - Ioi
>
>> Thanks,
>> Jiangli
>>>
>>> Thanks
>>> - Ioi
>>>
>>> On 11/7/18 7:24 PM, Jiangli Zhou wrote:
>>>> Thanks a lot for the careful review, David!
>>>>
>>>> Thanks,
>>>> Jiangli
>>>>
>>>>> On Nov 7, 2018, at 6:47 PM, David Holmes <david.holmes at oracle.com> 
>>>>> wrote:
>>>>>
>>>>> 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
>>>>>>>>>>
>>>
>>
>