# HG changeset patch
# User kbarrett
# Date 1544569217 18000
# Node ID f0f3dc30e3bb4234c7e20c4cd1998b804a4d7651
# Parent  799e964e32b62999afb4859f85d7e5a3951de629
8215097: Do not create NonJavaThreads before BarrierSet
Summary: G1 and CMS delay worker thread creation until BarrierSet exists.
Reviewed-by: dholmes, tschatzl

diff -r 799e964e32b6 -r f0f3dc30e3bb src/hotspot/share/gc/cms/cmsHeap.cpp
--- a/src/hotspot/share/gc/cms/cmsHeap.cpp	Tue Dec 11 13:13:18 2018 -0800
+++ b/src/hotspot/share/gc/cms/cmsHeap.cpp	Tue Dec 11 18:00:17 2018 -0500
@@ -70,19 +70,24 @@
                      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()) {
diff -r 799e964e32b6 -r f0f3dc30e3bb src/hotspot/share/gc/g1/g1CollectedHeap.cpp
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp	Tue Dec 11 13:13:18 2018 -0800
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp	Tue Dec 11 18:00:17 2018 -0500
@@ -1531,10 +1531,6 @@
   _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 @@
     _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);
diff -r 799e964e32b6 -r f0f3dc30e3bb src/hotspot/share/gc/shared/barrierSet.cpp
--- a/src/hotspot/share/gc/shared/barrierSet.cpp	Tue Dec 11 13:13:18 2018 -0800
+++ b/src/hotspot/share/gc/shared/barrierSet.cpp	Tue Dec 11 18:00:17 2018 -0500
@@ -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
-  // BarrierSet::on_thread_create had to be deferred for them.  Now that
-  // we have the barrier set, do those deferred calls.
-
-  // 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
+  // Notify barrier set of the current (main) thread.  Normally the
+  // Thread constructor deals with this, but the main thread is
+  // created before we get here.  Verify 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());
diff -r 799e964e32b6 -r f0f3dc30e3bb src/hotspot/share/runtime/thread.cpp
--- a/src/hotspot/share/runtime/thread.cpp	Tue Dec 11 13:13:18 2018 -0800
+++ b/src/hotspot/share/runtime/thread.cpp	Tue Dec 11 18:00:17 2018 -0500
@@ -306,15 +306,19 @@
   }
 #endif // ASSERT
 
-  // Notify the barrier set that a thread is being created. Note that some
-  // threads are created before a barrier set is available. The call to
-  // BarrierSet::on_thread_create() for these threads is therefore deferred
+  // Notify the barrier set that a thread is being created. The initial
+  // thread is created before the barrier set is available.  The call to
+  // 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 @@
 
 #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) {
diff -r 799e964e32b6 -r f0f3dc30e3bb src/hotspot/share/runtime/thread.hpp
--- a/src/hotspot/share/runtime/thread.hpp	Tue Dec 11 13:13:18 2018 -0800
+++ b/src/hotspot/share/runtime/thread.hpp	Tue Dec 11 18:00:17 2018 -0500
@@ -2156,7 +2156,6 @@
   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 @@
 
 #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