8221360: Eliminate Shared_DirtyCardQ_lock

Reviewed-by: tschatzl, iwalulya

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

@@ -58,8 +58,7 @@ G1BarrierSet::G1BarrierSet(G1CardTable* card_table) :
   _satb_mark_queue_buffer_allocator("SATB Buffer Allocator", G1SATBBufferSize),
   _dirty_card_queue_buffer_allocator("DC Buffer Allocator", G1UpdateBufferSize),
   _satb_mark_queue_set(&_satb_mark_queue_buffer_allocator),
-  _dirty_card_queue_set(&_dirty_card_queue_buffer_allocator),
-  _shared_dirty_card_queue(&_dirty_card_queue_set)
+  _dirty_card_queue_set(&_dirty_card_queue_buffer_allocator)
 {}
 
 void G1BarrierSet::enqueue(oop pre_val) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2021, 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
@@ -27,7 +27,6 @@
 
 #include "gc/g1/g1DirtyCardQueue.hpp"
 #include "gc/g1/g1SATBMarkQueueSet.hpp"
-#include "gc/g1/g1SharedDirtyCardQueue.hpp"
 #include "gc/shared/cardTable.hpp"
 #include "gc/shared/cardTableBarrierSet.hpp"
 
@@ -43,7 +42,6 @@ class G1BarrierSet: public CardTableBarrierSet {
   BufferNode::Allocator _dirty_card_queue_buffer_allocator;
   G1SATBMarkQueueSet _satb_mark_queue_set;
   G1DirtyCardQueueSet _dirty_card_queue_set;
-  G1SharedDirtyCardQueue _shared_dirty_card_queue;
 
   static G1BarrierSet* g1_barrier_set() {
     return barrier_set_cast<G1BarrierSet>(BarrierSet::barrier_set());
@@ -94,10 +92,6 @@ class G1BarrierSet: public CardTableBarrierSet {
     return g1_barrier_set()->_dirty_card_queue_set;
   }
 
-  static G1SharedDirtyCardQueue& shared_dirty_card_queue() {
-    return g1_barrier_set()->_shared_dirty_card_queue;
-  }
-
   // Callbacks for runtime accesses.
   template <DecoratorSet decorators, typename BarrierSetT = G1BarrierSet>
   class AccessBarrier: public ModRefBarrierSet::AccessBarrier<decorators, BarrierSetT> {

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

@@ -546,8 +546,6 @@ void G1DirtyCardQueueSet::abandon_logs() {
     }
   } closure(*this);
   Threads::threads_do(&closure);
-
-  G1BarrierSet::shared_dirty_card_queue().reset();
 }
 
 void G1DirtyCardQueueSet::concatenate_logs() {
@@ -571,7 +569,6 @@ void G1DirtyCardQueueSet::concatenate_logs() {
   } closure(*this);
   Threads::threads_do(&closure);
 
-  G1BarrierSet::shared_dirty_card_queue().flush();
   enqueue_all_paused_buffers();
   verify_num_cards();
   set_max_cards(old_limit);

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

@@ -41,7 +41,6 @@
 #include "gc/g1/g1RootClosures.hpp"
 #include "gc/g1/g1RemSet.hpp"
 #include "gc/g1/g1ServiceThread.hpp"
-#include "gc/g1/g1SharedDirtyCardQueue.hpp"
 #include "gc/g1/g1_globals.hpp"
 #include "gc/g1/heapRegion.inline.hpp"
 #include "gc/g1/heapRegionManager.inline.hpp"
@@ -1688,12 +1687,27 @@ void G1RemSet::refine_card_concurrently(CardValue* const card_ptr,
     return;
   }
 
-  // Re-dirty the card and enqueue in the *shared* queue.  Can't use
-  // the thread-local queue, because that might be the queue that is
-  // being processed by us; we could be a Java thread conscripted to
-  // perform refinement on our queue's current buffer.
+  enqueue_for_reprocessing(card_ptr);
+}
+
+// Re-dirty and re-enqueue the card to retry refinement later.
+// This is used to deal with a rare race condition in concurrent refinement.
+void G1RemSet::enqueue_for_reprocessing(CardValue* card_ptr) {
+  // We can't use the thread-local queue, because that might be the queue
+  // that is being processed by us; we could be a Java thread conscripted to
+  // perform refinement on our queue's current buffer.  This situation only
+  // arises from rare race condition, so it's not worth any significant
+  // development effort or clever lock-free queue implementation.  Instead
+  // we use brute force, allocating and enqueuing an entire buffer for just
+  // this card.  Since buffers are processed in FIFO order and we try to
+  // keep some in the queue, it is likely that the racing state will have
+  // resolved by the time this card comes up for reprocessing.
   *card_ptr = G1CardTable::dirty_card_val();
-  G1BarrierSet::shared_dirty_card_queue().enqueue(card_ptr);
+  G1DirtyCardQueueSet& dcqs = G1BarrierSet::dirty_card_queue_set();
+  void** buffer = dcqs.allocate_buffer();
+  size_t index = dcqs.buffer_size() - 1;
+  buffer[index] = card_ptr;
+  dcqs.enqueue_completed_buffer(BufferNode::make_node_from_buffer(buffer, index));
 }
 
 void G1RemSet::print_periodic_summary_info(const char* header, uint period_count) {

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

@@ -57,6 +57,9 @@ class HeapRegionClaimer;
 // external heap references into it.  Uses a mod ref bs to track updates,
 // so that they can be used to update the individual region remsets.
 class G1RemSet: public CHeapObj<mtGC> {
+public:
+  typedef CardTable::CardValue CardValue;
+
 private:
   G1RemSetScanState* _scan_state;
 
@@ -72,10 +75,10 @@ class G1RemSet: public CHeapObj<mtGC> {
   void print_merge_heap_roots_stats();
 
   void assert_scan_top_is_null(uint hrm_index) NOT_DEBUG_RETURN;
-public:
 
-  typedef CardTable::CardValue CardValue;
+  void enqueue_for_reprocessing(CardValue* card_ptr);
 
+public:
   // Initialize data that depends on the heap size being known.
   void initialize(uint max_reserved_regions);
 

src/hotspot/share/runtime/mutexLocker.cpp

@@ -74,7 +74,6 @@ Mutex*   NonJavaThreadsListSync_lock  = NULL;
 Monitor* CGC_lock                     = NULL;
 Monitor* STS_lock                     = NULL;
 Monitor* G1OldGCCount_lock            = NULL;
-Mutex*   Shared_DirtyCardQ_lock       = NULL;
 Mutex*   G1DetachedRefinementStats_lock = NULL;
 Mutex*   MarkStackFreeList_lock       = NULL;
 Mutex*   MarkStackChunkList_lock      = NULL;
@@ -217,8 +216,6 @@ void mutex_init() {
   if (UseG1GC) {
     def(G1OldGCCount_lock          , PaddedMonitor, leaf,        true,  _safepoint_check_always);
 
-    def(Shared_DirtyCardQ_lock     , PaddedMutex  , access + 1,  true,  _safepoint_check_never);
-
     def(G1DetachedRefinementStats_lock, PaddedMutex, leaf    ,   true, _safepoint_check_never);
 
     def(FreeList_lock              , PaddedMutex  , leaf     ,   true,  _safepoint_check_never);

src/hotspot/share/runtime/mutexLocker.hpp

@@ -68,9 +68,6 @@ extern Monitor* CGC_lock;                        // used for coordination betwee
                                                  // fore- & background GC threads.
 extern Monitor* STS_lock;                        // used for joining/leaving SuspendibleThreadSet.
 extern Monitor* G1OldGCCount_lock;               // in support of "concurrent" full gc
-extern Mutex*   Shared_DirtyCardQ_lock;          // Lock protecting dirty card
-                                                 // queue shared by
-                                                 // non-Java threads.
 extern Mutex*   G1DetachedRefinementStats_lock;  // Lock protecting detached refinement stats
 extern Mutex*   MarkStackFreeList_lock;          // Protects access to the global mark stack free list.
 extern Mutex*   MarkStackChunkList_lock;         // Protects access to the global mark stack chunk list.