RFR: 8353115: GenShen: mixed evacuation candidate regions need accurate live_data

[William Kemper]

On Mon, 31 Mar 2025 03:17:51 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

> The existing implementation of get_live_data_bytes() and git_live_data_words() does not always behave as might be expected.  In particular, the value returned ignores any allocations that occur subsequent to the most recent mark effort that identified live data within the region.  This is typically ok for young regions that are going to be added or not to the collection set during final-mark safepoint.
> 
> However, old-gen regions that are placed into the set of candidates for mixed evacuation are more complicated.  In particular, by the time the old-gen region is added to a mixed evacuation, its live data may be much larger than at the time concurrent old marking ended.
> 
> This PR provides comments to clarify the shortcomings of the existing functions, and adds new functions that provide more accurate accountings of live data for mixed-evacuation candidate regions.

Changes requested by wkemper (Reviewer).

src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp line 78:

> 76:   _live_data(0),
> 77:   _critical_pins(0),
> 78:   _mixed_candidate_garbage_words(0),

Do we need a new field to track this? During `final_mark`, we call `increase_live_data_alloc_words` to add `TAMS + top` to `_live_data` to account for objects allocated during mark. Could we "fix" `get_live_data` so that it always returned marked objects (counted by `increase_live_data_gc_words`) _plus_ `top - TAMS`. This way, the live data would not become stale after `final_mark` and we wouldn't have another field to manage. What do you think?

src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp line 159:

> 157: 
> 158: inline size_t ShenandoahHeapRegion::get_mixed_candidate_live_data_bytes() const {
> 159:   assert(SafepointSynchronize::is_at_safepoint(), "Should be at Shenandoah safepoint");

Could we use `shenandoah_assert_safepoint` here (and other places) instead?

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

PR Review: https://git.openjdk.org/jdk/pull/24319#pullrequestreview-2733584314
PR Review Comment: https://git.openjdk.org/jdk/pull/24319#discussion_r2023461623
PR Review Comment: https://git.openjdk.org/jdk/pull/24319#discussion_r2023396124