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

[Ioi Lam]

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


Or else you might see an unexpected log message in minimum build.

I think we can fix the stylistic issues in the bug JDK-8247449 "Revisit 
the argument processing logic for 
MetaspaceShared::disable_optimized_module_handling()", which David has 
just filed.

I've added comments in the above bug the explain why the code was 
written like that in JDK-8240245.

Thanks
- Ioi

> thanks,
> Coleen
>>
>> Thanks,
>> David
>>
>>> Coleen
>>>
>>> On 6/11/20 7:53 AM, coleen.phillimore at oracle.com wrote:
>>>> This looks good and trivial.
>>>> thanks,
>>>> Coleen
>>>>
>>>> On 6/11/20 6:15 AM, Ao Qi wrote:
>>>>> Hi all,
>>>>>
>>>>> Could you please review this patch?
>>>>>
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8247388
>>>>> webrev: http://cr.openjdk.java.net/~aoqi/8247388/webrev.00/
>>>>>
>>>>> Thanks,
>>>>> Ao Qi
>>>>>
>>>>
>>>
>