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

[David Holmes]

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.

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

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