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

8260267: vmTestbase/gc/gctests/FinalizeLock/FinalizeLock.java fails with fatal error: Mark stack space exhausted #3262

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/hotspot/share/gc/z/zMark.cpp
Original file line number Diff line number Diff line change
@@ -66,6 +66,8 @@ static const ZStatSubPhase ZSubPhaseConcurrentMarkTryFlush("Concurrent Mark Try
static const ZStatSubPhase ZSubPhaseConcurrentMarkTryTerminate("Concurrent Mark Try Terminate");
static const ZStatSubPhase ZSubPhaseMarkTryComplete("Pause Mark Try Complete");

volatile bool ZMark::_push_local_stripe = false;

ZMark::ZMark(ZWorkers* workers, ZPageTable* page_table) :
_workers(workers),
_page_table(page_table),
@@ -117,6 +119,8 @@ void ZMark::start() {
const size_t nstripes = calculate_nstripes(_nworkers);
_stripes.set_nstripes(nstripes);

_push_local_stripe = false;

// Update statistics
ZStatMark::set_at_mark_start(nstripes);

@@ -157,7 +161,9 @@ bool ZMark::is_array(uintptr_t addr) const {
void ZMark::push_partial_array(uintptr_t addr, size_t size, bool finalizable) {
assert(is_aligned(addr, ZMarkPartialArrayMinSize), "Address misaligned");
ZMarkThreadLocalStacks* const stacks = ZThreadLocalData::stacks(Thread::current());
ZMarkStripe* const stripe = _stripes.stripe_for_addr(addr);
ZMarkStripe* const stripe = push_local_stripe() ?
_stripes.stripe_for_worker(_nworkers, ZThread::worker_id()) :
_stripes.stripe_for_addr(addr);
const uintptr_t offset = ZAddress::offset(addr) >> ZMarkPartialArrayMinSizeShift;
const uintptr_t length = size / oopSize;
const ZMarkStackEntry entry(offset, length, finalizable);
8 changes: 8 additions & 0 deletions src/hotspot/share/gc/z/zMark.hpp
Original file line number Diff line number Diff line change
@@ -53,6 +53,7 @@ class ZMark {
size_t _ntrycomplete;
size_t _ncontinue;
uint _nworkers;
static volatile bool _push_local_stripe;

size_t calculate_nstripes(uint nworkers) const;

@@ -109,6 +110,13 @@ class ZMark {
void mark(bool initial);
bool end();

static void set_push_local_stripe(bool mode) {
Atomic::store(&_push_local_stripe, mode);
}
static bool push_local_stripe() {
return Atomic::load(&_push_local_stripe);
}

void flush_and_free();
bool flush_and_free(Thread* thread);
};
5 changes: 4 additions & 1 deletion src/hotspot/share/gc/z/zMark.inline.hpp
Original file line number Diff line number Diff line change
@@ -28,14 +28,17 @@
#include "gc/z/zMark.hpp"
#include "gc/z/zMarkStack.inline.hpp"
#include "gc/z/zThreadLocalData.hpp"
#include "gc/z/zThread.inline.hpp"
#include "runtime/thread.hpp"
#include "utilities/debug.hpp"

template <bool follow, bool finalizable, bool publish>
inline void ZMark::mark_object(uintptr_t addr) {
assert(ZAddress::is_marked(addr), "Should be marked");
ZMarkThreadLocalStacks* const stacks = ZThreadLocalData::stacks(Thread::current());
ZMarkStripe* const stripe = _stripes.stripe_for_addr(addr);
ZMarkStripe* const stripe = push_local_stripe() && ZThread::is_worker() ?

Choose a reason for hiding this comment

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

This introduces a volatile read in every object mark, do you have concurrent mark time mesaurement with this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The volatile read has impact on the mark time, the average mark time increased by 4.7%

Jdk17 zgc origin:
RUN RESULT: hbIR (max attempted) = 124943, hbIR (settled) = 121646, max-jOPS = 111199, critical-jOPS = 91446
RUN RESULT: hbIR (max attempted) = 122581, hbIR (settled) = 118652, max-jOPS = 111549, critical-jOPS = 92803
RUN RESULT: hbIR (max attempted) = 130653, hbIR (settled) = 119162, max-jOPS = 111055, critical-jOPS = 95202
[2021-03-26T16:49:41.474+0800][info ][gc,stats ] Phase: Concurrent Mark 0.000 / 0.000 1142.885 / 1689.873 800.652 / 3473.744 800.652 / 3473.744 ms
[2021-03-26T19:03:55.173+0800][info ][gc,stats ] Phase: Concurrent Mark 0.000 / 0.000 1204.003 / 1975.038 843.825 / 3006.138 843.825 / 3006.138 ms
[2021-03-26T21:14:13.816+0800][info ][gc,stats ] Phase: Concurrent Mark 0.000 / 0.000 1147.566 / 2477.271 827.487 / 2982.508 827.487 / 2982.508 ms

Jdk17 zgc patch:
RUN RESULT: hbIR (max attempted) = 130653, hbIR (settled) = 116479, max-jOPS = 111055, critical-jOPS = 94165
RUN RESULT: hbIR (max attempted) = 130653, hbIR (settled) = 116752, max-jOPS = 109749, critical-jOPS = 97445
RUN RESULT: hbIR (max attempted) = 130653, hbIR (settled) = 118685, max-jOPS = 111055, critical-jOPS = 93540
[2021-03-30T20:53:35.593+0800][info ][gc,stats ] Phase: Concurrent Mark 0.000 / 0.000 1198.332 / 3098.680 860.451 / 3198.799 860.451 / 3198.799 ms
[2021-03-30T23:07:36.532+0800][info ][gc,stats ] Phase: Concurrent Mark 0.000 / 0.000 1155.819 / 1550.257 842.028 / 3189.933 842.028 / 3189.933 ms
[2021-03-31T01:27:26.091+0800][info ][gc,stats ] Phase: Concurrent Mark 0.000 / 0.000 1211.280 / 1964.112 883.528 / 3053.926 883.528 / 3053.926 ms

_stripes.stripe_for_worker(_nworkers, ZThread::worker_id()) :
_stripes.stripe_for_addr(addr);
ZMarkStackEntry entry(addr, follow, finalizable);

stacks->push(&_allocator, &_stripes, stripe, entry, publish);
37 changes: 36 additions & 1 deletion src/hotspot/share/gc/z/zMarkStackAllocator.cpp
Original file line number Diff line number Diff line change
@@ -27,6 +27,7 @@
#include "gc/z/zLock.inline.hpp"
#include "gc/z/zMarkStack.inline.hpp"
#include "gc/z/zMarkStackAllocator.hpp"
#include "gc/z/zMark.hpp"
#include "logging/log.hpp"
#include "runtime/atomic.hpp"
#include "runtime/os.hpp"
@@ -128,7 +129,8 @@ uintptr_t ZMarkStackSpace::alloc(size_t size) {

ZMarkStackAllocator::ZMarkStackAllocator() :
_freelist(),
_space() {
_space(),
_nmagazine(0) {
// Prime free list to avoid an immediate space
// expansion when marking starts.
if (_space.is_initialized()) {
@@ -159,13 +161,45 @@ ZMarkStackMagazine* ZMarkStackAllocator::create_magazine_from_space(uintptr_t ad
assert(success, "Magazine should never get full");
}

inc_nmagazine();

return magazine;
}

void ZMarkStackAllocator::inc_nmagazine() {
int cur = Atomic::add(&_nmagazine, 1);
if (!ZMark::push_local_stripe() &&
cur * ZMarkStackMagazineSize >= ZMarkStackSpaceLimit * ZStackModeChangeRatio) {
ZMark::set_push_local_stripe(true);
}
}

void ZMarkStackAllocator::dec_nmagazine() {
int nmagazine = Atomic::load(&_nmagazine);

for (;;) {
if (nmagazine == 0) {

Choose a reason for hiding this comment

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

It's confusing here _nmagazine can be negative after dec. might add some comment on this method.

break;
}
const int new_nmagazine = nmagazine - 1;
const int prev_nmagazine = Atomic::cmpxchg(&_nmagazine, nmagazine, new_nmagazine);
if (prev_nmagazine == nmagazine) {
break;
}
nmagazine = prev_nmagazine;
}

if (ZMark::push_local_stripe() &&
nmagazine * ZMarkStackMagazineSize * 2.0 < ZMarkStackSpaceLimit * ZStackModeChangeRatio) {
ZMark::set_push_local_stripe(false);
}
}

ZMarkStackMagazine* ZMarkStackAllocator::alloc_magazine() {
// Try allocating from the free list first
ZMarkStackMagazine* const magazine = _freelist.pop();
if (magazine != NULL) {
inc_nmagazine();
return magazine;
}

@@ -180,4 +214,5 @@ ZMarkStackMagazine* ZMarkStackAllocator::alloc_magazine() {

void ZMarkStackAllocator::free_magazine(ZMarkStackMagazine* magazine) {
_freelist.push(magazine);
dec_nmagazine();
}
3 changes: 3 additions & 0 deletions src/hotspot/share/gc/z/zMarkStackAllocator.hpp
Original file line number Diff line number Diff line change
@@ -52,8 +52,11 @@ class ZMarkStackAllocator {
private:
ZCACHE_ALIGNED ZMarkStackMagazineList _freelist;
ZCACHE_ALIGNED ZMarkStackSpace _space;
ZCACHE_ALIGNED volatile int _nmagazine;

void prime_freelist();
void inc_nmagazine();
void dec_nmagazine();
ZMarkStackMagazine* create_magazine_from_space(uintptr_t addr, size_t size);

public:
3 changes: 3 additions & 0 deletions src/hotspot/share/gc/z/z_globals.hpp
Original file line number Diff line number Diff line change
@@ -42,6 +42,9 @@
"Maximum number of bytes allocated for mark stacks") \
range(32*M, 1024*G) \
\
product(double, ZStackModeChangeRatio, 0.25, \
"Mark push mode is changed when usage is above the ratio") \
\
product(double, ZCollectionInterval, 0, \
"Force GC at a fixed time interval (in seconds)") \
\