Hi Vladimir,
On 2018-05-03 18:56, Vladimir Kozlov wrote:
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?
Right. The current patch leaves the Use*GC flags available in all
builds, so that we can accept the flag but give a warning:
$ build/slowdebug-no-g1/jdk/bin/java -XX:+UseG1GC
Java HotSpot(TM) 64-Bit Server VM warning: -XX:+UseG1GC not supported in
this VM
This is handled in GCConfig::select_gc_ergonomically with this line:
NOT_G1GC( UNSUPPORTED_OPTION(UseG1GC);)
That expands into:
// Disable options not supported in this release, with a warning if they
// were explicitly requested on the command-line
#define UNSUPPORTED_OPTION(opt) \
do { \
if (opt) { \
if (FLAG_IS_CMDLINE(opt)) { \
warning("-XX:+" #opt " not supported in this VM"); \
} \
FLAG_SET_DEFAULT(opt, false); \
} \
} while(0)
I don't see C1 changes but it looks like it was changed with 8201543.
Right. There's no INCLUDE_<GC> guards left in C1!
C2 changes seems fine but they will be also affected by 8202377.
Exactly.
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.
OK. Thanks for taking a close look at this.
I would suggest in addition to hs-tier2 testing run hs-tier2-graal to
make sure Graal still works.
I'll make sure to test with hs-tier2-graal.
Thanks for reviewing!
StefanK
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
