8079579: Add SuspendibleThreadSetLeaver and make SuspendibleThreadSet::joint()/leave() private

Reviewed-by: brutisso, david

hotspot/src/share/vm/gc_implementation/g1/concurrentG1RefineThread.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2015, 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
@@ -77,7 +77,7 @@ void ConcurrentG1RefineThread::initialize() {
 }
 
 void ConcurrentG1RefineThread::sample_young_list_rs_lengths() {
-  SuspendibleThreadSetJoiner sts;
+  SuspendibleThreadSetJoiner sts_join;
   G1CollectedHeap* g1h = G1CollectedHeap::heap();
   G1CollectorPolicy* g1p = g1h->g1_policy();
   if (g1p->adaptive_young_list_length()) {
@@ -89,8 +89,8 @@ void ConcurrentG1RefineThread::sample_young_list_rs_lengths() {
 
       // we try to yield every time we visit 10 regions
       if (regions_visited == 10) {
-        if (sts.should_yield()) {
-          sts.yield();
+        if (sts_join.should_yield()) {
+          sts_join.yield();
           // we just abandon the iteration
           break;
         }
@@ -188,7 +188,7 @@ void ConcurrentG1RefineThread::run() {
     }
 
     {
-      SuspendibleThreadSetJoiner sts;
+      SuspendibleThreadSetJoiner sts_join;
 
       do {
         int curr_buffer_num = (int)dcqs.completed_buffers_num();

hotspot/src/share/vm/gc_implementation/g1/concurrentMark.cpp

@@ -193,13 +193,8 @@ public:
       _cl(cl), _suspendible(suspendible), AbstractGangTask("Parallel Clear Bitmap Task"), _hrclaimer(n_workers) {}
 
   void work(uint worker_id) {
-    if (_suspendible) {
-      SuspendibleThreadSet::join();
-    }
+    SuspendibleThreadSetJoiner sts_join(_suspendible);
     G1CollectedHeap::heap()->heap_region_par_iterate(_cl, worker_id, &_hrclaimer, true);
-    if (_suspendible) {
-      SuspendibleThreadSet::leave();
-    }
   }
 };
 
@@ -956,19 +951,17 @@ void ConcurrentMark::checkpointRootsInitialPost() {
  */
 
 void ConcurrentMark::enter_first_sync_barrier(uint worker_id) {
+  bool barrier_aborted;
+
   if (verbose_low()) {
     gclog_or_tty->print_cr("[%u] entering first barrier", worker_id);
   }
 
-  if (concurrent()) {
-    SuspendibleThreadSet::leave();
+  {
+    SuspendibleThreadSetLeaver sts_leave(concurrent());
+    barrier_aborted = !_first_overflow_barrier_sync.enter();
   }
 
-  bool barrier_aborted = !_first_overflow_barrier_sync.enter();
-
-  if (concurrent()) {
-    SuspendibleThreadSet::join();
-  }
   // at this point everyone should have synced up and not be doing any
   // more work
 
@@ -1015,19 +1008,17 @@ void ConcurrentMark::enter_first_sync_barrier(uint worker_id) {
 }
 
 void ConcurrentMark::enter_second_sync_barrier(uint worker_id) {
+  bool barrier_aborted;
+
   if (verbose_low()) {
     gclog_or_tty->print_cr("[%u] entering second barrier", worker_id);
   }
 
-  if (concurrent()) {
-    SuspendibleThreadSet::leave();
+  {
+    SuspendibleThreadSetLeaver sts_leave(concurrent());
+    barrier_aborted = !_second_overflow_barrier_sync.enter();
   }
 
-  bool barrier_aborted = !_second_overflow_barrier_sync.enter();
-
-  if (concurrent()) {
-    SuspendibleThreadSet::join();
-  }
   // at this point everything should be re-initialized and ready to go
 
   if (verbose_low()) {
@@ -1078,40 +1069,41 @@ public:
 
     double start_vtime = os::elapsedVTime();
 
-    SuspendibleThreadSet::join();
+    {
+      SuspendibleThreadSetJoiner sts_join;
 
-    assert(worker_id < _cm->active_tasks(), "invariant");
-    CMTask* the_task = _cm->task(worker_id);
-    the_task->record_start_time();
-    if (!_cm->has_aborted()) {
-      do {
-        double start_vtime_sec = os::elapsedVTime();
-        double mark_step_duration_ms = G1ConcMarkStepDurationMillis;
+      assert(worker_id < _cm->active_tasks(), "invariant");
+      CMTask* the_task = _cm->task(worker_id);
+      the_task->record_start_time();
+      if (!_cm->has_aborted()) {
+        do {
+          double start_vtime_sec = os::elapsedVTime();
+          double mark_step_duration_ms = G1ConcMarkStepDurationMillis;
 
-        the_task->do_marking_step(mark_step_duration_ms,
-                                  true  /* do_termination */,
-                                  false /* is_serial*/);
+          the_task->do_marking_step(mark_step_duration_ms,
+                                    true  /* do_termination */,
+                                    false /* is_serial*/);
 
-        double end_vtime_sec = os::elapsedVTime();
-        double elapsed_vtime_sec = end_vtime_sec - start_vtime_sec;
-        _cm->clear_has_overflown();
+          double end_vtime_sec = os::elapsedVTime();
+          double elapsed_vtime_sec = end_vtime_sec - start_vtime_sec;
+          _cm->clear_has_overflown();
 
-        _cm->do_yield_check(worker_id);
+          _cm->do_yield_check(worker_id);
 
-        jlong sleep_time_ms;
-        if (!_cm->has_aborted() && the_task->has_aborted()) {
-          sleep_time_ms =
-            (jlong) (elapsed_vtime_sec * _cm->sleep_factor() * 1000.0);
-          SuspendibleThreadSet::leave();
-          os::sleep(Thread::current(), sleep_time_ms, false);
-          SuspendibleThreadSet::join();
-        }
-      } while (!_cm->has_aborted() && the_task->has_aborted());
+          jlong sleep_time_ms;
+          if (!_cm->has_aborted() && the_task->has_aborted()) {
+            sleep_time_ms =
+              (jlong) (elapsed_vtime_sec * _cm->sleep_factor() * 1000.0);
+            {
+              SuspendibleThreadSetLeaver sts_leave;
+              os::sleep(Thread::current(), sleep_time_ms, false);
+            }
+          }
+        } while (!_cm->has_aborted() && the_task->has_aborted());
+      }
+      the_task->record_end_time();
+      guarantee(!the_task->has_aborted() || _cm->has_aborted(), "invariant");
     }
-    the_task->record_end_time();
-    guarantee(!the_task->has_aborted() || _cm->has_aborted(), "invariant");
-
-    SuspendibleThreadSet::leave();
 
     double end_vtime = os::elapsedVTime();
     _cm->update_accum_task_vtime(worker_id, end_vtime - start_vtime);

hotspot/src/share/vm/gc_implementation/g1/concurrentMarkThread.cpp

@@ -192,7 +192,7 @@ void ConcurrentMarkThread::run() {
       } else {
         // We don't want to update the marking status if a GC pause
         // is already underway.
-        SuspendibleThreadSetJoiner sts;
+        SuspendibleThreadSetJoiner sts_join;
         g1h->set_marking_complete();
       }
 
@@ -262,7 +262,7 @@ void ConcurrentMarkThread::run() {
       // not needed any more as the concurrent mark state has been
       // already reset).
       {
-        SuspendibleThreadSetJoiner sts;
+        SuspendibleThreadSetJoiner sts_join;
         if (!cm()->has_aborted()) {
           g1_policy->record_concurrent_mark_cleanup_completed();
         }
@@ -291,7 +291,7 @@ void ConcurrentMarkThread::run() {
     // Java thread is waiting for a full GC to happen (e.g., it
     // called System.gc() with +ExplicitGCInvokesConcurrent).
     {
-      SuspendibleThreadSetJoiner sts;
+      SuspendibleThreadSetJoiner sts_join;
       g1h->increment_old_marking_cycles_completed(true /* concurrent */);
       g1h->register_concurrent_cycle_end();
     }

hotspot/src/share/vm/gc_implementation/g1/g1StringDedupThread.cpp

@@ -74,7 +74,7 @@ void G1StringDedupThread::run() {
 
     {
       // Include thread in safepoints
-      SuspendibleThreadSetJoiner sts;
+      SuspendibleThreadSetJoiner sts_join;
 
       stat.mark_exec();
 
@@ -88,9 +88,9 @@ void G1StringDedupThread::run() {
         G1StringDedupTable::deduplicate(java_string, stat);
 
         // Safepoint this thread if needed
-        if (sts.should_yield()) {
+        if (sts_join.should_yield()) {
           stat.mark_block();
-          sts.yield();
+          sts_join.yield();
           stat.mark_unblock();
         }
       }

hotspot/src/share/vm/gc_implementation/shared/suspendibleThreadSet.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2015, 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
@@ -36,19 +36,22 @@
 // suspending thread later calls desynchronize(), allowing the suspended
 // threads to continue.
 class SuspendibleThreadSet : public AllStatic {
+  friend class SuspendibleThreadSetJoiner;
+  friend class SuspendibleThreadSetLeaver;
+
 private:
   static uint   _nthreads;
   static uint   _nthreads_stopped;
   static bool   _suspend_all;
   static double _suspend_all_start;
 
-public:
   // Add the current thread to the set. May block if a suspension is in progress.
   static void join();
 
   // Removes the current thread from the set.
   static void leave();
 
+public:
   // Returns true if an suspension is in progress.
   static bool should_yield() { return _suspend_all; }
 
@@ -63,22 +66,52 @@ public:
 };
 
 class SuspendibleThreadSetJoiner : public StackObj {
+private:
+  bool _active;
+
 public:
-  SuspendibleThreadSetJoiner() {
-    SuspendibleThreadSet::join();
+  SuspendibleThreadSetJoiner(bool active = true) : _active(active) {
+    if (_active) {
+      SuspendibleThreadSet::join();
+    }
   }
 
   ~SuspendibleThreadSetJoiner() {
-    SuspendibleThreadSet::leave();
+    if (_active) {
+      SuspendibleThreadSet::leave();
+    }
   }
 
   bool should_yield() {
-    return SuspendibleThreadSet::should_yield();
+    if (_active) {
+      return SuspendibleThreadSet::should_yield();
+    } else {
+      return false;
+    }
   }
 
   void yield() {
+    assert(_active, "Thread has not joined the suspendible thread set");
     SuspendibleThreadSet::yield();
   }
 };
 
+class SuspendibleThreadSetLeaver : public StackObj {
+private:
+  bool _active;
+
+public:
+  SuspendibleThreadSetLeaver(bool active = true) : _active(active) {
+    if (_active) {
+      SuspendibleThreadSet::leave();
+    }
+  }
+
+  ~SuspendibleThreadSetLeaver() {
+    if (_active) {
+      SuspendibleThreadSet::join();
+    }
+  }
+};
+
 #endif // SHARE_VM_GC_IMPLEMENTATION_SHARED_SUSPENDIBLETHREADSET_HPP