Skip to content
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

8276098: Do precise BOT updates in G1 evacuation phase #6166

Closed
wants to merge 9 commits into from
Closed
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -43,10 +43,7 @@ void G1AllocRegion::setup(G1CollectedHeap* g1h, HeapRegion* dummy_region) {

// Make sure that any allocation attempt on this region will fail
// and will not trigger any asserts.
assert(dummy_region->allocate_no_bot_updates(1) == NULL, "should fail");
assert(dummy_region->allocate(1) == NULL, "should fail");
DEBUG_ONLY(size_t assert_tmp);
assert(dummy_region->par_allocate_no_bot_updates(1, 1, &assert_tmp) == NULL, "should fail");
assert(dummy_region->par_allocate(1, 1, &assert_tmp) == NULL, "should fail");

_g1h = g1h;
@@ -77,8 +74,9 @@ size_t G1AllocRegion::fill_up_remaining_space(HeapRegion* alloc_region) {
while (free_word_size >= min_word_size_to_fill) {
HeapWord* dummy = par_allocate(alloc_region, free_word_size);
if (dummy != NULL) {
// If the allocation was successful we should fill in the space.
CollectedHeap::fill_with_object(dummy, free_word_size);
// If the allocation was successful we should fill in the space. If the
// allocation was in old any necessary BOT updates will be done.
alloc_region->fill_with_dummy_object(dummy, free_word_size);
alloc_region->set_pre_dummy_top(dummy);
result += free_word_size * HeapWordSize;
break;
@@ -256,7 +254,6 @@ G1AllocRegion::G1AllocRegion(const char* name,
: _alloc_region(NULL),
_count(0),
_used_bytes_before(0),
_bot_updates(bot_updates),
_name(name),
_node_index(node_index)
{ }
@@ -389,7 +386,7 @@ HeapRegion* OldGCAllocRegion::release() {
// original problem cannot occur.
if (to_allocate_words >= G1CollectedHeap::min_fill_size()) {
HeapWord* dummy = attempt_allocation(to_allocate_words);
CollectedHeap::fill_with_object(dummy, to_allocate_words);
cur->fill_with_dummy_object(dummy, to_allocate_words);
}
}
}
@@ -68,9 +68,6 @@ class G1AllocRegion : public CHeapObj<mtGC> {
// we allocated in it.
size_t _used_bytes_before;

// When true, indicates that allocate calls should do BOT updates.
const bool _bot_updates;

// Useful for debugging and tracing.
const char* _name;

@@ -31,9 +31,9 @@

#define assert_alloc_region(p, message) \
do { \
assert((p), "[%s] %s c: %u b: %s r: " PTR_FORMAT " u: " SIZE_FORMAT, \
_name, (message), _count, BOOL_TO_STR(_bot_updates), \
p2i(_alloc_region), _used_bytes_before); \
assert((p), "[%s] %s c: %u r: " PTR_FORMAT " u: " SIZE_FORMAT, \
_name, (message), _count, p2i(_alloc_region), \
_used_bytes_before); \
} while (0)


@@ -45,11 +45,7 @@ inline HeapWord* G1AllocRegion::allocate(HeapRegion* alloc_region,
size_t word_size) {
assert(alloc_region != NULL, "pre-condition");

if (!_bot_updates) {
return alloc_region->allocate_no_bot_updates(word_size);
} else {
return alloc_region->allocate(word_size);
}
return alloc_region->allocate(word_size);
}

inline HeapWord* G1AllocRegion::par_allocate(HeapRegion* alloc_region, size_t word_size) {
@@ -64,11 +60,7 @@ inline HeapWord* G1AllocRegion::par_allocate(HeapRegion* alloc_region,
assert(alloc_region != NULL, "pre-condition");
assert(!alloc_region->is_empty(), "pre-condition");

if (!_bot_updates) {
return alloc_region->par_allocate_no_bot_updates(min_word_size, desired_word_size, actual_word_size);
} else {
return alloc_region->par_allocate(min_word_size, desired_word_size, actual_word_size);
}
return alloc_region->par_allocate(min_word_size, desired_word_size, actual_word_size);
}

inline HeapWord* G1AllocRegion::attempt_allocation(size_t word_size) {
@@ -300,7 +300,12 @@ G1PLABAllocator::G1PLABAllocator(G1Allocator* allocator) :
uint length = alloc_buffers_length(state);
_alloc_buffers[state] = NEW_C_HEAP_ARRAY(PLAB*, length, mtGC);
for (uint node_index = 0; node_index < length; node_index++) {
_alloc_buffers[state][node_index] = new PLAB(_g1h->desired_plab_sz(state));
if (state == G1HeapRegionAttr::Old) {
// Specialized PLABs for old that handle BOT updates for object allocations.
_alloc_buffers[state][node_index] = new G1BotUpdatingPLAB(_g1h->desired_plab_sz(state));
} else {
_alloc_buffers[state][node_index] = new PLAB(_g1h->desired_plab_sz(state));
}
Copy link
Member

@albertnetymk albertnetymk Nov 8, 2021

Choose a reason for hiding this comment

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

I think ternary operator can be used here:

word_sz = _g1h->desired_plab_sz(state);
// ...
_alloc_buffers[state][node_index] = (state == G1HeapRegionAttr::Old)
                                  ? new G1BotUpdatingPLAB(word_sz)
                                  : new PLAB(word_sz);

}
}
}
@@ -360,6 +365,7 @@ HeapWord* G1PLABAllocator::allocate_direct_or_new_plab(G1HeapRegionAttr dest,
// Try direct allocation.
HeapWord* result = _allocator->par_allocate_during_gc(dest, word_sz, node_index);
if (result != NULL) {
update_bot_for_direct_allocation(dest, result, word_sz);
_direct_allocated[dest.type()] += word_sz;
}
return result;
@@ -145,6 +145,22 @@ class G1Allocator : public CHeapObj<mtGC> {
uint node_index);
};

// Specialized PLAB for old generation promotions. For old regions the
// BOT needs to be updated and the relevant data to do this efficiently
// is stored in the PLAB.
class G1BotUpdatingPLAB : public PLAB {
// An object spanning this threshold will cause a BOT update.
HeapWord* _next_bot_threshold;
// The region in which the PLAB resides.
HeapRegion* _region;
public:
G1BotUpdatingPLAB(size_t word_sz) : PLAB(word_sz) { }
// Sets the new PLAB buffer as well as updates the threshold and region.
virtual void set_buf(HeapWord* buf, size_t word_sz);
Copy link
Member

@albertnetymk albertnetymk Nov 8, 2021

Choose a reason for hiding this comment

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

Is override better here?

Copy link
Contributor Author

@kstefanj kstefanj Nov 9, 2021

Choose a reason for hiding this comment

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

I can change this to override if you prefer. I see that we have some uses of it in G1 already.

Copy link
Member

@albertnetymk albertnetymk Nov 9, 2021

Choose a reason for hiding this comment

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

Yes please, since it makes the intention explicit and enforces it in compile-time.

// Updates the BOT if the last allocation crossed the threshold.
inline void update_bot(size_t word_sz);
};

// Manages the PLABs used during garbage collection. Interface for allocation from PLABs.
// Needs to handle multiple contexts, extra alignment in any "survivor" area and some
// statistics.
@@ -165,6 +181,9 @@ class G1PLABAllocator : public CHeapObj<mtGC> {
inline PLAB* alloc_buffer(G1HeapRegionAttr dest, uint node_index) const;
inline PLAB* alloc_buffer(region_type_t dest, uint node_index) const;

// Helpers to do explicit BOT updates for allocations in old generation regions.
void update_bot_for_direct_allocation(G1HeapRegionAttr attr, HeapWord* addr, size_t size);

// Returns the number of allocation buffers for the given dest.
// There is only 1 buffer for Old while Young may have multiple buffers depending on
// active NUMA nodes.
@@ -198,6 +217,9 @@ class G1PLABAllocator : public CHeapObj<mtGC> {
bool* refill_failed,
uint node_index);

// Update the BOT for the last PLAB allocation.
inline void update_bot_for_allocation(G1HeapRegionAttr dest, size_t word_sz, uint node_index);
Copy link
Contributor

@tschatzl tschatzl Nov 8, 2021

Choose a reason for hiding this comment

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

Suggested change
inline void update_bot_for_allocation(G1HeapRegionAttr dest, size_t word_sz, uint node_index);
inline void update_bot_for_allocation(G1HeapRegionAttr dest, size_t word_sz, uint node_index);
Suggested change
inline void update_bot_for_allocation(G1HeapRegionAttr dest, size_t word_sz, uint node_index);
inline void update_bot_for_plab_allocation(G1HeapRegionAttr dest, size_t word_sz, uint node_index);

I would explicitly call this out as to be used for PLAB allocation. If changed, obviously needs updates to the callers as well.


void undo_allocation(G1HeapRegionAttr dest, HeapWord* obj, size_t word_sz, uint node_index);
};

@@ -133,4 +133,46 @@ inline HeapWord* G1PLABAllocator::allocate(G1HeapRegionAttr dest,
return allocate_direct_or_new_plab(dest, word_sz, refill_failed, node_index);
}

inline void G1PLABAllocator::update_bot_for_direct_allocation(G1HeapRegionAttr attr, HeapWord* addr, size_t size) {
if (!attr.needs_bot_update()) {
// BOT updates are only done for old generation.
return;
}

// Out of PLAB allocations in an old generation region. Update BOT.
HeapRegion* region = _g1h->heap_region_containing(addr);
region->update_bot_at(addr, size);
}

inline void G1PLABAllocator::update_bot_for_allocation(G1HeapRegionAttr dest, size_t word_sz, uint node_index) {
assert(dest.needs_bot_update(), "Wrong destination: %s", dest.get_type_str());
G1BotUpdatingPLAB* plab = static_cast<G1BotUpdatingPLAB*>(alloc_buffer(dest, node_index));
plab->update_bot(word_sz);
}

inline void G1BotUpdatingPLAB::set_buf(HeapWord* buf, size_t word_sz) {
PLAB::set_buf(buf, word_sz);
// Update the region and threshold to allow efficient BOT updates.
_region = G1CollectedHeap::heap()->heap_region_containing(buf);
_next_bot_threshold = _region->bot_threshold_for_addr(buf);
}

inline void G1BotUpdatingPLAB::update_bot(size_t word_sz) {
// The last object end is at _top, if it did not cross the
// threshold, there is nothing to do.
if (_top <= _next_bot_threshold) {
return;
}

HeapWord* obj_start = _top - word_sz;
assert(contains(obj_start),
"Object start outside PLAB. bottom: " PTR_FORMAT " object: " PTR_FORMAT,
p2i(_bottom), p2i(obj_start));
assert(obj_start <= _next_bot_threshold,
"Object start not below or at threshold. threshold: " PTR_FORMAT " object: " PTR_FORMAT,
p2i(_next_bot_threshold), p2i(obj_start));

_region->update_bot_crossing_threshold(&_next_bot_threshold, obj_start, _top);
}

#endif // SHARE_GC_G1_G1ALLOCATOR_INLINE_HPP
@@ -108,6 +108,7 @@ class G1BlockOffsetTable: public CHeapObj<mtGC> {

class G1BlockOffsetTablePart {
friend class G1BlockOffsetTable;
friend class HeapRegion;
friend class VMStructs;
private:
// allocation boundary at which offset array must be updated
@@ -181,6 +182,9 @@ class G1BlockOffsetTablePart {

void verify() const;

// Given an address calculate where the next threshold needing an update is.
inline HeapWord* threshold_for_addr(const void* addr);

// Returns the address of the start of the block containing "addr", or
// else "null" if it is covered by no block. (May have side effects,
// namely updating of shared array entries that "point" too far
@@ -204,6 +208,11 @@ class G1BlockOffsetTablePart {
// updated.
HeapWord* threshold() const { return _next_offset_threshold; }

// Sets the threshold explicitly to keep it consistent with what has been
// updated. This needs to be done when the threshold is not used for updating
// the bot, for example when promoting to old in young collections.
void set_threshold(HeapWord* threshold) { _next_offset_threshold = threshold; }

// These must be guaranteed to work properly (i.e., do nothing)
// when "blk_start" ("blk" for second version) is "NULL". In this
// implementation, that's true because NULL is represented as 0, and thus
@@ -31,6 +31,20 @@
#include "gc/shared/memset_with_concurrent_readers.hpp"
#include "runtime/atomic.hpp"

inline HeapWord* G1BlockOffsetTablePart::threshold_for_addr(const void* addr) {
assert(addr >= _hr->bottom() && addr < _hr->top(), "invalid address");
size_t index = _bot->index_for(addr);
HeapWord* card_boundary = _bot->address_for_index(index);
// Address at card boundary, use as threshold.
if (card_boundary == addr) {
return card_boundary;
}

// Calculate next threshold.
HeapWord* threshold = card_boundary + BOTConstants::N_words;
return threshold;
}

inline HeapWord* G1BlockOffsetTablePart::block_start(const void* addr) {
assert(addr >= _hr->bottom() && addr < _hr->top(), "invalid address");
HeapWord* q = block_at_or_preceding(addr);
@@ -109,9 +123,6 @@ inline HeapWord* G1BlockOffsetTablePart::block_at_or_preceding(const void* addr)
"Object crossed region boundary, found offset %u instead of 0",
(uint) _bot->offset_array(_bot->index_for(_hr->bottom())));

// We must make sure that the offset table entry we use is valid.
assert(addr < _next_offset_threshold, "Precondition");

size_t index = _bot->index_for(addr);

HeapWord* q = _bot->address_for_index(index);
@@ -3293,6 +3293,7 @@ void G1CollectedHeap::retire_gc_alloc_region(HeapRegion* alloc_region,
_bytes_used_during_gc += allocated_bytes;
if (dest.is_old()) {
old_set_add(alloc_region);
alloc_region->update_bot_threshold();
} else {
assert(dest.is_young(), "Retiring alloc region should be young (%d)", dest.type());
_survivor.add_used_bytes(allocated_bytes);
@@ -3445,3 +3446,8 @@ GrowableArray<GCMemoryManager*> G1CollectedHeap::memory_managers() {
GrowableArray<MemoryPool*> G1CollectedHeap::memory_pools() {
return _monitoring_support->memory_pools();
}

void G1CollectedHeap::fill_with_dummy_object(HeapWord* start, HeapWord* end, bool zap) {
HeapRegion* region = heap_region_containing(start);
region->fill_with_dummy_object(start, pointer_delta(end, start), zap);
}
@@ -934,6 +934,8 @@ class G1CollectedHeap : public CollectedHeap {
virtual GrowableArray<GCMemoryManager*> memory_managers();
virtual GrowableArray<MemoryPool*> memory_pools();

virtual void fill_with_dummy_object(HeapWord* start, HeapWord* end, bool zap);

// Try to minimize the remembered set.
void scrub_rem_set();

@@ -103,6 +103,7 @@ struct G1HeapRegionAttr {
bool is_young() const { return type() == Young; }
bool is_old() const { return type() == Old; }
bool is_optional() const { return type() == Optional; }
bool needs_bot_update() const { return is_old(); }
Copy link
Contributor

@tschatzl tschatzl Nov 8, 2021

Choose a reason for hiding this comment

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

Not sure if that predicate needs to be here, I'd probably just add a method to G1PLABAllocator. But it is fine to me.


#ifdef ASSERT
bool is_default() const { return type() == NotInCSet; }
@@ -498,6 +498,9 @@ oop G1ParScanThreadState::do_copy_to_survivor_space(G1HeapRegionAttr const regio
obj->incr_age();
}
_age_table.add(age, word_sz);
} else {
// Currently we only have two destinations we only skip BOT updates for young.
_plab_allocator->update_bot_for_allocation(dest_attr, word_sz, node_index);
}

// Most objects are not arrays, so do one array check rather than
@@ -233,7 +233,6 @@ HeapRegion::HeapRegion(uint hrm_index,
_top(NULL),
_compaction_top(NULL),
_bot_part(bot, this),
_par_alloc_lock(Mutex::service-2, "HeapRegionParAlloc_lock"),
_pre_dummy_top(NULL),
_rem_set(NULL),
_hrm_index(hrm_index),
@@ -815,3 +814,12 @@ void HeapRegion::object_iterate(ObjectClosure* blk) {
p += block_size(p);
}
}

void HeapRegion::fill_with_dummy_object(HeapWord* address, size_t word_size, bool zap) {
// Keep the BOT in sync for old generation regions.
if (is_old()) {
update_bot_at(address, word_size);
}
// Fill in the object.
CollectedHeap::fill_with_object(address, word_size, zap);
}
@@ -77,7 +77,7 @@ class HeapRegion : public CHeapObj<mtGC> {
HeapWord* _compaction_top;

G1BlockOffsetTablePart _bot_part;
Mutex _par_alloc_lock;

// When we need to retire an allocation region, while other threads
// are also concurrently trying to allocate into it, we typically
// allocate a dummy object at the end of the region to ensure that
@@ -152,18 +152,24 @@ class HeapRegion : public CHeapObj<mtGC> {

void object_iterate(ObjectClosure* blk);

// Allocation (return NULL if full). Assumes the caller has established
// mutually exclusive access to the HeapRegion.
HeapWord* allocate(size_t min_word_size, size_t desired_word_size, size_t* actual_word_size);
// Allocation (return NULL if full). Enforces mutual exclusion internally.
HeapWord* par_allocate(size_t min_word_size, size_t desired_word_size, size_t* actual_word_size);

HeapWord* allocate(size_t word_size);
HeapWord* par_allocate(size_t word_size);

inline HeapWord* par_allocate_no_bot_updates(size_t min_word_size, size_t desired_word_size, size_t* word_size);
inline HeapWord* allocate_no_bot_updates(size_t word_size);
inline HeapWord* allocate_no_bot_updates(size_t min_word_size, size_t desired_word_size, size_t* actual_size);
// At the given address create an object with the given size. If the region
// is old the BOT will be updated if the object spans a threshold.
void fill_with_dummy_object(HeapWord* address, size_t word_size, bool zap = true);

// All allocations are done without updating the BOT. The BOT
// needs to be kept in sync for old generation regions and
// this is done by explicit updates when crossing thresholds.
inline HeapWord* par_allocate(size_t min_word_size, size_t desired_word_size, size_t* word_size);
inline HeapWord* allocate(size_t word_size);
inline HeapWord* allocate(size_t min_word_size, size_t desired_word_size, size_t* actual_size);

// Update the BOT for the given address if it crosses the next
// BOT threshold at or after obj_start.
inline void update_bot_at(HeapWord* obj_start, size_t obj_size);
// Update BOT at the given threshold for the given object. The
// given object must cross the threshold.
inline void update_bot_crossing_threshold(HeapWord** threshold, HeapWord* obj_start, HeapWord* obj_end);
inline HeapWord* bot_threshold_for_addr(const void* addr);

// Full GC support methods.

@@ -200,6 +206,10 @@ class HeapRegion : public CHeapObj<mtGC> {
_bot_part.update();
}

void update_bot_threshold() {
_bot_part.set_threshold(top());
}

private:
// The remembered set for this region.
HeapRegionRemSet* _rem_set;