RFR: JDK-8188312 Use CDS if present when running the Boot JDK during build

[Ioi Lam]

On 10/6/17 2:04 AM, Magnus Ihse Bursie wrote:
>
> On 2017-10-05 18:08, Ioi Lam wrote:
>>
>>
>> On 10/5/17 2:35 AM, Magnus Ihse Bursie wrote:
>>> On 2017-10-05 11:07, Claes Redestad wrote:
>>>>
>>>>
>>>> On 2017-10-05 10:59, Magnus Ihse Bursie wrote:
>>>>>> How often is -Xbootclasspath/p used?
>>>>>>
>>>>>> Why not use "-XX:-VerifySharedSpaces 
>>>>>> -XX:SharedArchiveFile=local.jsa -Xshare:auto"? That way you will 
>>>>>> have the start-up benefit if possible.
>>>>>
>>>>> My worry here is that -Xshare:auto will not work correctly if 
>>>>> -Xbootclasspath/p is used. Maybe someone can guarantee that this 
>>>>> will work and convince me that it will never fail, but I don't 
>>>>> think this risk is worth the marginal gain.
>>>>
>>>> Using -Xshare:auto should mean any case where a CDS archive can't 
>>>> be used (for whatever reason) should be silently ignored. I'd be more
>>>> worried if -Xshare:on didn't fail in this case!
>>>
>>> But we're actively disabling verification of the CDS archive! How is 
>>> then CDS supposed to know that it contains code for core classes 
>>> that has been superseded using -Xbootclasspath/p?
>>>
>>
>> Hi Magnus,
>>
>> Now I understand your hesitation -- sorry the docs aren't clear about 
>> how the CDS archive is validated.
>>
>> + The classpath validation is always done, and cannot be turned off. 
>> So if -Xbootclasspath/p is used, the CDS archive will not be loaded.
> And the error messsage about incorrect class paths when i used 
> -Xshare:on, was it the way CDS tells me it will not use the CDS 
> archive due to -Xbootclasspath/p?

Yes, when -Xbootclasspath/p is specified, -Xshare:auto would silently 
ignore archive, and -Xshare:on would print an error message and quit the VM.

>>
>> + After the CDS archive is loaded, we have a second step that does a 
>> checksum over its contents. This step can be disabled using 
>> -XX:-VerifySharedSpaces
>>
>> In fact, -Xshare:auto has been the default for the client VM for many 
>> JDK releases, and we have not had any issues. (JDK-8188105 will make 
>> -Xshare:auto the default for server VM as well.)
>>
>> So, it's safe to use the combination of "-XX:-VerifySharedSpaces 
>> -XX:SharedArchiveFile=local.jsa -Xshare:auto"
>
> Okay. I'll make a fourth try:
> http://cr.openjdk.java.net/~ihse/JDK-8188312-use-CDS-for-bootjdk/webrev.04 
>
>
> Changes compared to previous:
> * I do not check if the local archive is present, I always re-generate 
> it (Erik's concern about switching boot jdk)
> * If I cannot use the local archive, I always try adding -Xshare:auto 
> anyway (Claes concern)
> * If I can use local archive, I use -Xshare:auto instead of -Xshare:on 
> (which, according to the discussion above, should skip the use of the 
> CDS archive when -Xbootclasspath/p is used).
> * Finally I renamed the variable to BOOTJDK_USE_LOCAL_CDS to clarify 
> it's use.
>
> I'm currently running tests on this to see that it does not break.
>
This change looks good to me. Thanks for trying my suggestions :-)

- Ioi

> /Magnus
>
>>
>> Thanks
>> - Ioi
>>
>>>>
>>>>>
>>>>> This was supposed to be a quick and simple patch to get a small, 
>>>>> but useful improvement. It's not worth a lot of investigation or 
>>>>> fixes, imho.
>>>>
>>>> Repeating my suggestion I put as a comment in the RFE: add 
>>>> -Xshare:auto but leave out the code to dump an archive in the build 
>>>> (for now),
>>>> so that those of us who prepare our boot JDK to have CDS archive 
>>>> generated can get the benefit from it.
>>> Well then, can you then guarantee that this will not break when 
>>> replacing stuff using -Xbootclasspath/p? Because I don't want to be 
>>> debugging things when a slightly different version of the class was 
>>> *not* used as it should since an old cached version in CDS was 
>>> picked up instead. :-(
>>>
>>> /Magnus
>>>>
>>>> /Claes
>>>
>>
>