diff --git a/src/hotspot/share/gc/g1/g1ServiceThread.cpp b/src/hotspot/share/gc/g1/g1ServiceThread.cpp index 02762b375a043..3ff98089b595e 100644 --- a/src/hotspot/share/gc/g1/g1ServiceThread.cpp +++ b/src/hotspot/share/gc/g1/g1ServiceThread.cpp @@ -210,8 +210,7 @@ void G1ServiceThread::register_task(G1ServiceTask* task, jlong delay) { // Notify the service thread that there is a new task, thread might // be waiting and the newly added task might be first in the list. - MonitorLocker ml(&_monitor, Mutex::_no_safepoint_check_flag); - ml.notify(); + notify(); } void G1ServiceThread::schedule_task(G1ServiceTask* task, jlong delay_ms) { @@ -243,6 +242,11 @@ int64_t G1ServiceThread::time_to_next_task_ms() { return (int64_t) TimeHelper::counter_to_millis(time_diff); } +void G1ServiceThread::notify() { + MonitorLocker ml(&_monitor, Mutex::_no_safepoint_check_flag); + ml.notify(); +} + void G1ServiceThread::sleep_before_next_cycle() { if (should_terminate()) { return; @@ -309,8 +313,7 @@ void G1ServiceThread::run_service() { } void G1ServiceThread::stop_service() { - MonitorLocker ml(&_monitor, Mutex::_no_safepoint_check_flag); - ml.notify(); + notify(); } G1ServiceTask::G1ServiceTask(const char* name) : diff --git a/src/hotspot/share/gc/g1/g1ServiceThread.hpp b/src/hotspot/share/gc/g1/g1ServiceThread.hpp index ec86dead87981..463f10677f564 100644 --- a/src/hotspot/share/gc/g1/g1ServiceThread.hpp +++ b/src/hotspot/share/gc/g1/g1ServiceThread.hpp @@ -133,6 +133,9 @@ class G1ServiceThread: public ConcurrentGCThread { // Register a task with the service thread and schedule it. If // no delay is specified the task is scheduled to run directly. void register_task(G1ServiceTask* task, jlong delay = 0); + // Notify a change to the service thread. Used to stop either + // stop the service or to force check for new tasks. + void notify(); }; #endif // SHARE_GC_G1_G1SERVICETHREAD_HPP diff --git a/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp b/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp index 58e3e7e61322f..d5401ed1a710a 100644 --- a/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp +++ b/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp @@ -26,6 +26,7 @@ #include "gc/g1/g1CollectedHeap.inline.hpp" #include "gc/g1/g1UncommitRegionTask.hpp" #include "gc/shared/suspendibleThreadSet.hpp" +#include "runtime/globals.hpp" #include "utilities/ticks.hpp" G1UncommitRegionTask* G1UncommitRegionTask::_instance = NULL; @@ -58,8 +59,11 @@ void G1UncommitRegionTask::activate() { G1UncommitRegionTask* uncommit_task = instance(); if (!uncommit_task->is_active()) { + // Change state to active and schedule with no delay. uncommit_task->set_state(TaskState::active); uncommit_task->schedule(0); + // Notify service thread to avoid unnecesarry waiting. + G1CollectedHeap::heap()->service_thread()->notify(); } } @@ -70,7 +74,6 @@ bool G1UncommitRegionTask::is_active() { void G1UncommitRegionTask::set_state(TaskState state) { assert(_state != state, "Must do a state change"); _state = state; - log_trace(gc, heap)("%s, new state: %s", name(), is_active() ? "active" : "inactive"); } void G1UncommitRegionTask::report_execution(Tickspan time, uint regions) { @@ -100,12 +103,15 @@ void G1UncommitRegionTask::clear_summary() { void G1UncommitRegionTask::execute() { assert(_state == TaskState::active, "Must be active"); + // Each execution is limited to uncommit at most 256M worth of regions. + static const uint region_limit = (uint) (256 * M / G1HeapRegionSize); + // Prevent from running during a GC pause. SuspendibleThreadSetJoiner sts; HeapRegionManager* hrm = G1CollectedHeap::heap()->hrm(); Ticks start = Ticks::now(); - uint uncommit_count = hrm->uncommit_inactive_regions(UncommitRegionLimit); + uint uncommit_count = hrm->uncommit_inactive_regions(region_limit); Tickspan uncommit_time = (Ticks::now() - start); if (uncommit_count > 0) { diff --git a/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp b/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp index 089aa9cd904cf..a0fd15cd86561 100644 --- a/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp +++ b/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp @@ -30,10 +30,6 @@ #include "utilities/ticks.hpp" class G1UncommitRegionTask : public G1ServiceTask { - // Limit a single execution of the task to uncommit at most - // this many regions. - static const uint UncommitRegionLimit = 32; - static G1UncommitRegionTask* _instance; static void initialize(); static G1UncommitRegionTask* instance(); diff --git a/test/hotspot/jtreg/gc/stress/TestStressG1Uncommit.java b/test/hotspot/jtreg/gc/stress/TestStressG1Uncommit.java index 913ba68a60aa8..e256ea405c2c2 100644 --- a/test/hotspot/jtreg/gc/stress/TestStressG1Uncommit.java +++ b/test/hotspot/jtreg/gc/stress/TestStressG1Uncommit.java @@ -30,8 +30,6 @@ * @requires vm.gc.G1 * @library /test/lib * @modules java.base/jdk.internal.misc - * @build sun.hotspot.WhiteBox - * @run driver ClassFileInstaller sun.hotspot.WhiteBox * @run driver/timeout=1300 gc.stress.TestStressG1Uncommit */ @@ -53,26 +51,13 @@ import jdk.test.lib.process.ProcessTools; import jdk.test.lib.process.OutputAnalyzer; -import sun.hotspot.WhiteBox; - public class TestStressG1Uncommit { public static void main(String[] args) throws Exception { - runTest("Full"); - runTest("Concurrent"); - } - - // Run the test with explicit arguments to make sure heap is not - // fixed and that we get the expected log tags. - private static void runTest(String gcType) throws Exception{ ArrayList options = new ArrayList<>(); Collections.addAll(options, - "-Xbootclasspath/a:.", - "-XX:+UnlockDiagnosticVMOptions", - "-XX:+WhiteBoxAPI", "-Xlog:gc,gc+heap+region=debug", "-XX:+UseG1GC", - StressUncommit.class.getName(), - gcType + StressUncommit.class.getName() ); ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(options); OutputAnalyzer output = new OutputAnalyzer(pb.start()); @@ -87,18 +72,11 @@ class StressUncommit { private static final long G = 1024 * M; private static final Instant StartTime = Instant.now(); - private static final WhiteBox wb = WhiteBox.getWhiteBox(); private static final ThreadMXBean threadBean = (ThreadMXBean) ManagementFactory.getThreadMXBean(); private static final MemoryMXBean memoryBean = ManagementFactory.getMemoryMXBean(); private static ConcurrentLinkedQueue globalKeepAlive; public static void main(String args[]) throws InterruptedException { - // Get GC type for this run. - if (args.length != 1) { - throw new IllegalArgumentException("Missing argument: Full or Concurrent"); - } - GCType gc = GCType.valueOf(args[0]); - // Leave 20% head room to try to avoid Full GCs. long allocationSize = (long) (Runtime.getRuntime().maxMemory() * 0.8); @@ -127,13 +105,13 @@ public static void main(String args[]) throws InterruptedException { // Wait for tasks to complete. workersRunning.await(); - long committedBefore = memoryBean.getHeapMemoryUsage().getCommitted(); // Clear the reference holding all task allocations alive. globalKeepAlive = null; // Do a GC that should shrink the heap. - gc.invoke(); + long committedBefore = memoryBean.getHeapMemoryUsage().getCommitted(); + System.gc(); long committedAfter = memoryBean.getHeapMemoryUsage().getCommitted(); Asserts.assertLessThan(committedAfter, committedBefore); } @@ -168,42 +146,4 @@ private static long uptime() { private static void log(String text) { System.out.println(uptime() + "s: " + text); } - - private enum GCType { - Full { - public void invoke() { - log(" Full GC"); - System.gc(); - } - }, - Concurrent { - public void invoke() { - // First wait for any ergonomically started cycles. - if (wb.g1InConcurrentMark()) { - waitForConcurrentCycleToFinish(); - } - log(" Concurrent GC"); - wb.g1StartConcMarkCycle(); - waitForConcurrentCycleToFinish(); - // Trigger mixed GCs to compact old a bit. - wb.youngGC(); - wb.youngGC(); - wb.youngGC(); - // A second cycle to ensure we free up enough regions. - wb.g1StartConcMarkCycle(); - waitForConcurrentCycleToFinish(); - } - }; - public abstract void invoke(); - } - - private static void waitForConcurrentCycleToFinish() { - while (wb.g1InConcurrentMark()) { - try { - Thread.sleep(100); - } catch (InterruptedException e) { - e.printStackTrace(); - } - } - } }