RFR (trivial): 8247388: Minimal build broken after JDK-8240245 (undefined reference to `MetaspaceShared::_use_optimized_module_handling')
Ao Qi
aoqi at loongson.cn
Sat Jun 13 16:11:37 UTC 2020
Thanks Ioi. I think David Holmes also reviewed webrev.00. Could
someone help to sponsor this patch?
Thanks,
Ao Qi
On Sat, Jun 13, 2020 at 11:50 PM Ioi Lam <IOI.LAM at oracle.com> 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
More information about the hotspot-runtime-dev
mailing list