RFR: 8339616: GenShen: Introduce new state to distinguish promote-in-place phase as distinct from concurrent evacuation [v2]
William Kemper
wkemper at openjdk.org
Mon Sep 23 20:54:01 UTC 2024
On Mon, 23 Sep 2024 18:06:21 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>> William Kemper has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Do not enter oom-during-evac protocol when only promoting regions in place
>
> src/hotspot/share/gc/shenandoah/shenandoahGenerationalEvacuationTask.cpp line 82:
>
>> 80: if (_only_promote_regions) {
>> 81: // No allocations will be made, do not enter oom-during-evac protocol.
>> 82: do_evacuations();
>
> It really looks to me that promotion-in-place should be in a separate task? Or the name of the task should reflect it also does promotions? Or at least the "real evacuation" paths in `do_evacuations` be asserted with `!_only_promote_regions`?
I didn't go so far as to split them into separate tasks, but I did factor the different paths into separate methods. Also added an assert that the collection set is empty when we are only promoting regions in place.
> src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp line 1082:
>
>> 1080: _verify_liveness_complete, // liveness data must be complete here
>> 1081: _verify_regions_disable, // trash regions not yet recycled
>> 1082: sizeness, // expect generation and heap sizes to match exactly
>
> So... the comment is outdated? I suggest to split `ShenandoahVerifier::verify_after_concmark` into two methods, and clearly point out the expected state in comments then.
Done.
-------------
PR Review Comment: https://git.openjdk.org/shenandoah/pull/503#discussion_r1772059438
PR Review Comment: https://git.openjdk.org/shenandoah/pull/503#discussion_r1772059545
More information about the shenandoah-dev
mailing list