RFR: 8207812: Implement Dynamic CDS Archive

[Calvin Cheung]

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
>