New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8257774: G1: Trigger collect when free region count drops below threshold to prevent evacuation failures #3143
Changes from 8 commits
fe61d04
56056d7
5889ff8
eff81a2
d4e02ed
42f3d7d
4ec80ab
66fa097
80df203
57421e1
d6ea382
6325a72
a2b0b67
817e9bb
6e11e5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -112,10 +112,21 @@ class G1Allocator : public CHeapObj<mtGC> { | ||
|
||
// Allocate blocks of memory during mutator time. | ||
|
||
// Attempt allocation in the current alloc region. If use_retained_region_if_available | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the reference to use_retained_region_if_available out of date? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// is set and a retained region is available, the allocation will first be tried in the | ||
// retained region. | ||
inline HeapWord* attempt_allocation(size_t min_word_size, | ||
size_t desired_word_size, | ||
size_t* actual_word_size); | ||
|
||
// Attempt allocation, retiring the current region and allocating a new one. It is | ||
// assumed that attempt_allocation() has been tried and failed already first. | ||
inline HeapWord* attempt_allocation_use_new_region(size_t word_size); | ||
|
||
// This is to be called when holding an appropriate lock. It first tries in the | ||
// current allocation region, and then attempts an allocation using a new region. | ||
inline HeapWord* attempt_allocation_locked(size_t word_size); | ||
|
||
inline HeapWord* attempt_allocation_force(size_t word_size); | ||
|
||
size_t unsafe_max_tlab_alloc(); | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -52,16 +52,29 @@ inline HeapWord* G1Allocator::attempt_allocation(size_t min_word_size, | ||
size_t desired_word_size, | ||
size_t* actual_word_size) { | ||
uint node_index = current_node_index(); | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove at least one of those two blank lines. |
||
HeapWord* result = mutator_alloc_region(node_index)->attempt_retained_allocation(min_word_size, desired_word_size, actual_word_size); | ||
if (result != NULL) { | ||
return result; | ||
} | ||
|
||
return mutator_alloc_region(node_index)->attempt_allocation(min_word_size, desired_word_size, actual_word_size); | ||
} | ||
|
||
inline HeapWord* G1Allocator::attempt_allocation_use_new_region(size_t word_size) { | ||
uint node_index = current_node_index(); | ||
size_t temp; | ||
HeapWord* result = mutator_alloc_region(node_index)->attempt_allocation_use_new_region(word_size, word_size, &temp); | ||
assert(result != NULL || mutator_alloc_region(node_index)->get() == NULL, | ||
"Must not have a mutator alloc region if there is no memory, but is " PTR_FORMAT, p2i(mutator_alloc_region(node_index)->get())); | ||
adityamandaleeka marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return result; | ||
} | ||
|
||
inline HeapWord* G1Allocator::attempt_allocation_locked(size_t word_size) { | ||
uint node_index = current_node_index(); | ||
HeapWord* result = mutator_alloc_region(node_index)->attempt_allocation_locked(word_size); | ||
|
||
assert(result != NULL || mutator_alloc_region(node_index)->get() == NULL, | ||
"Must not have a mutator alloc region if there is no memory, but is " PTR_FORMAT, p2i(mutator_alloc_region(node_index)->get())); | ||
return result; | ||
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -68,6 +68,8 @@ G1Policy::G1Policy(STWGCTimer* gc_timer) : | ||||||||
_reserve_regions(0), | ||||||||
_young_gen_sizer(), | ||||||||
_free_regions_at_end_of_collection(0), | ||||||||
_predicted_surviving_bytes_from_survivor(0), | ||||||||
_predicted_surviving_bytes_from_old(0), | ||||||||
_rs_length(0), | ||||||||
_rs_length_prediction(0), | ||||||||
_pending_cards_at_gc_start(0), | ||||||||
@@ -450,6 +452,7 @@ void G1Policy::record_full_collection_end() { | ||||||||
// also call this on any additional surv rate groups | ||||||||
|
||||||||
_free_regions_at_end_of_collection = _g1h->num_free_regions(); | ||||||||
update_survival_estimates_for_next_collection(); | ||||||||
_survivor_surv_rate_group->reset(); | ||||||||
update_young_list_max_and_target_length(); | ||||||||
update_rs_length_prediction(); | ||||||||
@@ -780,6 +783,9 @@ void G1Policy::record_collection_pause_end(double pause_time_ms, bool concurrent | ||||||||
|
||||||||
update_rs_length_prediction(); | ||||||||
|
||||||||
// Is this the right place? Should it be in the below? | ||||||||
update_survival_estimates_for_next_collection(); | ||||||||
|
||||||||
// Do not update dynamic IHOP due to G1 periodic collection as it is highly likely | ||||||||
// that in this case we are not running in a "normal" operating mode. | ||||||||
if (_g1h->gc_cause() != GCCause::_g1_periodic_collection) { | ||||||||
@@ -1400,6 +1406,85 @@ void G1Policy::calculate_optional_collection_set_regions(G1CollectionSetCandidat | ||||||||
num_optional_regions, max_optional_regions, prediction_ms); | ||||||||
} | ||||||||
|
||||||||
// Number of regions required to store the given number of bytes, taking | ||||||||
// into account the target amount of wasted space in PLABs. | ||||||||
static size_t get_num_regions_adjust_for_plab_waste(size_t byte_count) { | ||||||||
size_t byte_count_adjusted = byte_count * (size_t)(100 + TargetPLABWastePct) / 100.0; | ||||||||
|
||||||||
// Round up the region count | ||||||||
return (byte_count_adjusted + HeapRegion::GrainBytes - 1) / HeapRegion::GrainBytes; | ||||||||
} | ||||||||
|
||||||||
bool G1Policy::proactive_collection_required(uint alloc_region_count) { | ||||||||
if (!Universe::is_fully_initialized()) { | ||||||||
adityamandaleeka marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
// Don't attempt any proactive GC's before initialization is complete. | ||||||||
return false; | ||||||||
} | ||||||||
|
||||||||
if (_g1h->young_regions_count() == 0 && _collection_set->candidates() != NULL && _collection_set->candidates()->is_empty()) { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a comment that we have no regions to collect. Adding a
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thomas and I talked about this before but decided not to do it because the two cases where we're doing this are doing slightly different things (one is checking for NULL || empty and the other is checking !NULL && empty, which is not the inverse of the first). Looking at it closer now though, I think this one should also be bailing in the case where candidates is NULL right? If we do that, we can definitely add the has_candidates function and use that in both cases. |
||||||||
return false; | ||||||||
} | ||||||||
|
||||||||
uint eden_count = _g1h->eden_regions_count(); | ||||||||
size_t const eden_surv_bytes_pred = _eden_surv_rate_group->accum_surv_rate_pred(eden_count) * HeapRegion::GrainBytes; | ||||||||
size_t const total_young_predicted_surviving_bytes = eden_surv_bytes_pred + _predicted_surviving_bytes_from_survivor; | ||||||||
|
||||||||
uint required_regions = (uint)(get_num_regions_adjust_for_plab_waste(total_young_predicted_surviving_bytes) + | ||||||||
get_num_regions_adjust_for_plab_waste(_predicted_surviving_bytes_from_old)); | ||||||||
|
||||||||
if (required_regions > _g1h->num_free_regions() - alloc_region_count) { | ||||||||
log_debug(gc, ergo, cset)("Proactive GC, insufficient free regions. Predicted need %u. Curr Eden %u (Pred %u). Curr Survivor %u (Pred %u). Curr Old %u (Pred %u) Free %u Alloc %u", | ||||||||
required_regions, | ||||||||
eden_count, | ||||||||
(uint)get_num_regions_adjust_for_plab_waste(eden_surv_bytes_pred), | ||||||||
_g1h->survivor_regions_count(), | ||||||||
(uint)get_num_regions_adjust_for_plab_waste(_predicted_surviving_bytes_from_survivor), | ||||||||
_g1h->old_regions_count(), | ||||||||
(uint)get_num_regions_adjust_for_plab_waste(_predicted_surviving_bytes_from_old), | ||||||||
_g1h->num_free_regions(), | ||||||||
alloc_region_count); | ||||||||
|
||||||||
return true; | ||||||||
} | ||||||||
|
||||||||
return false; | ||||||||
} | ||||||||
|
||||||||
void G1Policy::update_survival_estimates_for_next_collection() { | ||||||||
// Predict the number of bytes of surviving objects from survivor and old | ||||||||
// regions and update the associated members. | ||||||||
|
||||||||
// Survivor regions | ||||||||
size_t survivor_bytes = 0; | ||||||||
const GrowableArray<HeapRegion*>* survivor_regions = _g1h->survivor()->regions(); | ||||||||
for (GrowableArrayIterator<HeapRegion*> it = survivor_regions->begin(); | ||||||||
it != survivor_regions->end(); | ||||||||
++it) { | ||||||||
survivor_bytes += predict_bytes_to_copy(*it); | ||||||||
} | ||||||||
|
||||||||
_predicted_surviving_bytes_from_survivor = survivor_bytes; | ||||||||
|
||||||||
// Old regions | ||||||||
G1CollectionSetCandidates *candidates = _collection_set->candidates(); | ||||||||
if (candidates == NULL || candidates->is_empty()) { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could use |
||||||||
_predicted_surviving_bytes_from_old = 0; | ||||||||
return; | ||||||||
} | ||||||||
|
||||||||
// Use the minimum old gen collection set as conservative estimate for the number | ||||||||
// of regions to take for this calculation. | ||||||||
uint iterate_count = MIN2(candidates->num_remaining(), calc_min_old_cset_length(candidates)); | ||||||||
uint current_index = candidates->cur_idx(); | ||||||||
size_t old_bytes = 0; | ||||||||
for (uint i = 0; i < iterate_count; i++) { | ||||||||
HeapRegion *region = candidates->at(current_index + i); | ||||||||
old_bytes += predict_bytes_to_copy(region); | ||||||||
} | ||||||||
|
||||||||
_predicted_surviving_bytes_from_old = old_bytes; | ||||||||
} | ||||||||
|
||||||||
void G1Policy::transfer_survivors_to_cset(const G1SurvivorRegions* survivors) { | ||||||||
note_start_adding_survivor_regions(); | ||||||||
|
||||||||
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -122,7 +122,7 @@ VM_G1CollectForAllocation::VM_G1CollectForAllocation(size_t word_size, | ||||||
void VM_G1CollectForAllocation::doit() { | ||||||
G1CollectedHeap* g1h = G1CollectedHeap::heap(); | ||||||
|
||||||
if (_word_size > 0) { | ||||||
if (_word_size > 0 && _gc_cause != GCCause::_g1_proactive_collection) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about adding a helper to ´VM_G1CollectForAllocation`:
The condition would then read a bit better:
Suggested change
|
||||||
// An allocation has been requested. So, try to do that first. | ||||||
_result = g1h->attempt_allocation_at_safepoint(_word_size, | ||||||
false /* expect_null_cur_alloc_region */); | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please format these lines to fit the rest of the comment: