RFR: 8317371: GenShen: Needs improved support for humongous allocation [v3]

[William Kemper]

On Wed, 11 Oct 2023 19:09:52 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> Several improvements:
>> 1. If there is a humongous allocation failure, go immediately to Full GC rather than trying 64 degenerated GC first
>> 2. Reduce likelihood of humongous allocation errors by:
>>     a. Allocating regular objects from top end of memory while allocating humongous from bottom end of memory
>>     b. (Especially) in the case that regular regions that are promoted in place reside within the bottom end of memory, detect that old-gen memory has become fragmented and trigger old-gen GC, which will make a special effort to move these out-of-place old-gen regions to higher addresses.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Cosmetic improvements

Changes requested by wkemper (Committer).

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 405:

> 403:   if (cand_idx > _last_old_collection_candidate) {
> 404:     // Sort the regions that were initially rejected from the collection set in order of index.
> 405:     QuickSort::sort<RegionData>(candidates + _last_old_collection_candidate, cand_idx - _last_old_collection_candidate,

What is the purpose of sorting these regions by index? How does this aid defragmentation?

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 562:

> 560:     size_t first_active_region = heap->free_set()->first_old_region();
> 561:     size_t last_active_region = heap->free_set()->last_old_region();
> 562:     size_t span_of_active_regions = (last_active_region > first_active_region)? last_active_region + 1 - first_active_region: 0;

Why did we compute `span_of_active_regions`? Doesn't seem used. If we don't need `span_of_active_regions`, do we still need the new methods: `first_old_region` and `last_old_region`?

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 566:

> 564:     size_t first_old_region, last_old_region;
> 565:     double density;
> 566:     fragmentation_trigger_reason(density, first_old_region, last_old_region);

Just use the member fields here?

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 572:

> 570: 
> 571:     // New active regions may have came into play following the trigger.
> 572:     size_t first_region = MIN2(first_active_region, first_old_region);

I don't understand this. The active region counts are not updated after the trigger?

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 3115:

> 3113:   size_t young_cset_regions, old_cset_regions;
> 3114:   size_t first_old_region, last_old_region, old_region_count;
> 3115:   _free_set->prepare_to_rebuild(young_cset_regions, old_cset_regions, first_old_region, last_old_region, old_region_count);

Should we have an assert here like:
```C++
assert(first_old_region == _free_set->first_old_region() && last_old_region == _free_set->last_old_region());

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 3163:

> 3161: 
> 3162:     uint eighths = 8;
> 3163:     bool triggered = false;

`triggered` looks unused?

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

PR Review: https://git.openjdk.org/shenandoah/pull/336#pullrequestreview-1674779498
PR Review Comment: https://git.openjdk.org/shenandoah/pull/336#discussion_r1357200341
PR Review Comment: https://git.openjdk.org/shenandoah/pull/336#discussion_r1357190839
PR Review Comment: https://git.openjdk.org/shenandoah/pull/336#discussion_r1357198127
PR Review Comment: https://git.openjdk.org/shenandoah/pull/336#discussion_r1357195186
PR Review Comment: https://git.openjdk.org/shenandoah/pull/336#discussion_r1357188019
PR Review Comment: https://git.openjdk.org/shenandoah/pull/336#discussion_r1357172088