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

[Ioi Lam]

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