I see that you don't guard Use*GC flags with _ONLY macros. It is hard to
figure out from gcConfig.cpp what happened if UseG1GC is specified on
command line for VM which does not include it. What happens?
I don't see C1 changes but it looks like it was changed with 8201543.
C2 changes seems fine but they will be also affected by 8202377.
What is left is AOT and JVMCI/Graal. We thought to implement "cross"
compilation when we can specify for which configuration we should
generate AOT code. It includes GC barriers code. Currently I see that
you keep all GCs in build as before so it is not a issue. And we record
VM version so we will not generate and use G1 barriers if it is not in
VM used by jaotc.
So it seems to me compiler and AOT, Graal changes are fine.
I would suggest in addition to hs-tier2 testing run hs-tier2-graal to
make sure Graal still works.
Thanks,
Vladimir
On 5/2/18 7: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
