8183226: Remembered set summarization accesses not fully initialized java thread DCQS
authortschatzl
Wed, 12 Jul 2017 12:25:12 +0200
changeset 46652 ab8716d193bb
parent 46650 642365ee2b92
child 46653 d72083d17b19
8183226: Remembered set summarization accesses not fully initialized java thread DCQS Reviewed-by: sjohanss, ehelin
hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp
hotspot/src/share/vm/gc/g1/g1CollectedHeap.hpp
hotspot/src/share/vm/gc/g1/g1RemSet.cpp
hotspot/src/share/vm/gc/g1/g1RemSetSummary.cpp
hotspot/src/share/vm/gc/g1/g1RemSetSummary.hpp
--- a/hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp	Wed Jul 12 09:49:05 2017 +0200
+++ b/hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp	Wed Jul 12 12:25:12 2017 +0200
@@ -1779,6 +1779,14 @@
   return result;
 }
 
+jint G1CollectedHeap::initialize_concurrent_refinement() {
+  _refine_cte_cl = new RefineCardTableEntryClosure();
+
+  jint ecode = JNI_OK;
+  _cg1r = ConcurrentG1Refine::create(_refine_cte_cl, &ecode);
+  return ecode;
+}
+
 jint G1CollectedHeap::initialize() {
   CollectedHeap::pre_initialize();
   os::enable_vtime();
@@ -1803,14 +1811,6 @@
   Universe::check_alignment(max_byte_size, HeapRegion::GrainBytes, "g1 heap");
   Universe::check_alignment(max_byte_size, heap_alignment, "g1 heap");
 
-  _refine_cte_cl = new RefineCardTableEntryClosure();
-
-  jint ecode = JNI_OK;
-  _cg1r = ConcurrentG1Refine::create(_refine_cte_cl, &ecode);
-  if (_cg1r == NULL) {
-    return ecode;
-  }
-
   // Reserve the maximum.
 
   // When compressed oops are enabled, the preferred heap base
@@ -1839,9 +1839,6 @@
   // Create the hot card cache.
   _hot_card_cache = new G1HotCardCache(this);
 
-  // Also create a G1 rem set.
-  _g1_rem_set = new G1RemSet(this, g1_barrier_set(), _hot_card_cache);
-
   // Carve out the G1 part of the heap.
   ReservedSpace g1_rs = heap_rs.first_part(max_byte_size);
   size_t page_size = UseLargePages ? os::large_page_size() : os::vm_page_size();
@@ -1893,7 +1890,9 @@
   const uint max_region_idx = (1U << (sizeof(RegionIdx_t)*BitsPerByte-1)) - 1;
   guarantee((max_regions() - 1) <= max_region_idx, "too many regions");
 
-  g1_rem_set()->initialize(max_capacity(), max_regions());
+  // Also create a G1 rem set.
+  _g1_rem_set = new G1RemSet(this, g1_barrier_set(), _hot_card_cache);
+  _g1_rem_set->initialize(max_capacity(), max_regions());
 
   size_t max_cards_per_region = ((size_t)1 << (sizeof(CardIdx_t)*BitsPerByte-1)) - 1;
   guarantee(HeapRegion::CardsPerRegion > 0, "make sure it's initialized");
@@ -1936,6 +1935,11 @@
                                                G1SATBProcessCompletedThreshold,
                                                Shared_SATB_Q_lock);
 
+  jint ecode = initialize_concurrent_refinement();
+  if (ecode != JNI_OK) {
+    return ecode;
+  }
+
   JavaThread::dirty_card_queue_set().initialize(_refine_cte_cl,
                                                 DirtyCardQ_CBL_mon,
                                                 DirtyCardQ_FL_lock,
--- a/hotspot/src/share/vm/gc/g1/g1CollectedHeap.hpp	Wed Jul 12 09:49:05 2017 +0200
+++ b/hotspot/src/share/vm/gc/g1/g1CollectedHeap.hpp	Wed Jul 12 12:25:12 2017 +0200
@@ -954,6 +954,9 @@
   // May not return if something goes wrong.
   G1CollectedHeap(G1CollectorPolicy* policy);
 
+private:
+  jint initialize_concurrent_refinement();
+public:
   // Initialize the G1CollectedHeap to have the initial and
   // maximum sizes and remembered and barrier sets
   // specified by the policy object.
--- a/hotspot/src/share/vm/gc/g1/g1RemSet.cpp	Wed Jul 12 09:49:05 2017 +0200
+++ b/hotspot/src/share/vm/gc/g1/g1RemSet.cpp	Wed Jul 12 12:25:12 2017 +0200
@@ -290,9 +290,6 @@
   _prev_period_summary(),
   _into_cset_dirty_card_queue_set(false)
 {
-  if (log_is_enabled(Trace, gc, remset)) {
-    _prev_period_summary.initialize(this);
-  }
   // Initialize the card queue set used to hold cards containing
   // references into the collection set.
   _into_cset_dirty_card_queue_set.initialize(NULL, // Should never be called by the Java code
@@ -790,12 +787,7 @@
   if ((G1SummarizeRSetStatsPeriod > 0) && log_is_enabled(Trace, gc, remset) &&
       (period_count % G1SummarizeRSetStatsPeriod == 0)) {
 
-    if (!_prev_period_summary.initialized()) {
-      _prev_period_summary.initialize(this);
-    }
-
-    G1RemSetSummary current;
-    current.initialize(this);
+    G1RemSetSummary current(this);
     _prev_period_summary.subtract_from(&current);
 
     Log(gc, remset) log;
@@ -812,7 +804,6 @@
   if (log.is_trace()) {
     log.trace(" Cumulative RS summary");
     G1RemSetSummary current;
-    current.initialize(this);
     ResourceMark rm;
     current.print_on(log.trace_stream());
   }
--- a/hotspot/src/share/vm/gc/g1/g1RemSetSummary.cpp	Wed Jul 12 09:49:05 2017 +0200
+++ b/hotspot/src/share/vm/gc/g1/g1RemSetSummary.cpp	Wed Jul 12 12:25:12 2017 +0200
@@ -52,7 +52,7 @@
 };
 
 void G1RemSetSummary::update() {
-  _num_conc_refined_cards = remset()->num_conc_refined_cards();
+  _num_conc_refined_cards = _rem_set->num_conc_refined_cards();
   DirtyCardQueueSet& dcqs = JavaThread::dirty_card_queue_set();
   _num_processed_buf_mutator = dcqs.processed_buffers_mut();
   _num_processed_buf_rs_threads = dcqs.processed_buffers_rs_thread();
@@ -79,27 +79,29 @@
   return _rs_threads_vtimes[thread];
 }
 
-void G1RemSetSummary::initialize(G1RemSet* remset) {
-  assert(_rs_threads_vtimes == NULL, "just checking");
-  assert(remset != NULL, "just checking");
-
-  _remset = remset;
-  _num_vtimes = ConcurrentG1Refine::thread_num();
-  _rs_threads_vtimes = NEW_C_HEAP_ARRAY(double, _num_vtimes, mtGC);
-  memset(_rs_threads_vtimes, 0, sizeof(double) * _num_vtimes);
-
-  update();
-}
-
 G1RemSetSummary::G1RemSetSummary() :
-  _remset(NULL),
+  _rem_set(NULL),
   _num_conc_refined_cards(0),
   _num_processed_buf_mutator(0),
   _num_processed_buf_rs_threads(0),
   _num_coarsenings(0),
-  _rs_threads_vtimes(NULL),
-  _num_vtimes(0),
+  _num_vtimes(ConcurrentG1Refine::thread_num()),
+  _rs_threads_vtimes(NEW_C_HEAP_ARRAY(double, _num_vtimes, mtGC)),
   _sampling_thread_vtime(0.0f) {
+
+  memset(_rs_threads_vtimes, 0, sizeof(double) * _num_vtimes);
+}
+
+G1RemSetSummary::G1RemSetSummary(G1RemSet* rem_set) :
+  _rem_set(rem_set),
+  _num_conc_refined_cards(0),
+  _num_processed_buf_mutator(0),
+  _num_processed_buf_rs_threads(0),
+  _num_coarsenings(0),
+  _num_vtimes(ConcurrentG1Refine::thread_num()),
+  _rs_threads_vtimes(NEW_C_HEAP_ARRAY(double, _num_vtimes, mtGC)),
+  _sampling_thread_vtime(0.0f) {
+  update();
 }
 
 G1RemSetSummary::~G1RemSetSummary() {
@@ -110,7 +112,6 @@
 
 void G1RemSetSummary::set(G1RemSetSummary* other) {
   assert(other != NULL, "just checking");
-  assert(remset() == other->remset(), "just checking");
   assert(_num_vtimes == other->_num_vtimes, "just checking");
 
   _num_conc_refined_cards = other->num_conc_refined_cards();
@@ -127,7 +128,6 @@
 
 void G1RemSetSummary::subtract_from(G1RemSetSummary* other) {
   assert(other != NULL, "just checking");
-  assert(remset() == other->remset(), "just checking");
   assert(_num_vtimes == other->_num_vtimes, "just checking");
 
   _num_conc_refined_cards = other->num_conc_refined_cards() - _num_conc_refined_cards;
--- a/hotspot/src/share/vm/gc/g1/g1RemSetSummary.hpp	Wed Jul 12 09:49:05 2017 +0200
+++ b/hotspot/src/share/vm/gc/g1/g1RemSetSummary.hpp	Wed Jul 12 12:25:12 2017 +0200
@@ -36,11 +36,7 @@
 private:
   friend class GetRSThreadVTimeClosure;
 
-  G1RemSet* _remset;
-
-  G1RemSet* remset() const {
-    return _remset;
-  }
+  G1RemSet* _rem_set;
 
   size_t _num_conc_refined_cards;
   size_t _num_processed_buf_mutator;
@@ -48,8 +44,8 @@
 
   size_t _num_coarsenings;
 
+  size_t _num_vtimes;
   double* _rs_threads_vtimes;
-  size_t _num_vtimes;
 
   double _sampling_thread_vtime;
 
@@ -63,6 +59,8 @@
 
 public:
   G1RemSetSummary();
+  G1RemSetSummary(G1RemSet* remset);
+
   ~G1RemSetSummary();
 
   // set the counters in this summary to the values of the others
@@ -70,10 +68,6 @@
   // subtract all counters from the other summary, and set them in the current
   void subtract_from(G1RemSetSummary* other);
 
-  // initialize and get the first sampling
-  void initialize(G1RemSet* remset);
-  bool const initialized() { return _rs_threads_vtimes != NULL; }
-
   void print_on(outputStream* out);
 
   double rs_thread_vtime(uint thread) const;