Skip to content
Permalink
Browse files

8238855: Move G1ConcurrentMark flag sanity checks to g1Arguments

Reviewed-by: sjohanss, kbarrett
  • Loading branch information
Thomas Schatzl committed Mar 27, 2020
1 parent 67cf35e commit 7048684ca7ded8e95aa15c7a94d8768d899ccbe6
@@ -97,6 +97,23 @@ void G1Arguments::parse_verification_type(const char* type) {
}
}

// Returns the maximum number of workers to be used in a concurrent
// phase based on the number of GC workers being used in a STW
// phase.
static uint scale_concurrent_worker_threads(uint num_gc_workers) {
return MAX2((num_gc_workers + 2) / 4, 1U);
}

void G1Arguments::initialize_mark_stack_size() {
if (FLAG_IS_DEFAULT(MarkStackSize)) {
size_t mark_stack_size = MIN2(MarkStackSizeMax,
MAX2(MarkStackSize, (size_t)ConcGCThreads * TASKQUEUE_SIZE));
FLAG_SET_ERGO(MarkStackSize, mark_stack_size);
}

log_trace(gc)("MarkStackSize: %uk MarkStackSizeMax: %uk", (uint)(MarkStackSize / K), (uint)(MarkStackSizeMax / K));
}

void G1Arguments::initialize() {
GCArguments::initialize();
assert(UseG1GC, "Error");
@@ -117,12 +134,11 @@ void G1Arguments::initialize() {
FLAG_SET_ERGO(G1ConcRefinementThreads, ParallelGCThreads);
}

// MarkStackSize will be set (if it hasn't been set by the user)
// when concurrent marking is initialized.
// Its value will be based upon the number of parallel marking threads.
// But we do set the maximum mark stack size here.
if (FLAG_IS_DEFAULT(MarkStackSizeMax)) {
FLAG_SET_DEFAULT(MarkStackSizeMax, 128 * TASKQUEUE_SIZE);
if (FLAG_IS_DEFAULT(ConcGCThreads) || ConcGCThreads == 0) {
// Calculate the number of concurrent worker threads by scaling
// the number of parallel GC threads.
uint marking_thread_num = scale_concurrent_worker_threads(ParallelGCThreads);
FLAG_SET_ERGO(ConcGCThreads, marking_thread_num);
}

if (FLAG_IS_DEFAULT(GCTimeRatio) || GCTimeRatio == 0) {
@@ -158,8 +174,6 @@ void G1Arguments::initialize() {
FLAG_SET_DEFAULT(ParallelRefProcEnabled, true);
}

log_trace(gc)("MarkStackSize: %uk MarkStackSizeMax: %uk", (unsigned int) (MarkStackSize / K), (uint) (MarkStackSizeMax / K));

// By default do not let the target stack size to be more than 1/4 of the entries
if (FLAG_IS_DEFAULT(GCDrainStackTargetSize)) {
FLAG_SET_ERGO(GCDrainStackTargetSize, MIN2(GCDrainStackTargetSize, (uintx)TASKQUEUE_SIZE / 4));
@@ -175,6 +189,7 @@ void G1Arguments::initialize() {
}
#endif

initialize_mark_stack_size();
initialize_verification_types();
}

@@ -36,6 +36,7 @@ class G1Arguments : public GCArguments {
private:
static size_t MaxMemoryForYoung;

static void initialize_mark_stack_size();
static void initialize_verification_types();
static void parse_verification_type(const char* type);

@@ -1800,10 +1800,6 @@ jint G1CollectedHeap::initialize() {
// Create the G1ConcurrentMark data structure and thread.
// (Must do this late, so that "max_regions" is defined.)
_cm = new G1ConcurrentMark(this, prev_bitmap_storage, next_bitmap_storage);
if (!_cm->completed_initialization()) {
vm_shutdown_during_initialization("Could not initialize G1ConcurrentMark");
return JNI_ENOMEM;
}
_cm_thread = _cm->cm_thread();

// Now expand into the initial heap size.
@@ -353,19 +353,11 @@ bool G1CMRootMemRegions::wait_until_scan_finished() {
return true;
}

// Returns the maximum number of workers to be used in a concurrent
// phase based on the number of GC workers being used in a STW
// phase.
static uint scale_concurrent_worker_threads(uint num_gc_workers) {
return MAX2((num_gc_workers + 2) / 4, 1U);
}

G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h,
G1RegionToSpaceMapper* prev_bitmap_storage,
G1RegionToSpaceMapper* next_bitmap_storage) :
// _cm_thread set inside the constructor
_g1h(g1h),
_completed_initialization(false),

_mark_bitmap_1(),
_mark_bitmap_2(),
@@ -416,6 +408,8 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h,
_region_mark_stats(NEW_C_HEAP_ARRAY(G1RegionMarkStats, _g1h->max_regions(), mtGC)),
_top_at_rebuild_starts(NEW_C_HEAP_ARRAY(HeapWord*, _g1h->max_regions(), mtGC))
{
assert(CGC_lock != NULL, "CGC_lock must be initialized");

_mark_bitmap_1.initialize(g1h->reserved_region(), prev_bitmap_storage);
_mark_bitmap_2.initialize(g1h->reserved_region(), next_bitmap_storage);

@@ -425,22 +419,6 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h,
vm_shutdown_during_initialization("Could not create ConcurrentMarkThread");
}

assert(CGC_lock != NULL, "CGC_lock must be initialized");

if (FLAG_IS_DEFAULT(ConcGCThreads) || ConcGCThreads == 0) {
// Calculate the number of concurrent worker threads by scaling
// the number of parallel GC threads.
uint marking_thread_num = scale_concurrent_worker_threads(ParallelGCThreads);
FLAG_SET_ERGO(ConcGCThreads, marking_thread_num);
}

assert(ConcGCThreads > 0, "ConcGCThreads have been set.");
if (ConcGCThreads > ParallelGCThreads) {
log_warning(gc)("More ConcGCThreads (%u) than ParallelGCThreads (%u).",
ConcGCThreads, ParallelGCThreads);
return;
}

log_debug(gc)("ConcGCThreads: %u offset %u", ConcGCThreads, _worker_id_offset);
log_debug(gc)("ParallelGCThreads: %u", ParallelGCThreads);

@@ -450,40 +428,6 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h,
_concurrent_workers = new WorkGang("G1 Conc", _max_concurrent_workers, false, true);
_concurrent_workers->initialize_workers();

if (FLAG_IS_DEFAULT(MarkStackSize)) {
size_t mark_stack_size =
MIN2(MarkStackSizeMax,
MAX2(MarkStackSize, (size_t) (_max_concurrent_workers * TASKQUEUE_SIZE)));
// Verify that the calculated value for MarkStackSize is in range.
// It would be nice to use the private utility routine from Arguments.
if (!(mark_stack_size >= 1 && mark_stack_size <= MarkStackSizeMax)) {
log_warning(gc)("Invalid value calculated for MarkStackSize (" SIZE_FORMAT "): "
"must be between 1 and " SIZE_FORMAT,
mark_stack_size, MarkStackSizeMax);
return;
}
FLAG_SET_ERGO(MarkStackSize, mark_stack_size);
} else {
// Verify MarkStackSize is in range.
if (FLAG_IS_CMDLINE(MarkStackSize)) {
if (FLAG_IS_DEFAULT(MarkStackSizeMax)) {
if (!(MarkStackSize >= 1 && MarkStackSize <= MarkStackSizeMax)) {
log_warning(gc)("Invalid value specified for MarkStackSize (" SIZE_FORMAT "): "
"must be between 1 and " SIZE_FORMAT,
MarkStackSize, MarkStackSizeMax);
return;
}
} else if (FLAG_IS_CMDLINE(MarkStackSizeMax)) {
if (!(MarkStackSize >= 1 && MarkStackSize <= MarkStackSizeMax)) {
log_warning(gc)("Invalid value specified for MarkStackSize (" SIZE_FORMAT ")"
" or for MarkStackSizeMax (" SIZE_FORMAT ")",
MarkStackSize, MarkStackSizeMax);
return;
}
}
}
}

if (!_global_mark_stack.initialize(MarkStackSize, MarkStackSizeMax)) {
vm_exit_during_initialization("Failed to allocate initial concurrent mark overflow mark stack.");
}
@@ -505,7 +449,6 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h,
}

reset_at_marking_complete();
_completed_initialization = true;
}

void G1ConcurrentMark::reset() {
@@ -303,7 +303,6 @@ class G1ConcurrentMark : public CHeapObj<mtGC> {

G1ConcurrentMarkThread* _cm_thread; // The thread doing the work
G1CollectedHeap* _g1h; // The heap
bool _completed_initialization; // Set to true when initialization is complete

// Concurrent marking support structures
G1CMBitMap _mark_bitmap_1;
@@ -604,11 +603,6 @@ class G1ConcurrentMark : public CHeapObj<mtGC> {

inline bool is_marked_in_next_bitmap(oop p) const;

// Returns true if initialization was successfully completed.
bool completed_initialization() const {
return _completed_initialization;
}

ConcurrentGCTimer* gc_timer_cm() const { return _gc_timer_cm; }
G1OldTracer* gc_tracer_cm() const { return _gc_tracer_cm; }

@@ -258,6 +258,7 @@
product(size_t, MarkStackSize, NOT_LP64(32*K) LP64_ONLY(4*M), \
"Size of marking stack") \
constraint(MarkStackSizeConstraintFunc,AfterErgo) \
range(1, (max_jint - 1)) \
\
product(intx, RefDiscoveryPolicy, 0, \
"Select type of reference discovery policy: " \
@@ -172,6 +172,7 @@ JVMFlag::Error SoftRefLRUPolicyMSPerMBConstraintFunc(intx value, bool verbose) {
}

JVMFlag::Error MarkStackSizeConstraintFunc(size_t value, bool verbose) {
// value == 0 is handled by the range constraint.
if (value > MarkStackSizeMax) {
JVMFlag::printError(verbose,
"MarkStackSize (" SIZE_FORMAT ") must be "
@@ -0,0 +1,91 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package gc.g1;

/*
* @test TestMarkStackSizes
* @key gc regression
* @bug 8238855
* @summary Consistency checks for marking flag related options.
* @requires vm.gc.G1
* @library /test/lib
* @modules java.base/jdk.internal.misc
* java.management
* @run main gc.g1.TestMarkStackSizes
*/

import java.util.ArrayList;
import java.util.Collections;

import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.process.ProcessTools;

public class TestMarkStackSizes {
private static void runTest(boolean shouldSucceed, String... extraArgs) throws Exception {
ArrayList<String> testArguments = new ArrayList<String>();

testArguments.add("-XX:+UseG1GC");
testArguments.add("-Xmx12m");
testArguments.add("-XX:+PrintFlagsFinal");
Collections.addAll(testArguments, extraArgs);
testArguments.add("-version");

ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(testArguments.toArray(new String[0]));

OutputAnalyzer output = new OutputAnalyzer(pb.start());

System.out.println(output.getStderr());

if (shouldSucceed) {
long markStackSize = Long.parseLong(output.firstMatch("MarkStackSize\\s+=\\s(\\d+)",1));
long markStackSizeMax = Long.parseLong(output.firstMatch("MarkStackSizeMax\\s+=\\s(\\d+)",1));
long concGCThreads = Long.parseLong(output.firstMatch("ConcGCThreads\\s+=\\s(\\d+)",1));
long parallelGCThreads = Long.parseLong(output.firstMatch("ParallelGCThreads\\s+=\\s(\\d+)",1));

System.out.println("MarkStackSize=" + markStackSize + " MarkStackSizeMax=" + markStackSizeMax +
"ConcGCThreads=" + concGCThreads + " ParallelGCThreads=" + parallelGCThreads);

output.shouldHaveExitValue(0);
} else {
output.shouldNotHaveExitValue(0);
}
}

public static void main(String[] args) throws Exception {
runTest(false, "-XX:MarkStackSize=0");
runTest(false, "-XX:MarkStackSizeMax=0");

runTest(true, "-XX:MarkStackSize=100", "-XX:MarkStackSizeMax=101");
runTest(true, "-XX:MarkStackSize=101", "-XX:MarkStackSizeMax=101");
runTest(false, "-XX:MarkStackSize=101", "-XX:MarkStackSizeMax=100");

runTest(true, "-XX:ConcGCThreads=3", "-XX:ParallelGCThreads=3");
runTest(true, "-XX:ConcGCThreads=0", "-XX:ParallelGCThreads=3");
runTest(false, "-XX:ConcGCThreads=4", "-XX:ParallelGCThreads=3");

// With that high ParallelGCThreads the default ergonomics would calculate
// a mark stack size higher than maximum mark stack size.
runTest(true, "-XX:ParallelGCThreads=100", "-XX:MarkStackSizeMax=100");
}
}

0 comments on commit 7048684

Please sign in to comment.