Skip to content

Commit

Permalink
8261842: Shenandoah: cleanup ShenandoahHeapRegionSet
Browse files Browse the repository at this point in the history 
Reviewed-by: rkennke
  • Loading branch information
@shipilev
shipilev committed Feb 17, 2021
1 parent fc1d032 commit d195033
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 88 deletions.
60 changes: 5 additions & 55 deletions src/hotspot/share/gc/shenandoah/shenandoahHeapRegionSet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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");
}
24 changes: 1 addition & 23 deletions src/hotspot/share/gc/shenandoah/shenandoahHeapRegionSet.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,14 @@ 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);

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();
};
Expand All @@ -63,38 +54,25 @@ 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; }
bool is_empty() const { return _region_count == 0; }

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
10 changes: 0 additions & 10 deletions src/hotspot/share/gc/shenandoah/shenandoahHeapRegionSet.inline.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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

1 comment on commit d195033

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.