# HG changeset patch # User kbarrett # Date 1564597731 14400 # Node ID 8d3886985964cade6e469dcf2b00a12db9e8e516 # Parent b81062d47d619c99da12b946ec1a7ddd6f58772f 8048556: Unnecessary GCLocker-initiated young GCs Summary: Fixed recognition of unnecessary GCLocker collections. Reviewed-by: pliden, tschatzl diff -r b81062d47d61 -r 8d3886985964 src/hotspot/share/gc/g1/g1CollectedHeap.cpp --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp Wed Jul 31 13:40:59 2019 -0400 +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp Wed Jul 31 14:28:51 2019 -0400 @@ -2154,6 +2154,12 @@ 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)) { diff -r b81062d47d61 -r 8d3886985964 src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp --- a/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp Wed Jul 31 13:40:59 2019 -0400 +++ b/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp Wed Jul 31 14:28:51 2019 -0400 @@ -520,6 +520,10 @@ 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); } diff -r b81062d47d61 -r 8d3886985964 src/hotspot/share/gc/parallel/psVMOperations.cpp --- a/src/hotspot/share/gc/parallel/psVMOperations.cpp Wed Jul 31 13:40:59 2019 -0400 +++ b/src/hotspot/share/gc/parallel/psVMOperations.cpp Wed Jul 31 14:28:51 2019 -0400 @@ -49,11 +49,16 @@ } } +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 @@ 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 { diff -r b81062d47d61 -r 8d3886985964 src/hotspot/share/gc/shared/gcLocker.cpp --- a/src/hotspot/share/gc/shared/gcLocker.cpp Wed Jul 31 13:40:59 2019 -0400 +++ b/src/hotspot/share/gc/shared/gcLocker.cpp Wed Jul 31 14:28:51 2019 -0400 @@ -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 @@ } } +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 @@ 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 diff -r b81062d47d61 -r 8d3886985964 src/hotspot/share/gc/shared/gcLocker.hpp --- a/src/hotspot/share/gc/shared/gcLocker.hpp Wed Jul 31 13:40:59 2019 -0400 +++ b/src/hotspot/share/gc/shared/gcLocker.hpp Wed Jul 31 14:28:51 2019 -0400 @@ -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 @@ 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 @@ // 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 diff -r b81062d47d61 -r 8d3886985964 src/hotspot/share/gc/shared/gcVMOperations.hpp --- a/src/hotspot/share/gc/shared/gcVMOperations.hpp Wed Jul 31 13:40:59 2019 -0400 +++ b/src/hotspot/share/gc/shared/gcVMOperations.hpp Wed Jul 31 14:28:51 2019 -0400 @@ -194,7 +194,8 @@ 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; } diff -r b81062d47d61 -r 8d3886985964 src/hotspot/share/gc/shared/genCollectedHeap.cpp --- a/src/hotspot/share/gc/shared/genCollectedHeap.cpp Wed Jul 31 13:40:59 2019 -0400 +++ b/src/hotspot/share/gc/shared/genCollectedHeap.cpp Wed Jul 31 14:28:51 2019 -0400 @@ -947,8 +947,9 @@ // 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 @@ // 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, 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 diff -r b81062d47d61 -r 8d3886985964 test/hotspot/jtreg/gc/stress/gclocker/TestExcessGCLockerCollections.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/hotspot/jtreg/gc/stress/gclocker/TestExcessGCLockerCollections.java Wed Jul 31 14:28:51 2019 -0400 @@ -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 populateMap(int size) { + Map map = new HashMap(); + 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 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 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" + + " " + + " "); + throw new RuntimeException("Invalid arguments"); + } + + ArrayList finalArgs = new ArrayList(); + 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); + } +}