Skip to content
Permalink
Browse files

8235366: ZGC: Kitchensink.java fails in ZBarrier::should_mark_through

Reviewed-by: eosterlund, stefank
  • Loading branch information
pliden committed Dec 10, 2019
1 parent 0aaaed9 commit c04194036b8b8a3344940d4cc1e990c49f382273
@@ -46,6 +46,7 @@ class ZAddress : public AllStatic {
static bool is_weak_good(uintptr_t value);
static bool is_weak_good_or_null(uintptr_t value);
static bool is_marked(uintptr_t value);
static bool is_marked_or_null(uintptr_t value);
static bool is_finalizable(uintptr_t value);
static bool is_finalizable_good(uintptr_t value);
static bool is_remapped(uintptr_t value);
@@ -70,6 +70,10 @@ inline bool ZAddress::is_marked(uintptr_t value) {
return value & ZAddressMetadataMarked;
}

inline bool ZAddress::is_marked_or_null(uintptr_t value) {
return is_marked(value) || is_null(value);
}

inline bool ZAddress::is_finalizable(uintptr_t value) {
return value & ZAddressMetadataFinalizable;
}
@@ -84,6 +84,17 @@ uintptr_t ZBarrier::mark(uintptr_t addr) {
ZHeap::heap()->mark_object<follow, finalizable, publish>(good_addr);
}

if (finalizable) {
// Make the oop finalizable marked/good, instead of normal marked/good.
// This is needed because an object might first becomes finalizable
// marked by the GC, and then loaded by a mutator thread. In this case,
// the mutator thread must be able to tell that the object needs to be
// strongly marked. The finalizable bit in the oop exists to make sure
// that a load of a finalizable marked oop will fall into the barrier
// slow path so that we can mark the object as strongly reachable.
return ZAddress::finalizable_good(good_addr);
}

return good_addr;
}

@@ -166,25 +177,17 @@ uintptr_t ZBarrier::keep_alive_barrier_on_phantom_oop_slow_path(uintptr_t addr)
// Mark barrier
//
uintptr_t ZBarrier::mark_barrier_on_oop_slow_path(uintptr_t addr) {
assert(during_mark(), "Invalid phase");

// Mark
return mark<Follow, Strong, Overflow>(addr);
}

uintptr_t ZBarrier::mark_barrier_on_finalizable_oop_slow_path(uintptr_t addr) {
const uintptr_t good_addr = mark<Follow, Finalizable, Overflow>(addr);
if (ZAddress::is_good(addr)) {
// If the oop was already strongly marked/good, then we do
// not want to downgrade it to finalizable marked/good.
return good_addr;
}
assert(during_mark(), "Invalid phase");

// Make the oop finalizable marked/good, instead of normal marked/good.
// This is needed because an object might first becomes finalizable
// marked by the GC, and then loaded by a mutator thread. In this case,
// the mutator thread must be able to tell that the object needs to be
// strongly marked. The finalizable bit in the oop exists to make sure
// that a load of a finalizable marked oop will fall into the barrier
// slow path so that we can mark the object as strongly reachable.
return ZAddress::finalizable_good(good_addr);
// Mark
return mark<Follow, Finalizable, Overflow>(addr);
}

uintptr_t ZBarrier::mark_barrier_on_root_oop_slow_path(uintptr_t addr) {
@@ -41,15 +41,15 @@ class ZBarrier : public AllStatic {
static const bool Publish = true;
static const bool Overflow = false;

static void self_heal(volatile oop* p, uintptr_t addr, uintptr_t heal_addr);
template <ZBarrierFastPath fast_path> static void self_heal(volatile oop* p, uintptr_t addr, uintptr_t heal_addr);

template <ZBarrierFastPath fast_path, ZBarrierSlowPath slow_path> static oop barrier(volatile oop* p, oop o);
template <ZBarrierFastPath fast_path, ZBarrierSlowPath slow_path> static oop weak_barrier(volatile oop* p, oop o);
template <ZBarrierFastPath fast_path, ZBarrierSlowPath slow_path> static void root_barrier(oop* p, oop o);

static bool is_null_fast_path(uintptr_t addr);
static bool is_good_or_null_fast_path(uintptr_t addr);
static bool is_weak_good_or_null_fast_path(uintptr_t addr);
static bool is_marked_or_null_fast_path(uintptr_t addr);

static bool during_mark();
static bool during_relocate();
@@ -32,6 +32,77 @@
#include "oops/oop.hpp"
#include "runtime/atomic.hpp"

// A self heal must always "upgrade" the address metadata bits in
// accordance with the metadata bits state machine, which has the
// valid state transitions as described below (where N is the GC
// cycle).
//
// Note the subtleness of overlapping GC cycles. Specifically that
// oops are colored Remapped(N) starting at relocation N and ending
// at marking N + 1.
//
// +--- Mark Start
// | +--- Mark End
// | | +--- Relocate Start
// | | | +--- Relocate End
// | | | |
// Marked |---N---|--N+1--|--N+2--|----
// Finalizable |---N---|--N+1--|--N+2--|----
// Remapped ----|---N---|--N+1--|--N+2--|
//
// VALID STATE TRANSITIONS
//
// Marked(N) -> Remapped(N)
// -> Marked(N + 1)
// -> Finalizable(N + 1)
//
// Finalizable(N) -> Marked(N)
// -> Remapped(N)
// -> Marked(N + 1)
// -> Finalizable(N + 1)
//
// Remapped(N) -> Marked(N + 1)
// -> Finalizable(N + 1)
//
// PHASE VIEW
//
// ZPhaseMark
// Load & Mark
// Marked(N) <- Marked(N - 1)
// <- Finalizable(N - 1)
// <- Remapped(N - 1)
// <- Finalizable(N)
//
// Mark(Finalizable)
// Finalizable(N) <- Marked(N - 1)
// <- Finalizable(N - 1)
// <- Remapped(N - 1)
//
// Load(AS_NO_KEEPALIVE)
// Remapped(N - 1) <- Marked(N - 1)
// <- Finalizable(N - 1)
//
// ZPhaseMarkCompleted (Resurrection blocked)
// Load & Load(ON_WEAK/PHANTOM_OOP_REF | AS_NO_KEEPALIVE) & KeepAlive
// Marked(N) <- Marked(N - 1)
// <- Finalizable(N - 1)
// <- Remapped(N - 1)
// <- Finalizable(N)
//
// Load(ON_STRONG_OOP_REF | AS_NO_KEEPALIVE)
// Remapped(N - 1) <- Marked(N - 1)
// <- Finalizable(N - 1)
//
// ZPhaseMarkCompleted (Resurrection unblocked)
// Load
// Marked(N) <- Finalizable(N)
//
// ZPhaseRelocate
// Load & Load(AS_NO_KEEPALIVE)
// Remapped(N) <- Marked(N)
// <- Finalizable(N)

template <ZBarrierFastPath fast_path>
inline void ZBarrier::self_heal(volatile oop* p, uintptr_t addr, uintptr_t heal_addr) {
if (heal_addr == 0) {
// Never heal with null since it interacts badly with reference processing.
@@ -41,36 +112,33 @@ inline void ZBarrier::self_heal(volatile oop* p, uintptr_t addr, uintptr_t heal_
return;
}

for (;;) {
if (addr == heal_addr) {
// Already healed
return;
}
assert(!fast_path(addr), "Invalid self heal");
assert(fast_path(heal_addr), "Invalid self heal");

for (;;) {
// Heal
const uintptr_t prev_addr = Atomic::cmpxchg((volatile uintptr_t*)p, addr, heal_addr);
if (prev_addr == addr) {
// Success
return;
}

if (ZAddress::is_good_or_null(prev_addr)) {
// No need to heal
if (fast_path(prev_addr)) {
// Must not self heal
return;
}

// The oop location was healed by another barrier, but it is still not
// good or null. Re-apply healing to make sure the oop is not left with
// weaker (remapped or finalizable) metadata bits than what this barrier
// tried to apply.
// The oop location was healed by another barrier, but still needs upgrading.
// Re-apply healing to make sure the oop is not left with weaker (remapped or
// finalizable) metadata bits than what this barrier tried to apply.
assert(ZAddress::offset(prev_addr) == ZAddress::offset(heal_addr), "Invalid offset");
addr = prev_addr;
}
}

template <ZBarrierFastPath fast_path, ZBarrierSlowPath slow_path>
inline oop ZBarrier::barrier(volatile oop* p, oop o) {
uintptr_t addr = ZOop::to_address(o);
const uintptr_t addr = ZOop::to_address(o);

// Fast path
if (fast_path(addr)) {
@@ -81,7 +149,7 @@ inline oop ZBarrier::barrier(volatile oop* p, oop o) {
const uintptr_t good_addr = slow_path(addr);

if (p != NULL) {
self_heal(p, addr, good_addr);
self_heal<fast_path>(p, addr, good_addr);
}

return ZOop::from_address(good_addr);
@@ -104,7 +172,7 @@ inline oop ZBarrier::weak_barrier(volatile oop* p, oop o) {
if (p != NULL) {
// The slow path returns a good/marked address or null, but we never mark
// oops in a weak load barrier so we always heal with the remapped address.
self_heal(p, addr, ZAddress::remapped_or_null(good_addr));
self_heal<fast_path>(p, addr, ZAddress::remapped_or_null(good_addr));
}

return ZOop::from_address(good_addr);
@@ -132,10 +200,6 @@ inline void ZBarrier::root_barrier(oop* p, oop o) {
*p = ZOop::from_address(good_addr);
}

inline bool ZBarrier::is_null_fast_path(uintptr_t addr) {
return ZAddress::is_null(addr);
}

inline bool ZBarrier::is_good_or_null_fast_path(uintptr_t addr) {
return ZAddress::is_good_or_null(addr);
}
@@ -144,6 +208,10 @@ inline bool ZBarrier::is_weak_good_or_null_fast_path(uintptr_t addr) {
return ZAddress::is_weak_good_or_null(addr);
}

inline bool ZBarrier::is_marked_or_null_fast_path(uintptr_t addr) {
return ZAddress::is_marked_or_null(addr);
}

inline bool ZBarrier::during_mark() {
return ZGlobalPhase == ZPhaseMark;
}
@@ -300,23 +368,31 @@ inline void ZBarrier::keep_alive_barrier_on_phantom_root_oop_field(oop* p) {
}

inline void ZBarrier::keep_alive_barrier_on_oop(oop o) {
const uintptr_t addr = ZOop::to_address(o);
assert(ZAddress::is_good(addr), "Invalid address");

if (during_mark()) {
barrier<is_null_fast_path, mark_barrier_on_oop_slow_path>(NULL, o);
mark_barrier_on_oop_slow_path(addr);
}
}

//
// Mark barrier
//
inline void ZBarrier::mark_barrier_on_oop_field(volatile oop* p, bool finalizable) {
// The fast path only checks for null since the GC worker
// threads doing marking wants to mark through good oops.
const oop o = *p;

if (finalizable) {
barrier<is_null_fast_path, mark_barrier_on_finalizable_oop_slow_path>(p, o);
barrier<is_marked_or_null_fast_path, mark_barrier_on_finalizable_oop_slow_path>(p, o);
} else {
barrier<is_null_fast_path, mark_barrier_on_oop_slow_path>(p, o);
const uintptr_t addr = ZOop::to_address(o);
if (ZAddress::is_good(addr)) {
// Mark through good oop
mark_barrier_on_oop_slow_path(addr);
} else {
// Mark through bad oop
barrier<is_good_or_null_fast_path, mark_barrier_on_oop_slow_path>(p, o);
}
}
}

0 comments on commit c041940

Please sign in to comment.
You can’t perform that action at this time.