RFR: 8353115: GenShen: mixed evacuation candidate regions need accurate live_data [v13]
William Kemper
wkemper at openjdk.org
Mon Nov 10 21:57:11 UTC 2025
On Mon, 10 Nov 2025 14:39:09 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, where the amount of live data determines whether a region should be added to the collection set during the 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.
>
> Kelvin Nilsen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 58 commits:
>
> - Fix mistaken merge resolution
> - Merge remote-tracking branch 'jdk/master' into fix-live-data-for-mixed-evac-candidates
>
> The resulting fastdebug build has 64 failures. I need to debug these.
> Probably introduced by improper resolution of merge conflicts
> - fix error in merge conflict resolution
> - Merge remote-tracking branch 'jdk/master' into fix-live-data-for-mixed-evac-candidates
> - rework CompressedClassSpaceSizeinJmapHeap.java
> - fix errors in CompressedClassSpaceSizeInJmapHeap.java
> - Add debug instrumentation to CompressedClassSpaceSizeInJmapHeap.java
> - fix two indexing bugs
> - add an assert to detect suspected bug
> - Remove debug scaffolding
> - ... and 48 more: https://git.openjdk.org/jdk/compare/c272aca8...16cd6f8a
Changes requested by wkemper (Reviewer).
src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp line 165:
> 163: }
> 164:
> 165: inline size_t ShenandoahHeapRegion::get_live_data_words(ShenandoahMarkingContext* ctx, size_t index) const {
Why do we want to change this signature? If `index` is always `this->_index` why go through the trouble to pass a member field to a member function on the same instance? I have a similar sentiment about passing `ShenandoahMarkingContext` through the function. Should we have a member `ShenandoahMarkingContext* _marking_context`? Changing this signature creates a lot of noise on the PR and it's not clear to me why we would do this.
-------------
PR Review: https://git.openjdk.org/jdk/pull/24319#pullrequestreview-3445303571
PR Review Comment: https://git.openjdk.org/jdk/pull/24319#discussion_r2512084103
More information about the hotspot-gc-dev
mailing list