RFR: 8207812: Implement Dynamic CDS Archive

[Karen Kinnear]

Calvin,

Many thanks - I think that makes it clearer for everyone.

thanks,
Karen

> On Apr 25, 2019, at 6:23 PM, Calvin Cheung <calvin.cheung at oracle.com> wrote:
> 
> Hi Karen,
> 
> I've updated the comment to the following:
> 
>   if (DynamicDumpSharedSpaces) {
>     // For dynamic dumping, we can resolve all the constraint classes for all class loaders during
>     // the initial run prior to creating the archive before vm exit. We will also perform verification
>     // check when running with the archive.
>     return false;
>   } else {
>     if (is_builtin(k)) {
>       // For builtin class loaders, we can try to complete the verification check at dump time,
>       // because we can resolve all the constraint classes. We will also perform verification check
>       // when running with the archive.
>       return false;
>     } 
> 
> thanks,
> Calvin
> 
> On 4/25/19, 8:48 AM, Karen Kinnear wrote:
>> 
>> One follow-up question relative to Jiangli’s review questions please:
>> 
>>> On Apr 23, 2019, at 8:08 PM, Calvin Cheung <calvin.cheung at oracle.com <mailto:calvin.cheung at oracle.com>> wrote:
>>> 
>>> Hi Jiangli,
>>> 
>>> Thanks a lot for your review!
>>> 
>>> On 4/22/19, 2:07 PM, Jiangli Zhou wrote:
>>>> Hi Calvin,
>>>> 
>>>> 
>>>> - src/hotspot/share/classfile/systemDictionaryShared.cpp
>>>> 
>>>> 1218   if (DynamicDumpSharedSpaces) {
>>>> 1219     return false;
>>>> 1220   } else {
>>>> 
>>>> The above case for DynamicDumpSharedSpaces needs to be examined
>>>> carefully. Can you please ask Harold (and Coleen or Karen) to take a
>>>> look? Also, a comment is needed to explain that we can complete all
>>>> verification checks at dynamic dumping time.
>>> I've added a comment. If it return false, the caller will call VerificationType::resolve_and_check_assignability().
>> Just making sure I understand this correctly:
>> 
>> When we archive a class, we also archive all supertypes. We do not necessarily archive all classes in
>> the verification constraints list. We always record all verification constraints, whether or not we actually
>> continue to load and perform them.
>> 
>> When we run with an archive, we ensure that the supertypes that we use are also in the archive and are an exact match,
>> otherwise we don’t select the class from the archive.
>> We then check the verification constraints list, which might then cause additional class loading. If that fails,
>> again, we don’t select the class from the archive. This allows more flexibility in changes to the classes found
>> at runtime, as long as their “isAssignableFrom” constraints are still met.
>> 
>> So - prior to the DynamicDumpSharedSpace - 
>> 
>> In SystemDictionaryShared::add_verification_constraint:
>>   I agree with the logic that we add constraints for all archived classes.
>> 
>>   I agree that for non-built-in class loaders, we could not complete the verification check at dump time,
>>   and only perform this at runtime using the archive.
>>  
>> For built-in class loaders - I agree that we could ALSO perform the verification check at dump time, and
>> if it failed, eliminate archiving the class.
>> 
>> I assume that is intended behavior for DynamicDumpSharedSpaces?
>> 
>> Couple of questions:
>> 1. Does what I said above for built-in class loaders match your understanding? If so, perhaps this would
>> be a good time to clarify the comment - since we will perform the verification check now AND
>> and when running with the archive.
>> 
>> 2. For DynamicDumpSharedSpaces
>> Since we do not perform any loading as part of the PopulateDynamicDumpSharedSpaces,
>> I assume that this is all about recording verification constraints for all classes loaded while
>> we run, before we get to the archive creation on exit.
>> 
>> So your logic of creating constraints for all archived classes and then returning false so
>> that the verification is also run during the first run makes sense to me.
>> 
>> And yes, it makes sense to add a comment that we should ALSO perform verification checks during
>> the initial run in addition to creating the constraint for running later with the archive. I don’t conceptually
>> think of this as checking constraints “at dump time”, but rather for the initial run before creating
>> the dynamic archive.
>> 
>> thanks,
>> Karen
>>