RFR(trivial): 8232768: Configuration with --disable-cds --enable-generate-classlist should be reported as an error

Jie Fu fujie at loongson.cn
Tue Oct 22 22:48:01 UTC 2019


Hi Magnus,

Thank you very much.
Yes, I need a sponsor.

Thanks a lot.
Best regards,
Jie

On 2019/10/23 上午5:14, Magnus Ihse Bursie wrote:
>
>
> On 2019-10-22 14:52, Jie Fu wrote:
>> Hi Magnus,
>>
>> Thanks for your review and valuable comments.
>> It is the second case since the VM will report an error for 
>> DumpLoadedClassList when CDS is disabled.
>> So generate-classlist *never* will work with CDS disabled.
>>
>> Updated: http://cr.openjdk.java.net/~jiefu/8232768/webrev.01/
> This looks good to me. Do you need a sponsor for the patch?
>
> /Magnus
>>
>> Please review it and give me some advice.
>>
>> Thanks a lot.
>> Best regards,
>> Jie
>>
>> On 2019/10/22 下午5:54, Magnus Ihse Bursie wrote:
>>> On 2019-10-22 09:44, Jie Fu wrote:
>>>> Hi all,
>>>>
>>>> May I get reviews for this one-line change?
>>>>
>>>> JBS:    https://bugs.openjdk.java.net/browse/JDK-8232768
>>>> Webrev: http://cr.openjdk.java.net/~jiefu/8232768/webrev.00/
>>>>
>>>> This kind of configuration, with both --disable-cds and 
>>>> --enable-generate-classlist, will fail.
>>>> The failure is caused by -XX:DumpLoadedClassList [1], which doesn't 
>>>> work as expected when cds is disabled.
>>>>
>>>> It might be better to produce an error.
>>> I'm not sure I agree with this fix. Checking the code, we make an 
>>> effort to determine if we should warn the user:
>>>
>>>   # Check if it's likely that it's possible to generate the 
>>> classlist. Depending
>>>   # on exact jvm configuration it could be possible anyway.
>>>   if test "x$ENABLE_CDS" = "xtrue" && 
>>> (HOTSPOT_CHECK_JVM_VARIANT(server) || 
>>> HOTSPOT_CHECK_JVM_VARIANT(client) || 
>>> HOTSPOT_CHECK_JVM_FEATURE(cds)); then
>>>
>>> Only if the user *explicitly* sets --enable-generate-classlist, the 
>>> warning is printed. The comment clearly states that this might be 
>>> possible anyway, given the circumstances, so I believe a warning, 
>>> and not an error, is the right thing here. The user explicitly 
>>> requests generate-classlist. We warn that it might not work, but we 
>>> allow it since it can work in cases we don't really know about.
>>>
>>> If it is the case that generate-classlist *never* will work with CDS 
>>> disabled (this sounds reasonable, but I don't know if this really is 
>>> the case), and you want to catch this, then I suggest you 
>>> restructure the code so you catch the CDS && generate-classlist case 
>>> separately, and give a fatal error for that, and then keep the 
>>> current check for jvm variants/features and let it keep producing a 
>>> warning for that.
>>>
>>> /Magnus
>>>>
>>>> Thanks a lot.
>>>> Best regards,
>>>> Jie
>>>>
>>>> [1] 
>>>> http://hg.openjdk.java.net/jdk/jdk/file/ca620b06b5c9/make/GenerateLinkOptData.gmk#l68
>>>>
>>>>
>>>
>>
>




More information about the build-dev mailing list