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

[Ao Qi]

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?

Thanks,
Ao Qi

>
>
> 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
> >>>>>
> >>>>
> >>>
> >