8261842: Shenandoah: cleanup ShenandoahHeapRegionSet

Reviewed-by: rkennke

src/hotspot/share/gc/shenandoah/shenandoahHeapRegionSet.cpp

@@ -33,17 +33,10 @@
 ShenandoahHeapRegionSetIterator::ShenandoahHeapRegionSetIterator(const ShenandoahHeapRegionSet* const set) :
         _set(set), _heap(ShenandoahHeap::heap()), _current_index(0) {}
 
-void ShenandoahHeapRegionSetIterator::reset(const ShenandoahHeapRegionSet* const set) {
-  _set = set;
-  _current_index = 0;
-}
-
 ShenandoahHeapRegionSet::ShenandoahHeapRegionSet() :
   _heap(ShenandoahHeap::heap()),
   _map_size(_heap->num_regions()),
-  _region_size_bytes_shift(ShenandoahHeapRegion::region_size_bytes_shift()),
   _set_map(NEW_C_HEAP_ARRAY(jbyte, _map_size, mtGC)),
-  _biased_set_map(_set_map - ((uintx)_heap->base() >> _region_size_bytes_shift)),
   _region_count(0)
 {
   // Use 1-byte data type
@@ -58,83 +51,40 @@ ShenandoahHeapRegionSet::~ShenandoahHeapRegionSet() {
 }
 
 void ShenandoahHeapRegionSet::add_region(ShenandoahHeapRegion* r) {
-  assert(!is_in(r), "Already in collection set");
+  assert(!is_in(r), "Already in region set");
   _set_map[r->index()] = 1;
   _region_count++;
 }
 
-bool ShenandoahHeapRegionSet::add_region_check_for_duplicates(ShenandoahHeapRegion* r) {
-  if (!is_in(r)) {
-    add_region(r);
-    return true;
-  } else {
-    return false;
-  }
-}
-
 void ShenandoahHeapRegionSet::remove_region(ShenandoahHeapRegion* r) {
   assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Must be at a safepoint");
   assert(Thread::current()->is_VM_thread(), "Must be VMThread");
   assert(is_in(r), "Not in region set");
   _set_map[r->index()] = 0;
-  _region_count --;
+  _region_count--;
 }
 
 void ShenandoahHeapRegionSet::clear() {
   assert(ShenandoahSafepoint::is_at_shenandoah_safepoint(), "Must be at a safepoint");
   Copy::zero_to_bytes(_set_map, _map_size);
-
   _region_count = 0;
 }
 
-ShenandoahHeapRegion* ShenandoahHeapRegionSetIterator::claim_next() {
-  size_t num_regions = _heap->num_regions();
-  if (_current_index >= (jint)num_regions) {
-    return NULL;
-  }
-
-  jint saved_current = _current_index;
-  size_t index = (size_t)saved_current;
-
-  while(index < num_regions) {
-    if (_set->is_in(index)) {
-      jint cur = Atomic::cmpxchg(&_current_index, saved_current, (jint)(index + 1));
-      assert(cur >= (jint)saved_current, "Must move forward");
-      if (cur == saved_current) {
-        assert(_set->is_in(index), "Invariant");
-        return _heap->get_region(index);
-      } else {
-        index = (size_t)cur;
-        saved_current = cur;
-      }
-    } else {
-      index ++;
-    }
-  }
-  return NULL;
-}
-
 ShenandoahHeapRegion* ShenandoahHeapRegionSetIterator::next() {
-  size_t num_regions = _heap->num_regions();
-  for (size_t index = (size_t)_current_index; index < num_regions; index ++) {
+  for (size_t index = _current_index; index < _heap->num_regions(); index++) {
     if (_set->is_in(index)) {
-      _current_index = (jint)(index + 1);
+      _current_index = index + 1;
       return _heap->get_region(index);
     }
   }
-
   return NULL;
 }
 
 void ShenandoahHeapRegionSet::print_on(outputStream* out) const {
   out->print_cr("Region Set : " SIZE_FORMAT "", count());
-
-  debug_only(size_t regions = 0;)
-  for (size_t index = 0; index < _heap->num_regions(); index ++) {
+  for (size_t index = 0; index < _heap->num_regions(); index++) {
     if (is_in(index)) {
       _heap->get_region(index)->print_on(out);
-      debug_only(regions ++;)
     }
   }
-  assert(regions == count(), "Must match");
 }

src/hotspot/share/gc/shenandoah/shenandoahHeapRegionSet.hpp

@@ -37,10 +37,7 @@ class ShenandoahHeapRegionSetIterator : public StackObj {
 private:
   const ShenandoahHeapRegionSet* _set;
   ShenandoahHeap* const _heap;
-
-  shenandoah_padding(0);
-  volatile jint _current_index;
-  shenandoah_padding(1);
+  size_t _current_index;
 
   // No implicit copying: iterators should be passed by reference to capture the state
   NONCOPYABLE(ShenandoahHeapRegionSetIterator);
@@ -48,12 +45,6 @@ private:
 public:
   ShenandoahHeapRegionSetIterator(const ShenandoahHeapRegionSet* const set);
 
-  // Reset existing iterator to new set
-  void reset(const ShenandoahHeapRegionSet* const set);
-
-  // MT version
-  ShenandoahHeapRegion* claim_next();
-
   // Single-thread version
   ShenandoahHeapRegion* next();
 };
@@ -63,21 +54,14 @@ class ShenandoahHeapRegionSet : public CHeapObj<mtGC> {
 private:
   ShenandoahHeap* const _heap;
   size_t const          _map_size;
-  size_t const          _region_size_bytes_shift;
   jbyte* const          _set_map;
-  // Bias set map's base address for fast test if an oop is in set
-  jbyte* const          _biased_set_map;
   size_t                _region_count;
 
 public:
   ShenandoahHeapRegionSet();
   ~ShenandoahHeapRegionSet();
 
-  // Add region to set
   void add_region(ShenandoahHeapRegion* r);
-  bool add_region_check_for_duplicates(ShenandoahHeapRegion* r);
-
-  // Remove region from set
   void remove_region(ShenandoahHeapRegion* r);
 
   size_t count()  const { return _region_count; }
@@ -85,16 +69,10 @@ public:
 
   inline bool is_in(ShenandoahHeapRegion* r) const;
   inline bool is_in(size_t region_idx)       const;
-  inline bool is_in(oop p)                   const;
 
   void print_on(outputStream* out) const;
 
   void clear();
-
-private:
-  jbyte* biased_map_address() const {
-    return _biased_set_map;
-  }
 };
 
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHHEAPREGIONSET_HPP

src/hotspot/share/gc/shenandoah/shenandoahHeapRegionSet.inline.hpp

@@ -25,10 +25,8 @@
 #ifndef SHARE_GC_SHENANDOAH_SHENANDOAHHEAPREGIONSET_INLINE_HPP
 #define SHARE_GC_SHENANDOAH_SHENANDOAHHEAPREGIONSET_INLINE_HPP
 
-#include "gc/shenandoah/shenandoahAsserts.hpp"
 #include "gc/shenandoah/shenandoahHeapRegionSet.hpp"
 #include "gc/shenandoah/shenandoahHeap.hpp"
-#include "gc/shenandoah/shenandoahHeap.inline.hpp"
 #include "gc/shenandoah/shenandoahHeapRegion.hpp"
 
 bool ShenandoahHeapRegionSet::is_in(size_t region_idx) const {
@@ -40,12 +38,4 @@ bool ShenandoahHeapRegionSet::is_in(ShenandoahHeapRegion* r) const {
   return is_in(r->index());
 }
 
-bool ShenandoahHeapRegionSet::is_in(oop p) const {
-  shenandoah_assert_in_heap(NULL, p);
-  uintx index = (cast_from_oop<uintx>(p)) >> _region_size_bytes_shift;
-  // no need to subtract the bottom of the heap from p,
-  // _biased_set_map is biased
-  return _biased_set_map[index] == 1;
-}
-
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHHEAPREGIONSET_INLINE_HPP