ayang review 2

* removal of useless code
* renamings

src/hotspot/cpu/x86/gc/g1/g1BarrierSetAssembler_x86.cpp

@@ -103,7 +103,7 @@ void G1BarrierSetAssembler::gen_write_ref_array_post_barrier(MacroAssembler* mas
   Label done;
 
   __ testptr(count, count);
-  __ jcc(Assembler::equal, done);
+  __ jcc(Assembler::zero, done);
 
   // Calculate end address in "count".
   Address::ScaleFactor scale = UseCompressedOops ? Address::times_4 : Address::times_8;
@@ -129,8 +129,10 @@ void G1BarrierSetAssembler::gen_write_ref_array_post_barrier(MacroAssembler* mas
   __ bind(loop);
 
   Label is_clean_card;
-  __ cmpb(Address(addr, 0), G1CardTable::clean_card_val());
-  __ jcc(Assembler::equal, is_clean_card);
+  if (UseCondCardMark) {
+    __ cmpb(Address(addr, 0), G1CardTable::clean_card_val());
+    __ jcc(Assembler::equal, is_clean_card);
+  }
 
   Label next_card;
   __ bind(next_card);
@@ -320,7 +322,7 @@ static void generate_post_barrier_fast_path(MacroAssembler* masm,
 #ifdef _LP64
   assert(thread == r15_thread, "must be");
 #endif // _LP64
-  assert_different_registers(store_addr, new_val, thread, tmp1 /*, tmp2 unused */, noreg);
+  assert_different_registers(store_addr, new_val, thread, tmp1 /*, tmp2 unused for x86 */, noreg);
 
   // Does store cross heap regions?
   __ movptr(tmp1, store_addr);                                    // tmp1 := store address

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

@@ -46,17 +46,17 @@ void G1CardTableClaimTable::initialize(uint max_reserved_regions) {
   assert(_card_claims == nullptr, "Must not be initialized twice");
   _card_claims = NEW_C_HEAP_ARRAY(uint, max_reserved_regions, mtGC);
   _max_reserved_regions = max_reserved_regions;
-  reset_all_claims_to_unclaimed();
+  reset_all_to_unclaimed();
 }
 
-void G1CardTableClaimTable::reset_all_claims_to_unclaimed() {
-  for (size_t i = 0; i < _max_reserved_regions; i++) {
+void G1CardTableClaimTable::reset_all_to_unclaimed() {
+  for (uint i = 0; i < _max_reserved_regions; i++) {
     _card_claims[i] = 0;
   }
 }
 
-void G1CardTableClaimTable::reset_all_claims_to_claimed() {
-  for (size_t i = 0; i < _max_reserved_regions; i++) {
+void G1CardTableClaimTable::reset_all_to_claimed() {
+  for (uint i = 0; i < _max_reserved_regions; i++) {
     _card_claims[i] = (uint)G1HeapRegion::CardsPerRegion;
   }
 }

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

@@ -59,8 +59,8 @@ class G1CardTableClaimTable : public CHeapObj<mtGC> {
   // Allocates the data structure and initializes the claims to unclaimed.
   void initialize(uint max_reserved_regions);
 
-  void reset_all_claims_to_unclaimed();
-  void reset_all_claims_to_claimed();
+  void reset_all_to_unclaimed();
+  void reset_all_to_claimed();
 
   inline bool has_unclaimed_cards(uint region);
   inline void reset_to_unclaimed(uint region);

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

@@ -346,7 +346,7 @@ bool G1ConcurrentRefineWorkState::complete(bool concurrent, bool print_log) {
 void G1ConcurrentRefineWorkState::snapshot_heap_into(G1CardTableClaimTable* sweep_table) {
   // G1CollectedHeap::heap_region_iterate() below will only visit committed regions. Initialize
   // all entries in the state table here to not require special handling when iterating over it.
-  sweep_table->reset_all_claims_to_claimed();
+  sweep_table->reset_all_to_claimed();
 
   class SnapshotRegionsClosure : public G1HeapRegionClosure {
     G1CardTableClaimTable* _sweep_table;

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

@@ -268,7 +268,7 @@ class G1ClearCardTableTask : public G1AbstractSubTask {
     // regions.
     //assert(_next_dirty_regions->size() == 0, "next dirty regions must be empty");
 
-    _card_claim_table.reset_all_claims_to_unclaimed();
+    _card_claim_table.reset_all_to_unclaimed();
   }
 
   void complete_evac_phase(bool merge_dirty_regions) {
@@ -829,12 +829,12 @@ class MergeRefinementTableTask : public WorkerTask {
 
         size_t* card_cur_word = (size_t*)card_table->byte_for_index(start_idx);
 
-        size_t* refinement_cur_card = (size_t*)refinement_table->byte_for_index(start_idx);
-        size_t* const refinement_end_card = refinement_cur_card + claim.size() / (sizeof(size_t) / sizeof(G1CardTable::CardValue));
+        size_t* refinement_cur_word = (size_t*)refinement_table->byte_for_index(start_idx);
+        size_t* const refinement_end_word = refinement_cur_word + claim.size() / (sizeof(size_t) / sizeof(G1CardTable::CardValue));
 
-        for (; refinement_cur_card < refinement_end_card; ++refinement_cur_card, ++card_cur_word) {
-          size_t value = *refinement_cur_card;
-          *refinement_cur_card = G1CardTable::WordAllClean;
+        for (; refinement_cur_word < refinement_end_word; ++refinement_cur_word, ++card_cur_word) {
+          size_t value = *refinement_cur_word;
+          *refinement_cur_word = G1CardTable::WordAllClean;
           // Dirty is "0", so we need to logically-and here. This is also safe
           // for all other possible values in the card table; at this point this
           // can be either g1_dirty_card or g1_to_cset_card which will both be

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

@@ -91,6 +91,12 @@ class G1ThreadLocalData {
     data(thread)->_byte_map_base = new_byte_map_base;
   }
 
+#ifdef ASSERT
+  static G1CardTable::CardValue* get_byte_map_base(Thread* thread) {
+    return data(thread)->_byte_map_base;
+  }
+#endif
+
   static G1RegionPinCache& pin_count_cache(Thread* thread) {
     return data(thread)->_pin_cache;
   }

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

@@ -830,43 +830,41 @@ class G1PostEvacuateCollectionSetCleanupTask2::FreeCollectionSetTask : public G1
 
 class G1PostEvacuateCollectionSetCleanupTask2::ResizeTLABsAndSwapCardTableTask : public G1AbstractSubTask {
   G1JavaThreadsListClaimer _claimer;
-  volatile bool _non_java_threads_claim;
 
   // There is not much work per thread so the number of threads per worker is high.
   static const uint ThreadsPerWorker = 250;
 
 public:
   ResizeTLABsAndSwapCardTableTask()
-    : G1AbstractSubTask(G1GCPhaseTimes::ResizeThreadLABs), _claimer(ThreadsPerWorker), _non_java_threads_claim(false)
+    : G1AbstractSubTask(G1GCPhaseTimes::ResizeThreadLABs), _claimer(ThreadsPerWorker)
   {
     G1BarrierSet::g1_barrier_set()->swap_global_card_table();
-  }
 
-  void do_work(uint worker_id) override {
-    class SwapCardTableClosure : public ThreadClosure {
+#ifdef ASSERT
+    class AssertCardTableBaseNull : public ThreadClosure {
     public:
+
       void do_thread(Thread* thread) {
-        // The global card table references have already been swapped.
-        G1CardTable::CardValue* new_card_table_base = G1CollectedHeap::heap()->card_table_base();
-        G1ThreadLocalData::set_byte_map_base(thread, new_card_table_base);
+        assert(G1ThreadLocalData::get_byte_map_base(thread) == nullptr, "thread " PTR_FORMAT " (%s) has non-null card table base", p2i(thread), thread->name());
       }
-    } swap_cl;
+    } assert_cl;
+    Threads::non_java_threads_do(&assert_cl);
+#endif
+  }
 
-    // We do not expect too many non-Java threads compared to Java threads, so just
-    // let one worker claim that work.
-    if (!_non_java_threads_claim && !Atomic::cmpxchg(&_non_java_threads_claim, false, true, memory_order_relaxed)) {
-      Threads::non_java_threads_do(&swap_cl);
-    }
+  void do_work(uint worker_id) override {
 
     class ResizeAndSwapCardTableClosure : public ThreadClosure {
-    SwapCardTableClosure _cl;
-
     public:
+
       void do_thread(Thread* thread) {
         if (UseTLAB && ResizeTLAB) {
           static_cast<JavaThread*>(thread)->tlab().resize();
         }
-        _cl.do_thread(thread);
+
+        // The global card table references have already been swapped.
+        G1CardTable::CardValue* new_card_table_base = G1CollectedHeap::heap()->card_table_base();
+        G1ThreadLocalData::set_byte_map_base(thread, new_card_table_base);
       }
     } resize_and_swap_cl;