RFR (trivial): 8247388: Minimal build broken after JDK-8240245 (undefined reference to `MetaspaceShared::_use_optimized_module_handling')

[Ioi Lam]

I will sponsor this change and will push to jdk 15 repo later today. It 
will then be automatically synced to jdk/jdk

Thanks
- Ioi

On 6/13/20 9:21 AM, Ao Qi wrote:
> On Sun, Jun 14, 2020 at 12:02 AM <coleen.phillimore at oracle.com> wrote:
>> +1.  Sorry I made this difficult.
> Thank you, Coleen!
>
>> Should this be pushed to the jdk/jdk15 stabilization repository?
> I am not sure. I am not a Committer. I need a sponsor and maybe he/she
> will know where to push the fix:)
>
>> Also see: 8247466: AIX link error undefined
>> MetaspaceShared::_use_optimized_module_handling
>> which changes the same thing and more.
> I will leave a comment on the JDK-8247466.
>
> Cheers,
> Ao Qi
>
>> Coleen
>>
>> On 6/13/20 11:50 AM, Ioi Lam wrote:
>>>> 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