RFR: JDK-8223599: minimal build failed after JDK-8185525

[Ao Qi]

On Thu, May 9, 2019 at 9:43 PM David Holmes <david.holmes at oracle.com> wrote:
>
> On 9/05/2019 9:12 pm, Wang Haomin wrote:
> > Hi  Aleksey, David,
> >
> > Thanks for your suggestion. However, build is failed when using
> > "--with-jvm-features=-jfr --with-jvm-variants=server
> > --with-debug-level=release --disable-precompiled-headers".
> > I made a litter modification. Could you review it?
> >
> > Webrev: http://cr.openjdk.java.net/~aoqi/8223599/webrev.02/
>
> shenandoahCollectionSet.cpp was not altered by 8185525, so while I
> understand your desire to make this fix, it doesn't seem right to do it
> under the current bug as currently described. The issue with disabling
> PCH would not seem to be related to the minimal build at all.

Indeed. The initial desire is to fix the issue introduced by 8185525.
Should I change the description or file another issue?

>
> Actually I would have expected shenandoah to be excluded from a minimal
> build! (Like all the other non-experimental GCs are, other than SerialGC.)

minimal and --disable-precompiled-header build would not trigger the
shenandoahCollectionSet.cpp problem. server, -jfr and
--disable-precompiled-header build would.

Thanks,
Ao Qi


>
> David
>
> > Thanks,
> > Haomin
> >
> > 在 2019年05月09日 16:57, Aleksey Shipilev 写道:
> >> On 5/9/19 7:44 AM, David Holmes wrote:
> >>> On 9/05/2019 3:25 pm, Wang Haomin wrote:
> >>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8223599
> >>>> Webrev: http://cr.openjdk.java.net/~aoqi/8223599/webrev.00/
> >>>>
> >>>> Build is failed when using --with-jvm-features=-jfr. Would anyone
> >>>> review the fix?
> >>> I'm not sure this is the best way to fix this. I think JDK-8185525
> >>> needed to isolate
> >>> TableRateStatistics more completely if it is only really used by JFR.
> >>>
> >>> What you have will suffice for now, though you probably also want to
> >>> guard:
> >>>
> >>>   26 #include "jfr/jfr.hpp"
> >>>
> >>> with "#if INCLUDE_JFR"
> >> Yes, need to protect that.
> >>
> >> Plus, need macros.hpp to gain access to INCLUDE_JFR without
> >> precompiled headers.
> >>
> >> Plus, I think ifdef-ing the usages within the method bodies is
> >> cleaner, as it keeps method
> >> declarations and definitions defined together.
> >>
> >> Something like:
> >>
> >> http://cr.openjdk.java.net/~shade/8223599/webrev.01/src/hotspot/share/utilities/tableStatistics.cpp.sdiff.html
> >>
> >>
> >> -Aleksey
> >>
> >
> >