Skip to content
Permalink
Browse files

8238979: Improve G1DirtyCardQueueSet handling of previously paused bu…

…ffers

Move enqueuing of previously paused buffers.

Reviewed-by: sangheki, sjohanss
  • Loading branch information
Kim Barrett
Kim Barrett committed Feb 26, 2020
1 parent 6913bbc commit 257de28b2caaf1eea11d32e36ac42c79aa24d762
Showing with 13 additions and 43 deletions.
  1. +13 −38 src/hotspot/share/gc/g1/g1DirtyCardQueue.cpp
  2. +0 −5 src/hotspot/share/gc/g1/g1DirtyCardQueue.hpp
@@ -228,19 +228,17 @@ void G1DirtyCardQueueSet::enqueue_completed_buffer(BufferNode* cbn) {
}

BufferNode* G1DirtyCardQueueSet::get_completed_buffer(size_t stop_at) {
enqueue_previous_paused_buffers();

// Check for insufficient cards to satisfy request. We only do this once,
// up front, rather than on each iteration below, since the test is racy
// regardless of when we do it.
if (Atomic::load_acquire(&_num_cards) <= stop_at) {
if (Atomic::load_acquire(&_num_cards) < stop_at) {
return NULL;
}

BufferNode* result = _completed.pop();
if (result != NULL) {
Atomic::sub(&_num_cards, buffer_size() - result->index());
if (result == NULL) { // Unlikely if no paused buffers.
enqueue_previous_paused_buffers();
result = _completed.pop();
if (result == NULL) return NULL;
}
Atomic::sub(&_num_cards, buffer_size() - result->index());
return result;
}

@@ -298,31 +296,24 @@ G1DirtyCardQueueSet::PausedBuffers::PausedBuffers() : _plist(NULL) {}

#ifdef ASSERT
G1DirtyCardQueueSet::PausedBuffers::~PausedBuffers() {
assert(is_empty(), "invariant");
assert(Atomic::load(&_plist) == NULL, "invariant");
}
#endif // ASSERT

bool G1DirtyCardQueueSet::PausedBuffers::is_empty() const {
return Atomic::load(&_plist) == NULL;
}

void G1DirtyCardQueueSet::PausedBuffers::add(BufferNode* node) {
assert_not_at_safepoint();
PausedList* plist = Atomic::load_acquire(&_plist);
if (plist != NULL) {
// Already have a next list, so use it. We know it's a next list because
// of the precondition that take_previous() has already been called.
assert(plist->is_next(), "invariant");
} else {
if (plist == NULL) {
// Try to install a new next list.
plist = new PausedList();
PausedList* old_plist = Atomic::cmpxchg(&_plist, (PausedList*)NULL, plist);
if (old_plist != NULL) {
// Some other thread installed a new next list. Use it instead.
// Some other thread installed a new next list. Use it instead.
delete plist;
plist = old_plist;
}
}
assert(plist->is_next(), "invariant");
plist->add(node);
}

@@ -366,6 +357,8 @@ G1DirtyCardQueueSet::HeadTail G1DirtyCardQueueSet::PausedBuffers::take_all() {
void G1DirtyCardQueueSet::record_paused_buffer(BufferNode* node) {
assert_not_at_safepoint();
assert(node->next() == NULL, "precondition");
// Ensure there aren't any paused buffers from a previous safepoint.
enqueue_previous_paused_buffers();
// Cards for paused buffers are included in count, to contribute to
// notification checking after the coming safepoint if it doesn't GC.
// Note that this means the queue's _num_cards differs from the number
@@ -384,25 +377,7 @@ void G1DirtyCardQueueSet::enqueue_paused_buffers_aux(const HeadTail& paused) {

void G1DirtyCardQueueSet::enqueue_previous_paused_buffers() {
assert_not_at_safepoint();
// The fast-path still satisfies the precondition for record_paused_buffer
// and PausedBuffers::add, even with a racy test. If there are paused
// buffers from a previous safepoint, is_empty() will return false; there
// will have been a safepoint between recording and test, so there can't be
// a false negative (is_empty() returns true) while such buffers are present.
// If is_empty() is false, there are two cases:
//
// (1) There were paused buffers from a previous safepoint. A concurrent
// caller may take and enqueue them first, but that's okay; the precondition
// for a possible later record_paused_buffer by this thread will still hold.
//
// (2) There are paused buffers for a requested next safepoint.
//
// In each of those cases some effort may be spent detecting and dealing
// with those circumstances; any wasted effort in such cases is expected to
// be well compensated by the fast path.
if (!_paused.is_empty()) {
enqueue_paused_buffers_aux(_paused.take_previous());
}
enqueue_paused_buffers_aux(_paused.take_previous());
}

void G1DirtyCardQueueSet::enqueue_all_paused_buffers() {
@@ -178,10 +178,6 @@ class G1DirtyCardQueueSet: public PtrQueueSet {
PausedBuffers();
DEBUG_ONLY(~PausedBuffers();)

// Test whether there are any paused lists.
// Thread-safe, but the answer may change immediately.
bool is_empty() const;

// Thread-safe add the buffer to paused list for next safepoint.
// precondition: not at safepoint.
// precondition: does not have paused buffers from a previous safepoint.
@@ -228,7 +224,6 @@ class G1DirtyCardQueueSet: public PtrQueueSet {

// Thread-safe add a buffer to paused list for next safepoint.
// precondition: not at safepoint.
// precondition: does not have paused buffers from a previous safepoint.
void record_paused_buffer(BufferNode* node);
void enqueue_paused_buffers_aux(const HeadTail& paused);
// Thread-safe transfer paused buffers for previous safepoints to the queue.

0 comments on commit 257de28

Please sign in to comment.