RFR: 8370041: GenShen: Filter young pointers from thread local SATB buffers when only marking old [v3]

[Y. Srinivas Ramakrishna]

On Mon, 3 Nov 2025 18:01:31 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> When GenShen is only marking the old generation, we do not need the SATB mechanism to preserve young pointers. We currently filter these out of the SATB buffers during the final-update-refs and init-mark safepoints. This increases latency and introduces no small amount of complexity. It should be possible to instead filter out these pointers when the SATB buffers are 'compacted' before being 'completed'.
>> 
>> # Background
>> When GenShen is marking the old generation it leaves the SATB barrier enabled. When a young collection interrupts old marking, it creates a situation where a mutator thread could overwrite a field holding a pointer into a collection set region. The SATB barrier will dutifully place this object in the SATB queue. If this pointer makes it into a mark queue, the marking thread will crash. Prior to this change, GenShen filtered out such pointers _after_ the thread local SATB buffers were completed. After this change, such pointers are filtered out _before_ the buffers are completed. This is more inline with the natural way of things.
>
> William Kemper has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 21 commits:
> 
>  - Merge remote-tracking branch 'jdk/master' into piggyback-satb-flush-on-update-roots
>  - Merge remote-tracking branch 'jdk/master' into piggyback-satb-flush-on-update-roots
>  - Flush SATB buffers upon entering degenerated cycle when old marking is in progress
>    
>    This has to happen at least once during the degenerated cycle. Doing it at the start, rather than the end, simplifies the verifier.
>  - Fix typo in comment
>  - Remove duplicate satb flush closure
>  - Only flush satb once during degenerated cycle
>  - Cleanup and comments
>  - Merge remote-tracking branch 'jdk/master' into piggyback-satb-flush-on-update-roots
>  - Fix assertion
>  - Oops, move inline definition out of ifdef ASSERT
>  - ... and 11 more: https://git.openjdk.org/jdk/compare/1922c4fd...4bd602de

Changes look ok. I found some of the comments confusing -- I have left some remarks at those places, please have a look to see if they can be made clearer.

The improvement in performance looks good. Do we track the number of SATB pointers processed by the old marking (to compare between before and after your changes here)?

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp line 1143:

> 1141:   //     be in the collection set. If this happens, the pointer will be preserved, essentially
> 1142:   //     becoming part of the old snapshot.
> 1143:   //  2. The region is allocated during evacuation of old. This is also not a concern because

One related question. In both these cases, I assume the reference will look "marked" because it's above TAMS for the purposes of the old marking?

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp line 234:

> 232:   // marking phase. In some cases, this can cause a write to a perfectly
> 233:   // reachable oop to enqueue a pointer that later becomes garbage (because
> 234:   // it points at an object that is later chosen for the collection set). There are

> ```
>   // ... In some cases, this can cause a write to a perfectly
>   // reachable oop to enqueue a pointer that later becomes garbage (because
>   // it points at an object that is later chosen for the collection set).
> ```

I don't understand this statement. The SATB is supposed to be pointers to objects that we will preserve because they were reachable when the snapshot (marking) was started. Can you elaborate what you mean here? Did you mean that the filtering of the SATB didn't filter a (sometime) young reference which was then processed by the old marking?

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp line 237:

> 235:   // also cases where the referent of a weak reference ends up in the SATB
> 236:   // and is later collected. In these cases the oop in the SATB buffer becomes
> 237:   // invalid and the _next_ cycle will crash during its marking phase. To

Again I don't understand the concept of an SATB pointer to an object that was later collected? Are we talking about young objects that are subsequently processed by old marking because they weren't filtered out when they should be?

I think that is probably the case here, but it would be good to clean up these comments to avoid this confusion.

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

Marked as reviewed by ysr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27983#pullrequestreview-3425195561
PR Review Comment: https://git.openjdk.org/jdk/pull/27983#discussion_r2496682448
PR Review Comment: https://git.openjdk.org/jdk/pull/27983#discussion_r2496698952
PR Review Comment: https://git.openjdk.org/jdk/pull/27983#discussion_r2496702500