RFR: 8377713: Shenandoah: Convert ShenandoahReferenceProcessor to use Atomic<T> [v2]

Ben Taylor btaylor at openjdk.org
Thu Feb 26 22:10:32 UTC 2026


On Thu, 26 Feb 2026 21:27:00 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> Ben Taylor has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Change add_then_fetch - 1 to fetch_then_add
>
> src/hotspot/share/gc/shenandoah/shenandoahReferenceProcessor.cpp line 579:
> 
>> 577: void ShenandoahReferenceProcessor::enqueue_references_locked() {
>> 578:   // Prepend internal pending list to external pending list
>> 579:   shenandoah_assert_not_in_cset_except(&_pending_list, _pending_list.load_relaxed(), ShenandoahHeap::heap()->cancelled_gc() || !ShenandoahLoadRefBarrier);
> 
> Not sure about this one. The first argument to `shenandoah_assert_not_in_cset_except` is a `void *`, so it will take just about anything. The argument is meant to be the "interior location" (i.e., address of the field holding the pointer given in the second argument). With this change, we are passing the address of the `Atomic` wrapper class, but I think we should be passing the address of the _value_ it is holding. So maybe `&_pending_list.load_relaxed()`?

The problem with this is `T load_relaxed() const {` will end up giving you the address of a temporary copy of the value when used this way. There's no way to get a direct reference/pointer to the value, as doing so would violate the Atomic encapsulation. So the address of the Atomic itself seemed the most appropriate thing to provide

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/29767#discussion_r2861517852


More information about the shenandoah-dev mailing list