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
