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

8261495: Shenandoah: reconsider update references memory ordering #2498

Closed
Changes from all 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
@@ -70,7 +70,7 @@ inline oop ShenandoahBarrierSet::load_reference_barrier_mutator(oop obj, T* load

if (load_addr != NULL && fwd != obj) {
// Since we are here and we know the load address, update the reference.
ShenandoahHeap::cas_oop(fwd, load_addr, obj);
ShenandoahHeap::atomic_update_oop(fwd, load_addr, obj);
}

return fwd;
@@ -130,7 +130,7 @@ inline oop ShenandoahBarrierSet::load_reference_barrier(oop obj, T* load_addr) {
oop fwd = load_reference_barrier(obj);
if (ShenandoahSelfFixing && load_addr != NULL && fwd != obj) {
// Since we are here and we know the load address, update the reference.
ShenandoahHeap::cas_oop(fwd, load_addr, obj);
ShenandoahHeap::atomic_update_oop(fwd, load_addr, obj);
}

return fwd;
@@ -349,7 +349,7 @@ void ShenandoahBarrierSet::arraycopy_work(T* src, size_t count) {
fwd = _heap->evacuate_object(obj, thread);
}
assert(obj != fwd || _heap->cancelled_gc(), "must be forwarded");
oop witness = ShenandoahHeap::cas_oop(fwd, elem_ptr, o);
ShenandoahHeap::atomic_update_oop(fwd, elem_ptr, o);
obj = fwd;
}
if (ENQUEUE && !ctx->is_marked_strong(obj)) {
@@ -54,7 +54,7 @@ class ShenandoahUpdateRefsForOopClosure: public BasicOopIterateClosure {
fwd = _heap->evacuate_object(obj, _thread);
}
assert(obj != fwd || _heap->cancelled_gc(), "must be forwarded");
ShenandoahHeap::cas_oop(fwd, p, o);
ShenandoahHeap::atomic_update_oop(fwd, p, o);
obj = fwd;
}
if (ENQUEUE) {
@@ -161,7 +161,7 @@ void ShenandoahEvacuateUpdateRootsClosure::do_oop_work(T* p, Thread* t) {
if (resolved == obj) {
resolved = _heap->evacuate_object(obj, t);
}
_heap->cas_oop(resolved, p, o);
ShenandoahHeap::atomic_update_oop(resolved, p, o);
}
}
}
@@ -207,7 +207,7 @@ void ShenandoahCleanUpdateWeakOopsClosure<CONCURRENT, IsAlive, KeepAlive>::do_oo
_keep_alive->do_oop(p);
} else {
if (CONCURRENT) {
Atomic::cmpxchg(p, obj, oop());
ShenandoahHeap::atomic_clear_oop(p, obj);
} else {
RawAccess<IS_NOT_NULL>::oop_store(p, oop());
}
@@ -686,13 +686,13 @@ void ShenandoahEvacUpdateCleanupOopStorageRootsClosure::do_oop(oop* p) {
if (!CompressedOops::is_null(obj)) {
if (!_mark_context->is_marked(obj)) {
shenandoah_assert_correct(p, obj);
Atomic::cmpxchg(p, obj, oop(NULL));
ShenandoahHeap::atomic_clear_oop(p, obj);
} else if (_evac_in_progress && _heap->in_collection_set(obj)) {
oop resolved = ShenandoahBarrierSet::resolve_forwarded_not_null(obj);
if (resolved == obj) {
resolved = _heap->evacuate_object(obj, _thread);
}
Atomic::cmpxchg(p, obj, resolved);
ShenandoahHeap::atomic_update_oop(resolved, p, obj);
assert(_heap->cancelled_gc() ||
_mark_context->is_marked(resolved) && !_heap->in_collection_set(resolved),
"Sanity");
@@ -635,9 +635,17 @@ class ShenandoahHeap : public CollectedHeap {
template <class T>
inline void update_with_forwarded(T* p);

static inline oop cas_oop(oop n, narrowOop* addr, oop c);
static inline oop cas_oop(oop n, oop* addr, oop c);
static inline oop cas_oop(oop n, narrowOop* addr, narrowOop c);
static inline void atomic_update_oop(oop update, oop* addr, oop compare);
static inline void atomic_update_oop(oop update, narrowOop* addr, oop compare);
static inline void atomic_update_oop(oop update, narrowOop* addr, narrowOop compare);

static inline bool atomic_update_oop_check(oop update, oop* addr, oop compare);
static inline bool atomic_update_oop_check(oop update, narrowOop* addr, oop compare);
static inline bool atomic_update_oop_check(oop update, narrowOop* addr, narrowOop compare);

static inline void atomic_clear_oop( oop* addr, oop compare);
static inline void atomic_clear_oop(narrowOop* addr, oop compare);
static inline void atomic_clear_oop(narrowOop* addr, narrowOop compare);

void trash_humongous_region_at(ShenandoahHeapRegion *r);

@@ -132,29 +132,110 @@ inline void ShenandoahHeap::conc_update_with_forwarded(T* p) {

// Either we succeed in updating the reference, or something else gets in our way.
// We don't care if that is another concurrent GC update, or another mutator update.
// We only check that non-NULL store still updated with non-forwarded reference.
oop witness = cas_oop(fwd, p, obj);
shenandoah_assert_not_forwarded_except(p, witness, (witness == NULL) || (witness == obj));
atomic_update_oop(fwd, p, obj);
}
}
}

inline oop ShenandoahHeap::cas_oop(oop n, oop* addr, oop c) {
// Atomic updates of heap location. This is only expected to work with updating the same
// logical object with its forwardee. The reason why we need stronger-than-relaxed memory
// ordering has to do with coordination with GC barriers and mutator accesses.
//
// In essence, stronger CAS access is required to maintain the transitive chains that mutator
// accesses build by themselves. To illustrate this point, consider the following example.
//
// Suppose "o" is the object that has a field "x" and the reference to "o" is stored
// to field at "addr", which happens to be Java volatile field. Normally, the accesses to volatile
// field at "addr" would be matched with release/acquire barriers. This changes when GC moves
// the object under mutator feet.
//
// Thread 1 (Java)
// // --- previous access starts here
// ...
// T1.1: store(&o.x, 1, mo_relaxed)
// T1.2: store(&addr, o, mo_release) // volatile store
//
// // --- new access starts here
// // LRB: copy and install the new copy to fwdptr
// T1.3: var copy = copy(o)
// T1.4: cas(&fwd, t, copy, mo_release) // pointer-mediated publication
// <access continues>
//
// Thread 2 (GC updater)
// T2.1: var f = load(&fwd, mo_{consume|acquire}) // pointer-mediated acquisition
// T2.2: cas(&addr, o, f, mo_release) // this method
//
// Thread 3 (Java)
// T3.1: var o = load(&addr, mo_acquire) // volatile read
// T3.2: if (o != null)
// T3.3: var r = load(&o.x, mo_relaxed)
//
// r is guaranteed to contain "1".
//
// Without GC involvement, there is synchronizes-with edge from T1.2 to T3.1,
// which guarantees this. With GC involvement, when LRB copies the object and
// another thread updates the reference to it, we need to have the transitive edge
// from T1.4 to T2.1 (that one is guaranteed by forwarding accesses), plus the edge
// from T2.2 to T3.1 (which is brought by this CAS).
//
// Note that we do not need to "acquire" in these methods, because we do not read the
// failure witnesses contents on any path, and "release" is enough.
//

inline void ShenandoahHeap::atomic_update_oop(oop update, oop* addr, oop compare) {
assert(is_aligned(addr, HeapWordSize), "Address should be aligned: " PTR_FORMAT, p2i(addr));
return (oop) Atomic::cmpxchg(addr, c, n);
Atomic::cmpxchg(addr, compare, update, memory_order_release);
}

inline oop ShenandoahHeap::cas_oop(oop n, narrowOop* addr, narrowOop c) {
inline void ShenandoahHeap::atomic_update_oop(oop update, narrowOop* addr, narrowOop compare) {
assert(is_aligned(addr, sizeof(narrowOop)), "Address should be aligned: " PTR_FORMAT, p2i(addr));
narrowOop val = CompressedOops::encode(n);
return CompressedOops::decode(Atomic::cmpxchg(addr, c, val));
narrowOop u = CompressedOops::encode(update);
Atomic::cmpxchg(addr, compare, u, memory_order_release);
}

inline oop ShenandoahHeap::cas_oop(oop n, narrowOop* addr, oop c) {
inline void ShenandoahHeap::atomic_update_oop(oop update, narrowOop* addr, oop compare) {
assert(is_aligned(addr, sizeof(narrowOop)), "Address should be aligned: " PTR_FORMAT, p2i(addr));
narrowOop cmp = CompressedOops::encode(c);
narrowOop val = CompressedOops::encode(n);
return CompressedOops::decode(Atomic::cmpxchg(addr, cmp, val));
narrowOop c = CompressedOops::encode(compare);
narrowOop u = CompressedOops::encode(update);
Atomic::cmpxchg(addr, c, u, memory_order_release);
}

inline bool ShenandoahHeap::atomic_update_oop_check(oop update, oop* addr, oop compare) {
assert(is_aligned(addr, HeapWordSize), "Address should be aligned: " PTR_FORMAT, p2i(addr));
return (oop) Atomic::cmpxchg(addr, compare, update, memory_order_release) == compare;
}

inline bool ShenandoahHeap::atomic_update_oop_check(oop update, narrowOop* addr, narrowOop compare) {
assert(is_aligned(addr, sizeof(narrowOop)), "Address should be aligned: " PTR_FORMAT, p2i(addr));
narrowOop u = CompressedOops::encode(update);
return (narrowOop) Atomic::cmpxchg(addr, compare, u, memory_order_release) == compare;
}

inline bool ShenandoahHeap::atomic_update_oop_check(oop update, narrowOop* addr, oop compare) {
assert(is_aligned(addr, sizeof(narrowOop)), "Address should be aligned: " PTR_FORMAT, p2i(addr));
narrowOop c = CompressedOops::encode(compare);
narrowOop u = CompressedOops::encode(update);
return CompressedOops::decode(Atomic::cmpxchg(addr, c, u, memory_order_release)) == compare;
}

// The memory ordering discussion above does not apply for methods that store NULLs:
// then, there is no transitive reads in mutator (as we see NULLs), and we can do
// relaxed memory ordering there.

inline void ShenandoahHeap::atomic_clear_oop(oop* addr, oop compare) {
assert(is_aligned(addr, HeapWordSize), "Address should be aligned: " PTR_FORMAT, p2i(addr));
Atomic::cmpxchg(addr, compare, oop(), memory_order_relaxed);
}

inline void ShenandoahHeap::atomic_clear_oop(narrowOop* addr, oop compare) {
assert(is_aligned(addr, sizeof(narrowOop)), "Address should be aligned: " PTR_FORMAT, p2i(addr));
narrowOop cmp = CompressedOops::encode(compare);
Atomic::cmpxchg(addr, cmp, narrowOop(), memory_order_relaxed);
}

inline void ShenandoahHeap::atomic_clear_oop(narrowOop* addr, narrowOop compare) {
assert(is_aligned(addr, sizeof(narrowOop)), "Address should be aligned: " PTR_FORMAT, p2i(addr));
Atomic::cmpxchg(addr, compare, narrowOop(), memory_order_relaxed);
}

inline bool ShenandoahHeap::cancelled_gc() const {
@@ -117,20 +117,9 @@ void reference_set_discovered<narrowOop>(oop reference, oop discovered) {
}

template<typename T>
static bool reference_cas_discovered(oop reference, oop discovered);

template<>
bool reference_cas_discovered<narrowOop>(oop reference, oop discovered) {
volatile narrowOop* addr = reinterpret_cast<volatile narrowOop*>(java_lang_ref_Reference::discovered_addr_raw(reference));
narrowOop compare = CompressedOops::encode(NULL);
narrowOop exchange = CompressedOops::encode(discovered);
return Atomic::cmpxchg(addr, compare, exchange) == compare;
}

template<>
bool reference_cas_discovered<oop>(oop reference, oop discovered) {
volatile oop* addr = reinterpret_cast<volatile oop*>(java_lang_ref_Reference::discovered_addr_raw(reference));
return Atomic::cmpxchg(addr, oop(NULL), discovered) == NULL;
static bool reference_cas_discovered(oop reference, oop discovered) {
T* addr = reinterpret_cast<T *>(java_lang_ref_Reference::discovered_addr_raw(reference));
return ShenandoahHeap::atomic_update_oop_check(discovered, addr, NULL);
}

template <typename T>