8185278: TestGreyReclaimedHumongousObjects.java fails guarantee(index != trim_index(_head_index + 1)) failed: should not go past head
authorsangheki
Thu, 26 Oct 2017 21:30:48 -0700
changeset 47756 55714c3d544c
parent 47700 c6d2381c6932
child 47757 1821be9ca11b
8185278: TestGreyReclaimedHumongousObjects.java fails guarantee(index != trim_index(_head_index + 1)) failed: should not go past head Summary: Add STS to avoid MMU concurrency problem between VM Thread and Concurrent Mark Thread Reviewed-by: tschatzl, ehelin
src/hotspot/share/gc/g1/concurrentMarkThread.cpp
src/hotspot/share/gc/g1/concurrentMarkThread.hpp
src/hotspot/share/gc/g1/g1MMUTracker.cpp
src/hotspot/share/runtime/mutexLocker.cpp
src/hotspot/share/runtime/mutexLocker.hpp
test/hotspot/jtreg/gc/g1/TestGreyReclaimedHumongousObjects.java
--- a/src/hotspot/share/gc/g1/concurrentMarkThread.cpp	Tue Oct 10 14:05:04 2017 +0200
+++ b/src/hotspot/share/gc/g1/concurrentMarkThread.cpp	Thu Oct 26 21:30:48 2017 -0700
@@ -111,16 +111,31 @@
   }
 };
 
-// Marking pauses can be scheduled flexibly, so we might delay marking to meet MMU.
+double ConcurrentMarkThread::mmu_sleep_time(G1Policy* g1_policy, bool remark) {
+  // There are 3 reasons to use SuspendibleThreadSetJoiner.
+  // 1. To avoid concurrency problem.
+  //    - G1MMUTracker::add_pause(), when_sec() and its variation(when_ms() etc..) can be called
+  //      concurrently from ConcurrentMarkThread and VMThread.
+  // 2. If currently a gc is running, but it has not yet updated the MMU,
+  //    we will not forget to consider that pause in the MMU calculation.
+  // 3. If currently a gc is running, ConcurrentMarkThread will wait it to be finished.
+  //    And then sleep for predicted amount of time by delay_to_keep_mmu().
+  SuspendibleThreadSetJoiner sts_join;
+
+  const G1Analytics* analytics = g1_policy->analytics();
+  double now = os::elapsedTime();
+  double prediction_ms = remark ? analytics->predict_remark_time_ms()
+                                : analytics->predict_cleanup_time_ms();
+  G1MMUTracker *mmu_tracker = g1_policy->mmu_tracker();
+  return mmu_tracker->when_ms(now, prediction_ms);
+}
+
 void ConcurrentMarkThread::delay_to_keep_mmu(G1Policy* g1_policy, bool remark) {
-  const G1Analytics* analytics = g1_policy->analytics();
   if (g1_policy->adaptive_young_list_length()) {
-    double now = os::elapsedTime();
-    double prediction_ms = remark ? analytics->predict_remark_time_ms()
-                                  : analytics->predict_cleanup_time_ms();
-    G1MMUTracker *mmu_tracker = g1_policy->mmu_tracker();
-    jlong sleep_time_ms = mmu_tracker->when_ms(now, prediction_ms);
-    os::sleep(this, sleep_time_ms, false);
+    jlong sleep_time_ms = mmu_sleep_time(g1_policy, remark);
+    if (!cm()->has_aborted() && sleep_time_ms > 0) {
+      os::sleep(this, sleep_time_ms, false);
+    }
   }
 }
 
@@ -349,9 +364,11 @@
       if (!cm()->has_aborted()) {
         delay_to_keep_mmu(g1_policy, false /* cleanup */);
 
-        CMCleanUp cl_cl(_cm);
-        VM_CGC_Operation op(&cl_cl, "Pause Cleanup");
-        VMThread::execute(&op);
+        if (!cm()->has_aborted()) {
+          CMCleanUp cl_cl(_cm);
+          VM_CGC_Operation op(&cl_cl, "Pause Cleanup");
+          VMThread::execute(&op);
+        }
       } else {
         // We don't want to update the marking status if a GC pause
         // is already underway.
--- a/src/hotspot/share/gc/g1/concurrentMarkThread.hpp	Tue Oct 10 14:05:04 2017 +0200
+++ b/src/hotspot/share/gc/g1/concurrentMarkThread.hpp	Thu Oct 26 21:30:48 2017 -0700
@@ -55,7 +55,9 @@
   ConcurrentGCPhaseManager::Stack _phase_manager_stack;
 
   void sleepBeforeNextCycle();
+  // Delay marking to meet MMU.
   void delay_to_keep_mmu(G1Policy* g1_policy, bool remark);
+  double mmu_sleep_time(G1Policy* g1_policy, bool remark);
 
   void run_service();
   void stop_service();
--- a/src/hotspot/share/gc/g1/g1MMUTracker.cpp	Tue Oct 10 14:05:04 2017 +0200
+++ b/src/hotspot/share/gc/g1/g1MMUTracker.cpp	Thu Oct 26 21:30:48 2017 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2017, 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
@@ -117,7 +117,6 @@
 // of other places (debugging)
 
 double G1MMUTrackerQueue::when_sec(double current_time, double pause_time) {
-  MutexLockerEx x(MMUTracker_lock, Mutex::_no_safepoint_check_flag);
   remove_expired_entries(current_time);
 
   return when_internal(current_time, pause_time);
--- a/src/hotspot/share/runtime/mutexLocker.cpp	Tue Oct 10 14:05:04 2017 +0200
+++ b/src/hotspot/share/runtime/mutexLocker.cpp	Thu Oct 26 21:30:48 2017 -0700
@@ -116,7 +116,6 @@
 Monitor* SecondaryFreeList_lock       = NULL;
 Mutex*   OldSets_lock                 = NULL;
 Monitor* RootRegionScan_lock          = NULL;
-Mutex*   MMUTracker_lock              = NULL;
 
 Monitor* GCTaskManager_lock           = NULL;
 
@@ -193,7 +192,6 @@
     def(SecondaryFreeList_lock     , PaddedMonitor, leaf     ,   true,  Monitor::_safepoint_check_never);
     def(OldSets_lock               , PaddedMutex  , leaf     ,   true,  Monitor::_safepoint_check_never);
     def(RootRegionScan_lock        , PaddedMonitor, leaf     ,   true,  Monitor::_safepoint_check_never);
-    def(MMUTracker_lock            , PaddedMutex  , leaf     ,   true,  Monitor::_safepoint_check_never);
 
     def(StringDedupQueue_lock      , PaddedMonitor, leaf,        true,  Monitor::_safepoint_check_never);
     def(StringDedupTable_lock      , PaddedMutex  , leaf,        true,  Monitor::_safepoint_check_never);
--- a/src/hotspot/share/runtime/mutexLocker.hpp	Tue Oct 10 14:05:04 2017 +0200
+++ b/src/hotspot/share/runtime/mutexLocker.hpp	Thu Oct 26 21:30:48 2017 -0700
@@ -117,8 +117,6 @@
 extern Monitor* SecondaryFreeList_lock;          // protects the secondary free region list
 extern Mutex*   OldSets_lock;                    // protects the old region sets
 extern Monitor* RootRegionScan_lock;             // used to notify that the CM threads have finished scanning the IM snapshot regions
-extern Mutex*   MMUTracker_lock;                 // protects the MMU
-                                                 // tracker data structures
 
 extern Mutex*   Management_lock;                 // a lock used to serialize JVM management
 extern Monitor* Service_lock;                    // a lock used for service thread operation
--- a/test/hotspot/jtreg/gc/g1/TestGreyReclaimedHumongousObjects.java	Tue Oct 10 14:05:04 2017 +0200
+++ b/test/hotspot/jtreg/gc/g1/TestGreyReclaimedHumongousObjects.java	Thu Oct 26 21:30:48 2017 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test TestGreyReclaimedHumongousObjects.java
- * @bug 8069367
+ * @bug 8069367 8185278
  * @requires vm.gc.G1
  * @summary Test handling of marked but unscanned reclaimed humongous objects.
  * @key gc