8048556: Unnecessary GCLocker-initiated young GCs

Fixed recognition of unnecessary GCLocker collections.

Reviewed-by: pliden, tschatzl
This commit is contained in:
Kim Barrett 2019-07-31 14:28:51 -04:00
parent caf7ee9bb5
commit 6d65b4fd25
8 changed files with 238 additions and 18 deletions

View File

@ -2154,6 +2154,12 @@ bool G1CollectedHeap::try_collect(GCCause::Cause cause, bool retry_on_gc_failure
GCLocker::stall_until_clear();
}
}
} else if (GCLocker::should_discard(cause, gc_count_before)) {
// Return false to be consistent with VMOp failure due to
// another collection slipping in after our gc_count but before
// our request is processed. _gc_locker collections upgraded by
// GCLockerInvokesConcurrent are handled above and never discarded.
return false;
} else {
if (cause == GCCause::_gc_locker || cause == GCCause::_wb_young_gc
DEBUG_ONLY(|| cause == GCCause::_scavenge_alot)) {

View File

@ -520,6 +520,10 @@ void ParallelScavengeHeap::collect(GCCause::Cause cause) {
full_gc_count = total_full_collections();
}
if (GCLocker::should_discard(cause, gc_count)) {
return;
}
VM_ParallelGCSystemGC op(gc_count, full_gc_count, cause);
VMThread::execute(&op);
}

View File

@ -49,11 +49,16 @@ void VM_ParallelGCFailedAllocation::doit() {
}
}
static bool is_cause_full(GCCause::Cause cause) {
return (cause != GCCause::_gc_locker) && (cause != GCCause::_wb_young_gc)
DEBUG_ONLY(&& (cause != GCCause::_scavenge_alot));
}
// Only used for System.gc() calls
VM_ParallelGCSystemGC::VM_ParallelGCSystemGC(uint gc_count,
uint full_gc_count,
GCCause::Cause gc_cause) :
VM_GC_Operation(gc_count, gc_cause, full_gc_count, true /* full */)
VM_GC_Operation(gc_count, gc_cause, full_gc_count, is_cause_full(gc_cause))
{
}
@ -63,8 +68,7 @@ void VM_ParallelGCSystemGC::doit() {
ParallelScavengeHeap* heap = ParallelScavengeHeap::heap();
GCCauseSetter gccs(heap, _gc_cause);
if (_gc_cause == GCCause::_gc_locker || _gc_cause == GCCause::_wb_young_gc
DEBUG_ONLY(|| _gc_cause == GCCause::_scavenge_alot)) {
if (!_full) {
// If (and only if) the scavenge fails, this will invoke a full gc.
heap->invoke_scavenge();
} else {

View File

@ -36,6 +36,7 @@
volatile jint GCLocker::_jni_lock_count = 0;
volatile bool GCLocker::_needs_gc = false;
volatile bool GCLocker::_doing_gc = false;
unsigned int GCLocker::_total_collections = 0;
#ifdef ASSERT
volatile jint GCLocker::_debug_jni_lock_count = 0;
@ -115,6 +116,11 @@ void GCLocker::stall_until_clear() {
}
}
bool GCLocker::should_discard(GCCause::Cause cause, uint total_collections) {
return (cause == GCCause::_gc_locker) &&
(_total_collections != total_collections);
}
void GCLocker::jni_lock(JavaThread* thread) {
assert(!thread->in_critical(), "shouldn't currently be in a critical region");
MonitorLocker ml(JNICritical_lock);
@ -138,7 +144,13 @@ void GCLocker::jni_unlock(JavaThread* thread) {
decrement_debug_jni_lock_count();
thread->exit_critical();
if (needs_gc() && !is_active_internal()) {
// We're the last thread out. Cause a GC to occur.
// We're the last thread out. Request a GC.
// Capture the current total collections, to allow detection of
// other collections that make this one unnecessary. The value of
// total_collections() is only changed at a safepoint, so there
// must not be a safepoint between the lock becoming inactive and
// getting the count, else there may be unnecessary GCLocker GCs.
_total_collections = Universe::heap()->total_collections();
_doing_gc = true;
{
// Must give up the lock while at a safepoint

View File

@ -25,6 +25,7 @@
#ifndef SHARE_GC_SHARED_GCLOCKER_HPP
#define SHARE_GC_SHARED_GCLOCKER_HPP
#include "gc/shared/gcCause.hpp"
#include "memory/allocation.hpp"
#include "utilities/globalDefinitions.hpp"
#include "utilities/macros.hpp"
@ -45,6 +46,7 @@ class GCLocker: public AllStatic {
static volatile bool _needs_gc; // heap is filling, we need a GC
// note: bool is typedef'd as jint
static volatile bool _doing_gc; // unlock_critical() is doing a GC
static uint _total_collections; // value for _gc_locker collection
#ifdef ASSERT
// This lock count is updated for all operations and is used to
@ -98,6 +100,12 @@ class GCLocker: public AllStatic {
// Sets _needs_gc if is_active() is true. Returns is_active().
static bool check_active_before_gc();
// Return true if the designated collection is a GCLocker request
// that should be discarded. Returns true if cause == GCCause::_gc_locker
// and the given total collection value indicates a collection has been
// done since the GCLocker request was made.
static bool should_discard(GCCause::Cause cause, uint total_collections);
// Stalls the caller (who should not be in a jni critical section)
// until needs_gc() clears. Note however that needs_gc() may be
// set at a subsequent safepoint and/or cleared under the

View File

@ -194,7 +194,8 @@ class VM_GenCollectFull: public VM_GC_Operation {
uint full_gc_count_before,
GCCause::Cause gc_cause,
GenCollectedHeap::GenerationType max_generation)
: VM_GC_Operation(gc_count_before, gc_cause, full_gc_count_before, true /* full */),
: VM_GC_Operation(gc_count_before, gc_cause, full_gc_count_before,
max_generation != GenCollectedHeap::YoungGen /* full */),
_max_generation(max_generation) { }
~VM_GenCollectFull() {}
virtual VMOp_Type type() const { return VMOp_GenCollectFull; }

View File

@ -947,8 +947,9 @@ HeapWord** GenCollectedHeap::end_addr() const {
// public collection interfaces
void GenCollectedHeap::collect(GCCause::Cause cause) {
if (cause == GCCause::_wb_young_gc) {
// Young collection for the WhiteBox API.
if ((cause == GCCause::_wb_young_gc) ||
(cause == GCCause::_gc_locker)) {
// Young collection for WhiteBox or GCLocker.
collect(cause, YoungGen);
} else {
#ifdef ASSERT
@ -986,6 +987,11 @@ void GenCollectedHeap::collect_locked(GCCause::Cause cause, GenerationType max_g
// Read the GC count while holding the Heap_lock
unsigned int gc_count_before = total_collections();
unsigned int full_gc_count_before = total_full_collections();
if (GCLocker::should_discard(cause, gc_count_before)) {
return;
}
{
MutexUnlocker mu(Heap_lock); // give up heap lock, execute gets it back
VM_GenCollectFull op(gc_count_before, full_gc_count_before,
@ -1000,24 +1006,15 @@ void GenCollectedHeap::do_full_collection(bool clear_all_soft_refs) {
void GenCollectedHeap::do_full_collection(bool clear_all_soft_refs,
GenerationType last_generation) {
GenerationType local_last_generation;
if (!incremental_collection_will_fail(false /* don't consult_young */) &&
gc_cause() == GCCause::_gc_locker) {
local_last_generation = YoungGen;
} else {
local_last_generation = last_generation;
}
do_collection(true, // full
clear_all_soft_refs, // clear_all_soft_refs
0, // size
false, // is_tlab
local_last_generation); // last_generation
last_generation); // last_generation
// Hack XXX FIX ME !!!
// A scavenge may not have been attempted, or may have
// been attempted and failed, because the old gen was too full
if (local_last_generation == YoungGen && gc_cause() == GCCause::_gc_locker &&
incremental_collection_will_fail(false /* don't consult_young */)) {
if (gc_cause() == GCCause::_gc_locker && incremental_collection_failed()) {
log_debug(gc, jni)("GC locker: Trying a full collection because scavenge failed");
// This time allow the old gen to be collected as well
do_collection(true, // full

View File

@ -0,0 +1,188 @@
/*
* Copyright (c) 2019, 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.stress.gclocker;
/*
* @test TestExcessGCLockerCollections
* @key gc
* @bug 8048556
* @summary Check for GC Locker initiated GCs that immediately follow another
* GC and so have very little needing to be collected.
* @requires vm.gc != "Z"
* @requires vm.gc != "Epsilon"
* @requires vm.gc != "Shenandoah"
* @library /test/lib
* @modules java.base/jdk.internal.misc
* @run driver/timeout=1000 gc.stress.gclocker.TestExcessGCLockerCollections 300 4 2
*/
import java.util.HashMap;
import java.util.Map;
import java.util.zip.Deflater;
import java.util.ArrayList;
import java.util.Arrays;
import jdk.test.lib.Asserts;
import jdk.test.lib.process.ProcessTools;
import jdk.test.lib.process.OutputAnalyzer;
class TestExcessGCLockerCollectionsAux {
static private final int LARGE_MAP_SIZE = 64 * 1024;
static private final int MAP_ARRAY_LENGTH = 4;
static private final int MAP_SIZE = 1024;
static private final int BYTE_ARRAY_LENGTH = 128 * 1024;
static private void println(String str) { System.out.println(str); }
static private volatile boolean keepRunning = true;
static Map<Integer,String> populateMap(int size) {
Map<Integer,String> map = new HashMap<Integer,String>();
for (int i = 0; i < size; i += 1) {
Integer keyInt = Integer.valueOf(i);
String valStr = "value is [" + i + "]";
map.put(keyInt,valStr);
}
return map;
}
static private class AllocatingWorker implements Runnable {
private final Object[] array = new Object[MAP_ARRAY_LENGTH];
private int arrayIndex = 0;
private void doStep() {
Map<Integer,String> map = populateMap(MAP_SIZE);
array[arrayIndex] = map;
arrayIndex = (arrayIndex + 1) % MAP_ARRAY_LENGTH;
}
public void run() {
while (keepRunning) {
doStep();
}
}
}
static private class JNICriticalWorker implements Runnable {
private int count;
private void doStep() {
byte[] inputArray = new byte[BYTE_ARRAY_LENGTH];
for (int i = 0; i < inputArray.length; i += 1) {
inputArray[i] = (byte) (count + i);
}
Deflater deflater = new Deflater();
deflater.setInput(inputArray);
deflater.finish();
byte[] outputArray = new byte[2 * inputArray.length];
deflater.deflate(outputArray);
count += 1;
}
public void run() {
while (keepRunning) {
doStep();
}
}
}
static public Map<Integer,String> largeMap;
static public void main(String args[]) {
long durationSec = Long.parseLong(args[0]);
int allocThreadNum = Integer.parseInt(args[1]);
int jniCriticalThreadNum = Integer.parseInt(args[2]);
println("Running for " + durationSec + " secs");
largeMap = populateMap(LARGE_MAP_SIZE);
println("Starting " + allocThreadNum + " allocating threads");
for (int i = 0; i < allocThreadNum; i += 1) {
new Thread(new AllocatingWorker()).start();
}
println("Starting " + jniCriticalThreadNum + " jni critical threads");
for (int i = 0; i < jniCriticalThreadNum; i += 1) {
new Thread(new JNICriticalWorker()).start();
}
long durationMS = (long) (1000 * durationSec);
long start = System.currentTimeMillis();
long now = start;
long soFar = now - start;
while (soFar < durationMS) {
try {
Thread.sleep(durationMS - soFar);
} catch (Exception e) {
}
now = System.currentTimeMillis();
soFar = now - start;
}
println("Done.");
keepRunning = false;
}
}
public class TestExcessGCLockerCollections {
private static final String locker =
"\\[gc\\s*\\] .* \\(GCLocker Initiated GC\\)";
private static final String ANY_LOCKER = locker + " [1-9][0-9]*M";
private static final String BAD_LOCKER = locker + " [1-9][0-9]?M";
private static final String[] COMMON_OPTIONS = new String[] {
"-Xmx1G", "-Xms1G", "-Xmn256M", "-Xlog:gc" };
public static void main(String args[]) throws Exception {
if (args.length < 3) {
System.out.println("usage: TestExcessGCLockerCollectionsAux" +
" <duration sec> <alloc threads>" +
" <jni critical threads>");
throw new RuntimeException("Invalid arguments");
}
ArrayList<String> finalArgs = new ArrayList<String>();
finalArgs.addAll(Arrays.asList(COMMON_OPTIONS));
finalArgs.add(TestExcessGCLockerCollectionsAux.class.getName());
finalArgs.addAll(Arrays.asList(args));
// GC and other options obtained from test framework.
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
true, finalArgs.toArray(new String[0]));
OutputAnalyzer output = new OutputAnalyzer(pb.start());
output.shouldHaveExitValue(0);
//System.out.println("------------- begin stdout ----------------");
//System.out.println(output.getStdout());
//System.out.println("------------- end stdout ----------------");
output.stdoutShouldMatch(ANY_LOCKER);
output.stdoutShouldNotMatch(BAD_LOCKER);
}
}