8215097: Do not create NonJavaThreads before BarrierSet

G1 and CMS delay worker thread creation until BarrierSet exists.

Reviewed-by: dholmes, tschatzl

src/hotspot/share/gc/cms/cmsHeap.cpp

@@ -70,19 +70,24 @@ CMSHeap::CMSHeap(GenCollectorPolicy *policy) :
                      Generation::ParNew,
                      Generation::ConcurrentMarkSweep,
                      "ParNew:CMS"),
+    _workers(NULL),
     _eden_pool(NULL),
     _survivor_pool(NULL),
     _old_pool(NULL) {
-  _workers = new WorkGang("GC Thread", ParallelGCThreads,
-                          /* are_GC_task_threads */true,
-                          /* are_ConcurrentGC_threads */false);
-  _workers->initialize_workers();
 }
 
 jint CMSHeap::initialize() {
   jint status = GenCollectedHeap::initialize();
   if (status != JNI_OK) return status;
 
+  _workers = new WorkGang("GC Thread", ParallelGCThreads,
+                          /* are_GC_task_threads */true,
+                          /* are_ConcurrentGC_threads */false);
+  if (_workers == NULL) {
+    return JNI_ENOMEM;
+  }
+  _workers->initialize_workers();
+
   // If we are running CMS, create the collector responsible
   // for collecting the CMS generations.
   if (!create_cms_collector()) {

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

@@ -1531,10 +1531,6 @@ G1CollectedHeap::G1CollectedHeap(G1CollectorPolicy* collector_policy) :
   _is_subject_to_discovery_cm(this),
   _in_cset_fast_test() {
 
-  _workers = new WorkGang("GC Thread", ParallelGCThreads,
-                          true /* are_GC_task_threads */,
-                          false /* are_ConcurrentGC_threads */);
-  _workers->initialize_workers();
   _verifier = new G1HeapVerifier(this);
 
   _allocator = new G1Allocator(this);
@@ -1767,6 +1763,14 @@ jint G1CollectedHeap::initialize() {
     _humongous_reclaim_candidates.initialize(start, end, granularity);
   }
 
+  _workers = new WorkGang("GC Thread", ParallelGCThreads,
+                          true /* are_GC_task_threads */,
+                          false /* are_ConcurrentGC_threads */);
+  if (_workers == NULL) {
+    return JNI_ENOMEM;
+  }
+  _workers->initialize_workers();
+
   // Create the G1ConcurrentMark data structure and thread.
   // (Must do this late, so that "max_regions" is defined.)
   _cm = new G1ConcurrentMark(this, prev_bitmap_storage, next_bitmap_storage);

src/hotspot/share/gc/shared/barrierSet.cpp

@@ -26,47 +26,24 @@
 #include "gc/shared/barrierSet.hpp"
 #include "gc/shared/barrierSetAssembler.hpp"
 #include "runtime/thread.hpp"
+#include "utilities/debug.hpp"
 #include "utilities/macros.hpp"
 
 BarrierSet* BarrierSet::_barrier_set = NULL;
 
-class SetBarrierSetNonJavaThread : public ThreadClosure {
-  BarrierSet* _barrier_set;
-  size_t _count;
-
-public:
-  SetBarrierSetNonJavaThread(BarrierSet* barrier_set) :
-    _barrier_set(barrier_set), _count(0) {}
-
-  virtual void do_thread(Thread* thread) {
-    _barrier_set->on_thread_create(thread);
-    ++_count;
-  }
-
-  size_t count() const { return _count; }
-};
-
 void BarrierSet::set_barrier_set(BarrierSet* barrier_set) {
   assert(_barrier_set == NULL, "Already initialized");
   _barrier_set = barrier_set;
 
-  // Some threads are created before the barrier set, so the call to
+  // Notify barrier set of the current (main) thread.  Normally the
-  // BarrierSet::on_thread_create had to be deferred for them.  Now that
+  // Thread constructor deals with this, but the main thread is
-  // we have the barrier set, do those deferred calls.
+  // created before we get here.  Verify it isn't yet on the thread
-
-  // First do any non-JavaThreads.
-  SetBarrierSetNonJavaThread njt_closure(_barrier_set);
-  Threads::non_java_threads_do(&njt_closure);
-
-  // Do the current (main) thread.  Ensure it's the one and only
-  // JavaThread so far.  Also verify that it isn't yet on the thread
   // list, else we'd also need to call BarrierSet::on_thread_attach.
+  // This is the only thread that can exist at this point; the Thread
+  // constructor objects to other threads being created before the
+  // barrier set is available.
   assert(Thread::current()->is_Java_thread(),
          "Expected main thread to be a JavaThread");
-  assert((njt_closure.count() + 1) == Threads::threads_before_barrier_set(),
-         "Unexpected JavaThreads before barrier set initialization: "
-         "Non-JavaThreads: " SIZE_FORMAT ", all: " SIZE_FORMAT,
-         njt_closure.count(), Threads::threads_before_barrier_set());
   assert(!JavaThread::current()->on_thread_list(),
          "Main thread already on thread list.");
   _barrier_set->on_thread_create(Thread::current());

src/hotspot/share/runtime/thread.cpp

@@ -306,15 +306,19 @@ Thread::Thread() {
   }
 #endif // ASSERT
 
-  // Notify the barrier set that a thread is being created. Note that some
+  // Notify the barrier set that a thread is being created. The initial
-  // threads are created before a barrier set is available. The call to
+  // thread is created before the barrier set is available.  The call to
-  // BarrierSet::on_thread_create() for these threads is therefore deferred
+  // BarrierSet::on_thread_create() for this thread is therefore deferred
   // to BarrierSet::set_barrier_set().
   BarrierSet* const barrier_set = BarrierSet::barrier_set();
   if (barrier_set != NULL) {
     barrier_set->on_thread_create(this);
   } else {
-    DEBUG_ONLY(Threads::inc_threads_before_barrier_set();)
+#ifdef ASSERT
+    static bool initial_thread_created = false;
+    assert(!initial_thread_created, "creating thread before barrier set");
+    initial_thread_created = true;
+#endif // ASSERT
   }
 }
 
@@ -3395,7 +3399,6 @@ size_t      JavaThread::_stack_size_at_create = 0;
 
 #ifdef ASSERT
 bool        Threads::_vm_complete = false;
-size_t      Threads::_threads_before_barrier_set = 0;
 #endif
 
 static inline void *prefetch_and_load_ptr(void **addr, intx prefetch_interval) {

src/hotspot/share/runtime/thread.hpp

@@ -2156,7 +2156,6 @@ class Threads: AllStatic {
   static int         _thread_claim_parity;
 #ifdef ASSERT
   static bool        _vm_complete;
-  static size_t      _threads_before_barrier_set;
 #endif
 
   static void initialize_java_lang_classes(JavaThread* main_thread, TRAPS);
@@ -2226,14 +2225,6 @@ class Threads: AllStatic {
 
 #ifdef ASSERT
   static bool is_vm_complete() { return _vm_complete; }
-
-  static size_t threads_before_barrier_set() {
-    return _threads_before_barrier_set;
-  }
-
-  static void inc_threads_before_barrier_set() {
-    ++_threads_before_barrier_set;
-  }
 #endif // ASSERT
 
   // Verification