Skip to content

Commit

Permalink
8140326: G1: Consider putting regions where evacuation failed into ne…
Browse files Browse the repository at this point in the history
…xt collection set

Co-authored-by: Albert Mingkun Yang <ayang@openjdk.org>
Reviewed-by: iwalulya, ayang
  • Loading branch information
Thomas Schatzl and albertnetymk committed Aug 8, 2023
1 parent 28fd7a1 commit 7e20952
Show file tree
Hide file tree
Showing 36 changed files with 593 additions and 151 deletions.
14 changes: 11 additions & 3 deletions src/hotspot/share/gc/g1/g1CollectedHeap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2464,6 +2464,12 @@ void G1CollectedHeap::verify_after_young_collection(G1HeapVerifier::G1VerifyType
_verifier->verify_after_gc();
verify_numa_regions("GC End");
_verifier->verify_region_sets_optional();

if (collector_state()->in_concurrent_start_gc()) {
log_debug(gc, verify)("Marking state");
_verifier->verify_marking_state();
}

phase_times()->record_verify_after_time_ms((Ticks::now() - start).seconds() * MILLIUNITS);
}

Expand Down Expand Up @@ -2652,6 +2658,11 @@ void G1CollectedHeap::free_region(HeapRegion* hr, FreeRegionList* free_list) {
}
}

void G1CollectedHeap::retain_region(HeapRegion* hr) {
MutexLocker x(G1RareEvent_lock, Mutex::_no_safepoint_check_flag);
collection_set()->candidates()->add_retained_region_unsorted(hr);
}

void G1CollectedHeap::free_humongous_region(HeapRegion* hr,
FreeRegionList* free_list) {
assert(hr->is_humongous(), "this is only for humongous regions");
Expand Down Expand Up @@ -2977,9 +2988,6 @@ void G1CollectedHeap::mark_evac_failure_object(uint worker_id, const oop obj, si
assert(!_cm->is_marked_in_bitmap(obj), "must be");

_cm->raw_mark_in_bitmap(obj);
if (collector_state()->in_concurrent_start_gc()) {
_cm->add_to_liveness(worker_id, obj, obj_size);
}
}

// Optimized nmethod scanning
Expand Down
4 changes: 3 additions & 1 deletion src/hotspot/share/gc/g1/g1CollectedHeap.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,8 @@ class G1CollectedHeap : public CollectedHeap {
// at the same time.
void free_region(HeapRegion* hr, FreeRegionList* free_list);

// Add the given region to the retained regions collection set candidates.
void retain_region(HeapRegion* hr);
// It dirties the cards that cover the block so that the post
// write barrier never queues anything when updating objects on this
// block. It is assumed (and in fact we assert) that the block
Expand Down Expand Up @@ -1028,7 +1030,7 @@ class G1CollectedHeap : public CollectedHeap {

// Return "TRUE" iff the given object address is within the collection
// set. Assumes that the reference points into the heap.
inline bool is_in_cset(const HeapRegion *hr) const;
inline bool is_in_cset(const HeapRegion* hr) const;
inline bool is_in_cset(oop obj) const;
inline bool is_in_cset(HeapWord* addr) const;

Expand Down
15 changes: 10 additions & 5 deletions src/hotspot/share/gc/g1/g1CollectionSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,16 +320,21 @@ static int compare_region_idx(const uint a, const uint b) {
void G1CollectionSet::finalize_old_part(double time_remaining_ms) {
double non_young_start_time_sec = os::elapsedTime();

if (collector_state()->in_mixed_phase()) {
if (!candidates()->is_empty()) {
candidates()->verify();

G1CollectionCandidateRegionList initial_old_regions;
assert(_optional_old_regions.length() == 0, "must be");

_policy->select_candidates_from_marking(&candidates()->marking_regions(),
time_remaining_ms,
&initial_old_regions,
&_optional_old_regions);
time_remaining_ms = _policy->select_candidates_from_marking(&candidates()->marking_regions(),
time_remaining_ms,
&initial_old_regions,
&_optional_old_regions);

_policy->select_candidates_from_retained(&candidates()->retained_regions(),
time_remaining_ms,
&initial_old_regions,
&_optional_old_regions);

// Move initially selected old regions to collection set directly.
move_candidates_to_collection_set(&initial_old_regions);
Expand Down
65 changes: 61 additions & 4 deletions src/hotspot/share/gc/g1/g1CollectionSetCandidates.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ void G1CollectionCandidateList::set(G1CollectionCandidateList::CandidateInfo* ca
_candidates.appendAll(&a);
}

void G1CollectionCandidateList::append_unsorted(HeapRegion* r) {
CandidateInfo c(r, r->calc_gc_efficiency());
_candidates.append(c);
}

void G1CollectionCandidateList::sort_by_efficiency() {
_candidates.sort(compare);
}

void G1CollectionCandidateList::remove(G1CollectionCandidateRegionList* other) {
guarantee((uint)_candidates.length() >= other->length(), "must be");

Expand Down Expand Up @@ -142,6 +151,7 @@ void G1CollectionCandidateRegionList::clear() {

G1CollectionSetCandidates::G1CollectionSetCandidates() :
_marking_regions(),
_retained_regions(),
_contains_map(nullptr),
_max_regions(0),
_last_marking_candidates_length(0)
Expand All @@ -165,6 +175,7 @@ void G1CollectionSetCandidates::initialize(uint max_regions) {

void G1CollectionSetCandidates::clear() {
_marking_regions.clear();
_retained_regions.clear();
for (uint i = 0; i < _max_regions; i++) {
_contains_map[i] = CandidateOrigin::Invalid;
}
Expand All @@ -188,8 +199,40 @@ void G1CollectionSetCandidates::set_candidates_from_marking(G1CollectionCandidat
verify();
}

void G1CollectionSetCandidates::sort_by_efficiency() {
// From marking regions must always be sorted so no reason to actually sort
// them.
_marking_regions.verify();
_retained_regions.sort_by_efficiency();
_retained_regions.verify();
}

void G1CollectionSetCandidates::add_retained_region_unsorted(HeapRegion* r) {
assert(!contains(r), "must not contain region %u", r->hrm_index());
_contains_map[r->hrm_index()] = CandidateOrigin::Retained;
_retained_regions.append_unsorted(r);
}

void G1CollectionSetCandidates::remove(G1CollectionCandidateRegionList* other) {
_marking_regions.remove(other);
// During removal, we exploit the fact that elements in the marking_regions,
// retained_regions and other list are sorted by gc_efficiency. Furthermore,
// all regions in the passed other list are in one of the two other lists.
//
// Split original list into elements for the marking list and elements from the
// retained list.
G1CollectionCandidateRegionList other_marking_regions;
G1CollectionCandidateRegionList other_retained_regions;

for (HeapRegion* r : *other) {
if (is_from_marking(r)) {
other_marking_regions.append(r);
} else {
other_retained_regions.append(r);
}
}

_marking_regions.remove(&other_marking_regions);
_retained_regions.remove(&other_retained_regions);

for (HeapRegion* r : *other) {
assert(contains(r), "must contain region %u", r->hrm_index());
Expand All @@ -204,7 +247,15 @@ bool G1CollectionSetCandidates::is_empty() const {
}

bool G1CollectionSetCandidates::has_more_marking_candidates() const {
return _marking_regions.length() != 0;
return marking_regions_length() != 0;
}

uint G1CollectionSetCandidates::marking_regions_length() const {
return _marking_regions.length();
}

uint G1CollectionSetCandidates::retained_regions_length() const {
return _retained_regions.length();
}

#ifndef PRODUCT
Expand All @@ -218,7 +269,7 @@ void G1CollectionSetCandidates::verify_helper(G1CollectionCandidateList* list, u
from_marking++;
}
const uint hrm_index = r->hrm_index();
assert(_contains_map[hrm_index] == CandidateOrigin::Marking,
assert(_contains_map[hrm_index] == CandidateOrigin::Marking || _contains_map[hrm_index] == CandidateOrigin::Retained,
"must be %u is %u", hrm_index, (uint)_contains_map[hrm_index]);
assert(verify_map[hrm_index] == CandidateOrigin::Invalid, "already added");

Expand All @@ -235,9 +286,14 @@ void G1CollectionSetCandidates::verify() {
}

verify_helper(&_marking_regions, from_marking, verify_map);

assert(from_marking == marking_regions_length(), "must be");

uint from_marking_retained = 0;
verify_helper(&_retained_regions, from_marking_retained, verify_map);
assert(from_marking_retained == 0, "must be");

assert(length() >= marking_regions_length(), "must be");

// Check whether the _contains_map is consistent with the list.
for (uint i = 0; i < _max_regions; i++) {
assert(_contains_map[i] == verify_map[i] ||
Expand All @@ -262,6 +318,7 @@ const char* G1CollectionSetCandidates::get_short_type_str(const HeapRegion* r) c
static const char* type_strings[] = {
"Ci", // Invalid
"Cm", // Marking
"Cr", // Retained
"Cv" // Verification
};

Expand Down
39 changes: 28 additions & 11 deletions src/hotspot/share/gc/g1/g1CollectionSetCandidates.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ class G1CollectionCandidateList : public CHeapObj<mtGC> {

// Put the given set of candidates into this list, preserving the efficiency ordering.
void set(CandidateInfo* candidate_infos, uint num_infos);
// Add the given HeapRegion to this list at the end, (potentially) making the list unsorted.
void append_unsorted(HeapRegion* r);
// Restore sorting order by decreasing gc efficiency, using the existing efficiency
// values.
void sort_by_efficiency();
// Removes any HeapRegions stored in this list also in the other list. The other
// list may only contain regions in this list, sorted by gc efficiency. It need
// not be a prefix of this list. Returns the number of regions removed.
Expand Down Expand Up @@ -129,13 +134,14 @@ class G1CollectionCandidateList : public CHeapObj<mtGC> {
}
};

// Iterator for G1CollectionSetCandidates.
// Iterator for G1CollectionSetCandidates. There are no guarantees on the order
// of the regions returned.
class G1CollectionSetCandidatesIterator : public StackObj {
G1CollectionSetCandidates* _which;
uint _marking_position;
uint _position;

public:
G1CollectionSetCandidatesIterator(G1CollectionSetCandidates* which, uint marking_position);
G1CollectionSetCandidatesIterator(G1CollectionSetCandidates* which, uint position);

G1CollectionSetCandidatesIterator& operator++();
HeapRegion* operator*();
Expand All @@ -146,26 +152,30 @@ class G1CollectionSetCandidatesIterator : public StackObj {

// Tracks all collection set candidates, i.e. regions that could/should be evacuated soon.
//
// These candidate regions are tracked in a list of regions, sorted by decreasing
// These candidate regions are tracked in two list of regions, sorted by decreasing
// "gc efficiency".
//
// Currently there is only one type of such regions:
//
// * marking_regions: the set of regions selected by concurrent marking to be
// evacuated to keep overall heap occupancy stable.
// They are guaranteed to be evacuated and cleared out during
// the mixed phase.
//
// * retained_regions: set of regions selected for evacuation during evacuation
// failure.
// Any young collection will try to evacuate them.
//
class G1CollectionSetCandidates : public CHeapObj<mtGC> {
friend class G1CollectionSetCandidatesIterator;

enum class CandidateOrigin : uint8_t {
Invalid,
Marking, // This region has been determined as candidate by concurrent marking.
Retained, // This region has been added because it has been retained after evacuation.
Verify // Special value for verification.
};

G1CollectionCandidateList _marking_regions;
G1CollectionCandidateList _marking_regions; // Set of regions selected by concurrent marking.
G1CollectionCandidateList _retained_regions; // Set of regions selected from evacuation failed regions.

CandidateOrigin* _contains_map;
uint _max_regions;
Expand All @@ -180,6 +190,7 @@ class G1CollectionSetCandidates : public CHeapObj<mtGC> {
~G1CollectionSetCandidates();

G1CollectionCandidateList& marking_regions() { return _marking_regions; }
G1CollectionCandidateList& retained_regions() { return _retained_regions; }

void initialize(uint max_regions);

Expand All @@ -194,6 +205,11 @@ class G1CollectionSetCandidates : public CHeapObj<mtGC> {
// regions.
uint last_marking_candidates_length() const { return _last_marking_candidates_length; }

void sort_by_efficiency();

// Add the given region to the set of retained regions without regards to the
// gc efficiency sorting. The retained regions must be re-sorted manually later.
void add_retained_region_unsorted(HeapRegion* r);
// Remove the given regions from the candidates. All given regions must be part
// of the candidates.
void remove(G1CollectionCandidateRegionList* other);
Expand All @@ -203,25 +219,26 @@ class G1CollectionSetCandidates : public CHeapObj<mtGC> {
const char* get_short_type_str(const HeapRegion* r) const;

bool is_empty() const;
bool has_more_marking_candidates() const;

uint marking_regions_length() const { return _marking_regions.length(); }
bool has_more_marking_candidates() const;
uint marking_regions_length() const;
uint retained_regions_length() const;

private:
void verify_helper(G1CollectionCandidateList* list, uint& from_marking, CandidateOrigin* verify_map) PRODUCT_RETURN;

public:
void verify() PRODUCT_RETURN;

uint length() const { return marking_regions_length(); }
uint length() const { return marking_regions_length() + retained_regions_length(); }

// Iteration
G1CollectionSetCandidatesIterator begin() {
return G1CollectionSetCandidatesIterator(this, 0);
}

G1CollectionSetCandidatesIterator end() {
return G1CollectionSetCandidatesIterator(this, marking_regions_length());
return G1CollectionSetCandidatesIterator(this, length());
}
};

Expand Down
19 changes: 11 additions & 8 deletions src/hotspot/share/gc/g1/g1CollectionSetCandidates.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,25 +51,28 @@ inline bool G1CollectionCandidateListIterator::operator!=(const G1CollectionCand
return !(*this == rhs);
}

inline G1CollectionSetCandidatesIterator::G1CollectionSetCandidatesIterator(G1CollectionSetCandidates* which, uint marking_position) :
_which(which), _marking_position(marking_position) {
inline G1CollectionSetCandidatesIterator::G1CollectionSetCandidatesIterator(G1CollectionSetCandidates* which, uint position) :
_which(which), _position(position) {
}

inline G1CollectionSetCandidatesIterator& G1CollectionSetCandidatesIterator::operator++() {
assert(_marking_position < _which->_marking_regions.length(),
"must not be at end already");

_marking_position++;
assert(_position < _which->length(), "must not be at end already");
_position++;
return *this;
}

inline HeapRegion* G1CollectionSetCandidatesIterator::operator*() {
return _which->_marking_regions.at(_marking_position)._r;
uint length = _which->marking_regions_length();
if (_position < length) {
return _which->_marking_regions.at(_position)._r;
} else {
return _which->_retained_regions.at(_position - length)._r;
}
}

inline bool G1CollectionSetCandidatesIterator::operator==(const G1CollectionSetCandidatesIterator& rhs) {
assert(_which == rhs._which, "iterator belongs to different array");
return _marking_position == rhs._marking_position;
return _position == rhs._position;
}

inline bool G1CollectionSetCandidatesIterator::operator!=(const G1CollectionSetCandidatesIterator& rhs) {
Expand Down
Loading

1 comment on commit 7e20952

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.