8197994: Move JavaThread::initialize_queues() logic to G1SATBCardTableLoggingModRefBS

Reviewed-by: tschatzl, eosterlund

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

@@ -220,8 +220,39 @@ bool G1SATBCardTableModRefBS::is_in_young(oop obj) const {
   return *p == g1_young_card_val();
 }
 
-void G1SATBCardTableLoggingModRefBS::flush_deferred_barriers(JavaThread* thread) {
+void G1SATBCardTableLoggingModRefBS::on_thread_attach(JavaThread* thread) {
-  CardTableModRefBS::flush_deferred_barriers(thread);
+  // This method initializes the SATB and dirty card queues before a
+  // JavaThread is added to the Java thread list. Right now, we don't
+  // have to do anything to the dirty card queue (it should have been
+  // activated when the thread was created), but we have to activate
+  // the SATB queue if the thread is created while a marking cycle is
+  // in progress. The activation / de-activation of the SATB queues at
+  // the beginning / end of a marking cycle is done during safepoints
+  // so we have to make sure this method is called outside one to be
+  // able to safely read the active field of the SATB queue set. Right
+  // now, it is called just before the thread is added to the Java
+  // thread list in the Threads::add() method. That method is holding
+  // the Threads_lock which ensures we are outside a safepoint. We
+  // cannot do the obvious and set the active field of the SATB queue
+  // when the thread is created given that, in some cases, safepoints
+  // might happen between the JavaThread constructor being called and the
+  // thread being added to the Java thread list (an example of this is
+  // when the structure for the DestroyJavaVM thread is created).
+  assert(!SafepointSynchronize::is_at_safepoint(), "We should not be at a safepoint");
+  assert(!thread->satb_mark_queue().is_active(), "SATB queue should not be active");
+  assert(thread->satb_mark_queue().is_empty(), "SATB queue should be empty");
+  assert(thread->dirty_card_queue().is_active(), "Dirty card queue should be active");
+
+  // If we are creating the thread during a marking cycle, we should
+  // set the active field of the SATB queue to true.
+  if (thread->satb_mark_queue_set().is_active()) {
+    thread->satb_mark_queue().set_active(true);
+  }
+}
+
+void G1SATBCardTableLoggingModRefBS::on_thread_detach(JavaThread* thread) {
+  // Flush any deferred card marks, SATB buffers and dirty card queue buffers
+  CardTableModRefBS::on_thread_detach(thread);
   thread->satb_mark_queue().flush();
   thread->dirty_card_queue().flush();
 }

src/hotspot/share/gc/g1/g1SATBCardTableModRefBS.hpp

@@ -154,7 +154,8 @@ class G1SATBCardTableLoggingModRefBS: public G1SATBCardTableModRefBS {
   void write_ref_field_post(T* field, oop new_val);
   void write_ref_field_post_slow(volatile jbyte* byte);
 
-  virtual void flush_deferred_barriers(JavaThread* thread);
+  virtual void on_thread_attach(JavaThread* thread);
+  virtual void on_thread_detach(JavaThread* thread);
 
   virtual bool card_mark_must_follow_store() const {
     return true;

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2018, 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
@@ -40,7 +40,8 @@ SATBMarkQueue::SATBMarkQueue(SATBMarkQueueSet* qset, bool permanent) :
   // them with their active field set to false. If a thread is
   // created during a cycle and its SATB queue needs to be activated
   // before the thread starts running, we'll need to set its active
-  // field to true. This is done in JavaThread::initialize_queues().
+  // field to true. This is done in G1SATBCardTableLoggingModRefBS::
+  // on_thread_attach().
   PtrQueue(qset, permanent, false /* active */)
 { }
 

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

@@ -115,7 +115,8 @@ public:
   // is redone until it succeeds. This can e.g. prevent allocations from the slow path
   // to be in old.
   virtual void on_slowpath_allocation_exit(JavaThread* thread, oop new_obj) {}
-  virtual void flush_deferred_barriers(JavaThread* thread) {}
+  virtual void on_thread_attach(JavaThread* thread) {}
+  virtual void on_thread_detach(JavaThread* thread) {}
   virtual void make_parsable(JavaThread* thread) {}
 
 protected:

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

@@ -627,7 +627,7 @@ void CardTableModRefBS::flush_deferred_card_mark_barrier(JavaThread* thread) {
 #endif
 }
 
-void CardTableModRefBS::flush_deferred_barriers(JavaThread* thread) {
+void CardTableModRefBS::on_thread_detach(JavaThread* thread) {
   // The deferred store barriers must all have been flushed to the
   // card-table (or other remembered set structure) before GC starts
   // processing the card-table (or other remembered set).

src/hotspot/share/gc/shared/cardTableModRefBS.hpp

@@ -357,7 +357,7 @@ class CardTableModRefBS: public ModRefBarrierSet {
   virtual bool is_in_young(oop obj) const = 0;
 
   virtual void on_slowpath_allocation_exit(JavaThread* thread, oop new_obj);
-  virtual void flush_deferred_barriers(JavaThread* thread);
+  virtual void on_thread_detach(JavaThread* thread);
 
   virtual void make_parsable(JavaThread* thread) { flush_deferred_card_mark_barrier(thread); }
 

src/hotspot/share/runtime/thread.cpp

@@ -1994,10 +1994,7 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
     JvmtiExport::cleanup_thread(this);
   }
 
-  // We must flush any deferred card marks and other various GC barrier
+  BarrierSet::barrier_set()->on_thread_detach(this);
-  // related buffers (e.g. G1 SATB buffer and G1 dirty card queue buffer)
-  // before removing a thread from the list of active threads.
-  BarrierSet::barrier_set()->flush_deferred_barriers(this);
 
   log_info(os, thread)("JavaThread %s (tid: " UINTX_FORMAT ").",
     exit_type == JavaThread::normal_exit ? "exiting" : "detaching",
@@ -2026,30 +2023,6 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) {
   }
 }
 
-#if INCLUDE_ALL_GCS
-void JavaThread::initialize_queues() {
-  assert(!SafepointSynchronize::is_at_safepoint(),
-         "we should not be at a safepoint");
-
-  SATBMarkQueue& satb_queue = satb_mark_queue();
-  SATBMarkQueueSet& satb_queue_set = satb_mark_queue_set();
-  // The SATB queue should have been constructed with its active
-  // field set to false.
-  assert(!satb_queue.is_active(), "SATB queue should not be active");
-  assert(satb_queue.is_empty(), "SATB queue should be empty");
-  // If we are creating the thread during a marking cycle, we should
-  // set the active field of the SATB queue to true.
-  if (satb_queue_set.is_active()) {
-    satb_queue.set_active(true);
-  }
-
-  DirtyCardQueue& dirty_queue = dirty_card_queue();
-  // The dirty card queue should have been constructed with its
-  // active field set to true.
-  assert(dirty_queue.is_active(), "dirty card queue should be active");
-}
-#endif // INCLUDE_ALL_GCS
-
 void JavaThread::cleanup_failed_attach_current_thread() {
   if (active_handles() != NULL) {
     JNIHandleBlock* block = active_handles();
@@ -2070,15 +2043,12 @@ void JavaThread::cleanup_failed_attach_current_thread() {
     tlab().make_parsable(true);  // retire TLAB, if any
   }
 
-  BarrierSet::barrier_set()->flush_deferred_barriers(this);
+  BarrierSet::barrier_set()->on_thread_detach(this);
 
   Threads::remove(this);
   this->smr_delete();
 }
 
-
-
-
 JavaThread* JavaThread::active() {
   Thread* thread = Thread::current();
   if (thread->is_Java_thread()) {
@@ -4332,9 +4302,8 @@ void Threads::add(JavaThread* p, bool force_daemon) {
   // The threads lock must be owned at this point
   assert_locked_or_safepoint(Threads_lock);
 
-  // See the comment for this method in thread.hpp for its purpose and
+  BarrierSet::barrier_set()->on_thread_attach(p);
-  // why it is called here.
+
-  p->initialize_queues();
   p->set_next(_thread_list);
   _thread_list = p;
 

src/hotspot/share/runtime/thread.hpp

@@ -1966,29 +1966,6 @@ class JavaThread: public Thread {
   }
 #endif // INCLUDE_ALL_GCS
 
-  // This method initializes the SATB and dirty card queues before a
-  // JavaThread is added to the Java thread list. Right now, we don't
-  // have to do anything to the dirty card queue (it should have been
-  // activated when the thread was created), but we have to activate
-  // the SATB queue if the thread is created while a marking cycle is
-  // in progress. The activation / de-activation of the SATB queues at
-  // the beginning / end of a marking cycle is done during safepoints
-  // so we have to make sure this method is called outside one to be
-  // able to safely read the active field of the SATB queue set. Right
-  // now, it is called just before the thread is added to the Java
-  // thread list in the Threads::add() method. That method is holding
-  // the Threads_lock which ensures we are outside a safepoint. We
-  // cannot do the obvious and set the active field of the SATB queue
-  // when the thread is created given that, in some cases, safepoints
-  // might happen between the JavaThread constructor being called and the
-  // thread being added to the Java thread list (an example of this is
-  // when the structure for the DestroyJavaVM thread is created).
-#if INCLUDE_ALL_GCS
-  void initialize_queues();
-#else  // INCLUDE_ALL_GCS
-  void initialize_queues() { }
-#endif // INCLUDE_ALL_GCS
-
   // Machine dependent stuff
 #include OS_CPU_HEADER(thread)