RFR (trivial): 8247388: Minimal build broken after JDK-8240245 (undefined reference to `MetaspaceShared::_use_optimized_module_handling')
Ioi Lam
IOI.LAM at ORACLE.COM
Sat Jun 13 15:50:29 UTC 2020
> On Jun 13, 2020, at 7:37 AM, Ao Qi <aoqi at loongson.cn> wrote:
>
> On Fri, Jun 12, 2020 at 3:29 PM David Holmes <david.holmes at oracle.com> wrote:
>>
>>
>>
>>> On 12/06/2020 4:36 pm, Ao Qi wrote:
>>> On Fri, Jun 12, 2020 at 1:22 PM Ioi Lam <ioi.lam at oracle.com> wrote:
>>>>
>>>>
>>>>
>>>> On 6/11/20 7:45 AM, coleen.phillimore at oracle.com wrote:
>>>>>
>>>>>
>>>>> On 6/11/20 9:30 AM, David Holmes wrote:
>>>>>> On 11/06/2020 9:55 pm, coleen.phillimore at oracle.com wrote:
>>>>>>>
>>>>>>> On further viewing, I think adding NOT_CDS_RETURN to
>>>>>>>
>>>>>>> MetaspaceShared::disable_optimized_module_handling();
>>>>>>>
>>>>>>> would be a lot better actually.
>>>>>>
>>>>>> That's fine by me to get this fixed for the build. But what Ao Qi has
>>>>>> highlights that this code is IMO in the wrong place to begin with.
>>>>>> This logic should be in Arguments::create_module_property not the
>>>>>> general purpose Arguments::add_property. I will file a follow up bug
>>>>>> to address that.
>>>>>
>>>>> I think this is fine also, but the cleanup I'd like to see is if
>>>>> metaspaceShared.cpp is not included in the INCLUDE_CDS build, then the
>>>>> declarations should not be included either.
>>>>>
>>>>
>>>> I think the posted webrev is fine. Because jdk15 has been forked now,
>>>> the fix should be pushed to jdk15 (and then it will be automatically
>>>> synced down to jdk/jdk??).
>>>>
>>>> Adding NOT_CDS_RETURN
>>>> MetaspaceShared::disable_optimized_module_handling() would be nice to
>>>> have, but this code still needs to be #ifdef'ed
>>>>
>>>> + #if INCLUDE_CDS
>>>> if (is_internal_module_property(key) ||
>>>> strcmp(key, "jdk.module.main") == 0) {
>>>> MetaspaceShared::disable_optimized_module_handling();
>>>> log_info(cds)("Using optimized module handling disabled due to incompatible property: %s=%s", key, value);
>>>> }
>>>> + #endif
>>>
>>> also:
>>>
>>> +#if INCLUDE_CDS
>>> MetaspaceShared::disable_optimized_module_handling();
>>> log_info(cds)("Using optimized module handling disabled due to
>>> bootclasspath was appended");
>>> +#endif
>>>
>>> then I think webrev.01 has to include all the changes in webrev.00
>>>
>>> Shell we push the change and let the build work?
>>
>> I think push webrev.00. As Ioi stated we don't want to execute the other
>> code unnecessarily in a build without CDS.
>
> Since there are no objections, shall we push webrev.00 to let the
> build work? If Coleen has further suggestions we can do it in
> JDK-8247449 or I can do it in another bug.
>
> Thanks,
> Ao Qi
>
Yes, please push webrev.00
Thanks
Ioi
More information about the hotspot-runtime-dev
mailing list