Skip to content
Open
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class ShenandoahBarrierSet: public BarrierSet {

private:
template <class T>
inline void arraycopy_marking(T* src, T* dst, size_t count, bool is_old_marking);
inline void arraycopy_marking(T* dst, size_t count);
template <class T>
inline void arraycopy_evacuation(T* src, size_t count);
template <class T>
Expand Down
75 changes: 17 additions & 58 deletions src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -387,13 +387,11 @@ template <class T, bool HAS_FWD, bool EVAC, bool ENQUEUE>
void ShenandoahBarrierSet::arraycopy_work(T* src, size_t count) {
// Young cycles are allowed to run when old marking is in progress. When old marking is in progress,
// this barrier will be called with ENQUEUE=true and HAS_FWD=false, even though the young generation
// may have forwarded objects. In this case, the `arraycopy_work` is first called with HAS_FWD=true and
// ENQUEUE=false.
assert(HAS_FWD == _heap->has_forwarded_objects() || _heap->is_concurrent_old_mark_in_progress(),
"Forwarded object status is sane");
// may have forwarded objects.
assert(HAS_FWD == _heap->has_forwarded_objects() || _heap->is_concurrent_old_mark_in_progress(), "Forwarded object status is sane");
// This function cannot be called to handle marking and evacuation at the same time (they operate on
// different sides of the copy).
assert((HAS_FWD || EVAC) != ENQUEUE, "Cannot evacuate and mark both sides of copy.");
static_assert((HAS_FWD || EVAC) != ENQUEUE, "Cannot evacuate and mark both sides of copy.");

Thread* thread = Thread::current();
SATBMarkQueue& queue = ShenandoahThreadLocalData::satb_mark_queue(thread);
Expand All @@ -412,7 +410,7 @@ void ShenandoahBarrierSet::arraycopy_work(T* src, size_t count) {
shenandoah_assert_forwarded_except(elem_ptr, obj, _heap->cancelled_gc());
ShenandoahHeap::atomic_update_oop(fwd, elem_ptr, o);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the comment at the start of arraycopy_work() still relevant? The description of the PR suggests that we will no longer call arraycopy_work() twice, but I'm not sure I fully understand the context of that statement.

Copy link
Contributor Author

@earthling-amzn earthling-amzn Nov 10, 2025

Choose a reason for hiding this comment

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

We used to call this twice (as arraycopy_marking for each object if young and old marking were both in progress). The comment at the start is trying to explain how the arraycopy_work could be called with different template parameters depending on the gc_state. It is confusingly worded, but I believe the comment is correct and the code is wrong here. The way the code is in the PR, it would effectively turn off the old SATB during young evacuation and young update-refs, which is not what we want. Good catch!

}
if (ENQUEUE && !ctx->is_marked_strong_or_old(obj)) {
if (ENQUEUE && !ctx->is_marked_strong(obj)) {
_satb_mark_queue_set.enqueue_known_active(queue, obj);
}
}
Expand All @@ -426,68 +424,29 @@ void ShenandoahBarrierSet::arraycopy_barrier(T* src, T* dst, size_t count) {
return;
}

char gc_state = ShenandoahThreadLocalData::gc_state(Thread::current());
const char gc_state = ShenandoahThreadLocalData::gc_state(Thread::current());
if ((gc_state & ShenandoahHeap::MARKING) != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

A question on sequencing. Can we have marking and update-refs at the same time? If so, should update-refs happen first, and then marking should be doing the walk over (fixed-up) objects?

// If marking old or young, we must evaluate the SATB barrier. This will be the only
// action if we are not marking old. If we are marking old, we must still evaluate the
// load reference barrier for a young collection.
arraycopy_marking(dst, count);
}

if ((gc_state & ShenandoahHeap::EVACUATION) != 0) {
assert((gc_state & ShenandoahHeap::YOUNG_MARKING) == 0, "Cannot be marking young during evacuation");
arraycopy_evacuation(src, count);
} else if ((gc_state & ShenandoahHeap::UPDATE_REFS) != 0) {
assert((gc_state & ShenandoahHeap::YOUNG_MARKING) == 0, "Cannot be marking young during update-refs");
arraycopy_update(src, count);
}

if (_heap->mode()->is_generational()) {
assert(ShenandoahSATBBarrier, "Generational mode assumes SATB mode");
if ((gc_state & ShenandoahHeap::YOUNG_MARKING) != 0) {
arraycopy_marking(src, dst, count, false);
}
if ((gc_state & ShenandoahHeap::OLD_MARKING) != 0) {
arraycopy_marking(src, dst, count, true);
}
} else if ((gc_state & ShenandoahHeap::MARKING) != 0) {
arraycopy_marking(src, dst, count, false);
}
}

template <class T>
void ShenandoahBarrierSet::arraycopy_marking(T* src, T* dst, size_t count, bool is_old_marking) {
void ShenandoahBarrierSet::arraycopy_marking(T* dst, size_t count) {
assert(_heap->is_concurrent_mark_in_progress(), "only during marking");
/*
* Note that an old-gen object is considered live if it is live at the start of OLD marking or if it is promoted
* following the start of OLD marking.
*
* 1. Every object promoted following the start of OLD marking will be above TAMS within its old-gen region
* 2. Every object live at the start of OLD marking will be referenced from a "root" or it will be referenced from
* another live OLD-gen object. With regards to old-gen, roots include stack locations and all of live young-gen.
* All root references to old-gen are identified during a bootstrap young collection. All references from other
* old-gen objects will be marked during the traversal of all old objects, or will be marked by the SATB barrier.
*
* During old-gen marking (which is interleaved with young-gen collections), call arraycopy_work() if:
*
* 1. The overwritten array resides in old-gen and it is below TAMS within its old-gen region
* 2. Do not call arraycopy_work for any array residing in young-gen because young-gen collection is idle at this time
*
* During young-gen marking, call arraycopy_work() if:
*
* 1. The overwritten array resides in young-gen and is below TAMS within its young-gen region
* 2. Additionally, if array resides in old-gen, regardless of its relationship to TAMS because this old-gen array
* may hold references to young-gen
*/
if (ShenandoahSATBBarrier) {
T* array = dst;
HeapWord* array_addr = reinterpret_cast<HeapWord*>(array);
ShenandoahHeapRegion* r = _heap->heap_region_containing(array_addr);
if (is_old_marking) {
// Generational, old marking
assert(_heap->mode()->is_generational(), "Invariant");
if (r->is_old() && (array_addr < _heap->marking_context()->top_at_mark_start(r))) {
arraycopy_work<T, false, false, true>(array, count);
}
} else if (_heap->mode()->is_generational()) {
// Generational, young marking
if (r->is_old() || (array_addr < _heap->marking_context()->top_at_mark_start(r))) {
arraycopy_work<T, false, false, true>(array, count);
}
} else if (array_addr < _heap->marking_context()->top_at_mark_start(r)) {
// Non-generational, marking
arraycopy_work<T, false, false, true>(array, count);
if (!_heap->marking_context()->allocated_after_mark_start(reinterpret_cast<HeapWord*>(dst))) {
arraycopy_work<T, false, false, true>(dst, count);
}
}
}
Expand Down