RFR: 8338534: GenShen: Handle alloc failure differently when immediate garbage is pending

[William Kemper]

On Fri, 30 Aug 2024 23:03:10 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Several changes are implemented here:
>> 
>> 1. Re-order the phases that execute immediately after final-mark so that we do concurrent-cleanup quicker (but still after concurrent weak references)
>> 2. After immediate garbage has been reclaimed by concurrent cleanup, notify waiting allocators
>> 3. If an allocation failure occurs while immediate garbage recycling is pending, stall the allocation but do not cancel the concurrent gc.
>
> src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp line 756:
> 
>> 754:     size_t first_old, last_old, num_old;
>> 755:     heap->free_set()->prepare_to_rebuild(young_cset_regions, old_cset_regions, first_old, last_old, num_old);
>> 756:     size_t anticipated_immediate_garbage = (old_cset_regions + young_cset_regions) * ShenandoahHeapRegion::region_size_words();
> 
> This makes it sound like old_cset_regions & young_cset_regions hold all regions that will be part of `immediate garbage` -- which indeed is the case. The name `prepare_to_rebuild()` does not give one much clue as to the fact that that's happening when we return from the call, and the API spec of the method does not explicitly specify it. One needs to read into the code of the method `find_regions_with_alloc_capacity()` to realize that this is what is happening.
> 
> In summary, firstly, I feel some of these methods are in need of tighter header file documentation of post-conditions that callers are relying on and, secondly, I feel given the extremely fat APIs (lots of reference variables that are modified by these methods) that some amount of refactoring is needed in the longer term. The refactoring should be a separate effort, but in the shorter term I think the API/spec documentation of `prepare_to_rebuild` and `find_regions_with_alloc_capacity` could be improved.

The names `old_cset_regions` and `young_cset_regions` is very confusing as well. These regions are already trash _before_ the collection set is chosen during final mark (and so, will not themselves be part of the collection set). Suggest calling them `old_trashed_regions` and `young_trashed_regions` here.

-------------

PR Review Comment: https://git.openjdk.org/shenandoah/pull/479#discussion_r1739541915