RFR: 8200729: Conditional compilation of GCs

[coleen.phillimore at oracle.com]

http://cr.openjdk.java.net/~stefank/8200729/webrev.04/02.mainPatch/src/hotspot/share/oops/instanceKlass.hpp.frames.html

1237 #endif //INCLUDE_OOP_OOP_ITERATE_BACKWARDS


Can you add // INCLUDE... here.    My personal nit is an #endif that's 
far away from the #ifdef.

http://cr.openjdk.java.net/~stefank/8200729/webrev.04/02.mainPatch/src/hotspot/share/oops/instanceMirrorKlass.inline.hpp.frames.html

This one too although these are a bit closer.

I reviewed oops, runtime and utilities directories.  Looks good.
Thanks,
Coleen

On 5/2/18 10:37 AM, Stefan Karlsson wrote:
> Hi all,
>
> Please review these patches to allow for conditional compilation of 
> the GCs in HotSpot.
>
> Full patch:
>
> http://cr.openjdk.java.net/~stefank/8200729/webrev.04/all/
>
> (See below for a more fine-grained division into smaller patches)
>
> Today Parallel, G1, and CMS, are all guarded by INCLUDE_ALL_GCS. 
> INCLUDE_ALL_GCS becomes defined to 1 for our server 
> (--with-jvm-variants=server) builds, and is defined to 0 for the 
> minimal (--with-jvm-variants=minimal) builds. There are also ways to 
> forcefully remove these GCs from the compilation by configuring with, 
> for example, --with-jvm-features=all-gcs.
>
> The proposed patch removes INCLUDE_ALL_GCS (and all-gcs) and replaces 
> it with INCLUDE_CMSGC, INCLUDE_G1GC, and INCLUDE_PARALLELGC. In 
> addition to that, INCLUDE_SERIALGC has been added to guard the Serial 
> GC code.
>
> Future GCs should adopt this scheme before they get incorporated into 
> the code base. Note, that there will be some files in gc/shared that 
> are going to have to know about all GCs, but the goal is to have very 
> few of these INCLUDE_<GC> checks in the non-GC parts of HotSpot.
>
> With this patch, it's also going to be easier to stop compiling CMS 
> when the time as come for it to move from deprecated to removed.
>
> Note, that even though this adds great flexibility, and allows for 
> easy inclusion / exclusion of GCs, there's no guarantee that a 
> specific combination of GCs is going to be maintained in the future. 
> Just like not all combinations of the different Runtime features (CDS, 
> NMT, JFR) and Compiler features (AOT, COMPILER1, COMPILER2) are 
> guaranteed to compile and be supported. I've sanity checked the 
> different GC configurations build locally, but I've only fully tested 
> the "server" variant, and "minimal" has only been built locally.
>
> There's a more high-level discussion around the flexibility the 
> --with-jvm-features flag adds, and if we really should allow it. Since 
> this patch only builds upon that existing flexibility, and I don't 
> change the two defined jvm variants, I'd appreciate if that discussion 
> could be kept out of this review thread. For further discussion around 
> this, please see:
>
> http://mail.openjdk.java.net/pipermail/build-dev/2018-April/021663.html
>
> This is the patch queue:
>
> The first patch simply cleans up some INCLUDE_ALL_GCS usages in 
> platform-specific files. Some of these changes are already being 
> cleaned up by other RFEs:
>
> http://cr.openjdk.java.net/~stefank/8200729/webrev.04/00.removeUnneededIncludeAllGCs/ 
>
>
>
> The second patch pre-cleans some include files:
>
> http://cr.openjdk.java.net/~stefank/8200729/webrev.04/01.fixIncludes/
>
>
> The following is the main patch, which include all relevant HotSpot 
> changes. For a while now, we have been actively working on abstracting 
> away GC specific code from non-GC directories, but as can be seen in 
> this patch, there are still a few places left. Hopefully, we will get 
> rid of most of these in the near future.
>
> http://cr.openjdk.java.net/~stefank/8200729/webrev.04/02.mainPatch/
>
>
> The last patch adds the make file support to turn on and off the 
> different GCs. The content of this patch has evolved from versions 
> written by myself, Per, and Magnus Ihse Bursie. Magnus last proposed 
> version used the names gc-cms, gc-g1, gc-parallel, and gc-serial, but 
> I've changed them to cmsgc, g1gc, parallelgc, and serialgc, so that 
> they match the INCLUDE_<GC> defines.
>
> http://cr.openjdk.java.net/~stefank/8200729/webrev.04/03.selectIndivudualGCsMakePatch/ 
>
>
> Thanks,
> StefanK