RFR: 8200729: Conditional compilation of GCs

[Magnus Ihse Bursie]

On 2018-05-02 16:37, 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/

It's nice to see this cleanup in build logic!

I spotted one issue with the build logic, otherwise it looks good. In 
JvmFeatures.gmk, remove the  "JVM_EXCLUDE_FILES += none" lines. They are 
not needed and are technically incorrect (makes the build tries to 
exclude files named "none").

/Magnus

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