RFR: 8339616: GenShen: Introduce new state to distinguish promote-in-place phase as distinct from concurrent evacuation [v2]
Aleksey Shipilev
shade at openjdk.org
Mon Sep 23 18:11:07 UTC 2024
On Fri, 20 Sep 2024 23:11:22 GMT, William Kemper <wkemper at openjdk.org> wrote:
>> The generational mode for Shenandoah is able to promote regions in place (effectively moving an entire region of live objects, without evacuating any objects). It does this during a normal evacuation phase. However, in some cases, Shenandoah may choose to skip the evacuation phase, _even when there are entire regions to promote_. Prior to this PR, Shenandoah would essentially force itself into an evacuation phase, with nothing to evacuate. Though this was expedient, it caused all manner of knock-on effects and unintended consequences with barriers and asserts and the state of the heap. Here, we have reverted that business and created a path for this special "only doing in place promotions" cycle that does not cause the heap to expect to evacuate objects or find forwarding pointers in them.
>
> 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
Looks okay, but a few cosmetics:
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`?
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.
-------------
Marked as reviewed by shade (Committer).
PR Review: https://git.openjdk.org/shenandoah/pull/503#pullrequestreview-2322981435
PR Review Comment: https://git.openjdk.org/shenandoah/pull/503#discussion_r1771876993
PR Review Comment: https://git.openjdk.org/shenandoah/pull/503#discussion_r1771879017
More information about the shenandoah-dev
mailing list