RFR: 8200729: Conditional compilation of GCs

[Stefan Karlsson]

On 2018-05-03 11:06, Magnus Ihse Bursie wrote:
> 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").


Thanks for reviewing this, and helping out writing the make files 
changes! I'll remove the += none lines.

StefanK

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