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