RFR: 8201136: Move GC flags from globals.hpp to GC specific files

[Aleksey Shipilev]

On 04/05/2018 11:58 AM, Stefan Karlsson wrote:
>> *) In globals.hpp, L2987-..., you probably want to right-adjust the backslashes?
> 
> I sent an updated webrev to my answer to StefanJ's review. I hope that looks good enough.

Ok, that looks good.

>> *) In gc_globals.hpp, we still have Use{CMS,Serial,G1,Parallel,...}GC -- is the intent to have these
>> in shared gc_globals.hpp, not in per-GC-globals.hpp, and check for GC support at init? In other
>> words, when conditional GC compilation arrives, we still have GC enabling flags available, but not
>> the GC-specific flags?
> 
> The current state of my patch guards most usages of the Use{CMS,Serial,G1,Parallel,...}GC flags with
> the appropriate INCLUDE_<GC> guard. I'm not sure if it is possible to take it all the way and guard
> all usages, but I try and we'll see if it is possible.
> 
> What do you think? Should we try to move the flags to their respective <gc>_globals.hpp file?

Meh. On one hand, it is really an UX question: should, say, Minimal VM barf with "Unrecognized VM
option: UseG1GC" or say "G1 is not supported"? I think it should do the former, which means UseG1GC
should be in gc_globals.hpp.

Guard-wise, it seems more convenient to have:

#ifdef INCLUDE_G1
 if (UseG1GC) {
   // <G1-specific thing>
 }
#endif

...because the protected block may reference G1-specific classes.

This looks more awkward:

 if (UseG1GC) {
#ifdef INCLUDE_G1
   // <G1-specific thing>
#endif
 }

UseG1GC in gc_globals.hpp allows both, and moving UseG1GC to g1_globals.hpp allows only the first form.

But doing the first form probably makes one-off checks and asserts that include UseG1GC in shared
parts more awkward, because we would need to take care of INCLUDE_G1 on those paths, which would
give raise to crud like:

 if (... || G1_ONLY(UseG1GC) NOT_G1(true)) ...

...instead of just:

 if (... || UseG1GC) ...

I am leaning towards having Use*GC flags in gc_globals.hpp. I was wondering if that is intentional,
or just preserving the status quo.

Aside, it would be interesting to see if we can easily make --disable-g1gc (or whatever it is called
in the build-configurable GC RFE) to turn UseG1GC to compile-time constant, so that the whole thing
would fold.

Thanks,
-Aleksey