RFR: 8201136: Move GC flags from globals.hpp to GC specific files (original) (raw)

Aleksey Shipilev shade at redhat.com
Thu Apr 5 10:39:15 UTC 2018

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 gcglobals.hpp, we still have Use{CMS,Serial,G1,Parallel,...}GC -- is the intent to have these in shared gcglobals.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 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 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) { // } #endif

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

This looks more awkward:

if (UseG1GC) { #ifdef INCLUDE_G1 // #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

More information about the hotspot-dev mailing list