RFR 8078555(M): GC: implement ranges (optionally constraints) for those flags that have them missing
Kim Barrett
kim.barrett at oracle.com
Fri Aug 21 21:34:14 UTC 2015
Comments below are for:
http://cr.openjdk.java.net/~sangheki/8078555/webrev.01/
http://cr.openjdk.java.net/~sangheki/8078555/webrev.01_to_00/
No additional comments for:
http://cr.openjdk.java.net/~sangheki/8078555/webrev.02/
http://cr.openjdk.java.net/~sangheki/8078555/webrev.02_to_01
------------------------------------------------------------------------------
src/share/vm/runtime/commandLineFlagConstraintsGC.cpp
General comment - all the conditionalizations on INCLUDE_ALL_GCS
seriously uglifies this code. We should be conditionalizing for that,
but I wonder if there's a better way. Most of the conditionalizations
are for effectively the entire constraint function. Rather than that,
could we arrange for the relevant constraint functions to be
unreferenced when not needed, and then just have a block
conditionalization around the whole set of functions? One way to do
this might be to add a NOT_INCLUDE_ALL_GCS macro and wrap the
"constraint(...)" forms with that new macro where appropriate.
This probably should be addressed under a separate RFE though.
If I'd realized how ugly it would make the code, I probably wouldn't
have suggested doing the conditionalization now. But now that it's
here, I'm not sure its worth backing out.
------------------------------------------------------------------------------
src/share/vm/runtime/commandLineFlagConstraintsGC.cpp
63 Flag::Error ParallelGCThreadsConstraintFunc(uint value, bool verbose) {
[removed the following]
71 // G1, CMS and Parallel GC modes disallows ParallelGCThreads=0
72 if ((UseG1GC || UseConcMarkSweepGC || UseParallelGC) && (value == 0)) {
73 CommandLineError::print(verbose, "ParallelGCThreads cannot be zero\n");
74 return Flag::VIOLATES_CONSTRAINT;
75 }
Why was this removed?
------------------------------------------------------------------------------
src/share/vm/runtime/commandLineFlagConstraintsGC.cpp
#if INCLUDE_ALL_GCS
271 Flag::Error G1RSetRegionEntriesConstraintFunc(intx value, bool verbose) {
287 Flag::Error G1RSetSparseRegionEntriesConstraintFunc(intx value, bool verbose) {
303 Flag::Error G1YoungSurvRateNumRegionsSummaryConstraintFunc(intx value, bool verbose) {
317 Flag::Error G1HeapRegionSizeConstraintFunc(size_t value, bool verbose) {
332 Flag::Error G1NewSizePercentConstraintFunc(uintx value, bool verbose) {
346 Flag::Error G1MaxNewSizePercentConstraintFunc(uintx value, bool verbose) {
#endif
[pre-existing]
Shouldn't these G1-specific constraint functions (whose options are
declared in g1_globals.hpp) be declared and defined in G1-specific
files?
This should be handled as a separate issue.
------------------------------------------------------------------------------
src/share/vm/runtime/globals.hpp
1857 product(size_t, MarkStackSize, NOT_LP64(32*K) LP64_ONLY(4*M), \
1858 "Size of marking stack") \
1859 range(0, MarkStackSizeMax) \
I raised an issue here previously, and re-ordering the declarations in
the macros doesn't address the problem.
The value of MarkStackSizeMax will be read at the time of the call to
CommandLineFlagRangeList::init(). This is prior to any command line
option processing for a value for MarkStackSizeMax. So the range
won't reflect command line options.
Option values cannot be used in the computation of ranges.
Dependencies between options need to be handled via constraint
functions.
------------------------------------------------------------------------------
More information about the hotspot-dev
mailing list