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