RFR: 8338780: GenShen: Fix up some comments

[Kelvin Nilsen]

On Thu, 22 Aug 2024 17:41:22 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> src/hotspot/share/gc/shenandoah/heuristics/shenandoahGlobalHeuristics.cpp line 115:
>> 
>>> 113:     ShenandoahHeapRegion* r = data[idx].get_region();
>>> 114:     if (cset->is_preselected(r->index())) {
>>> 115:       assert(false, "There should be no preselected regions during GLOBAL GC");
>> 
>> If this is a true invariant (which it is in testing to date), then it must be the case that the call at line 55 above to `add_preselected_regions_to_collection_set()` doesn't do anything, and indeed returns `0` into `cur_young_garbage` there.
>> 
>> Clearly, I am missing something here. @kdnilsen ?
>
> I'll run some testing with another assert at the code at line 55 to check my (mis)understanding.

You are correct.  In ShenandoahGenerationalHeuristics::choose_collection_set(), we only call prime_collection_set() if generation->is_young().  For global collections, we will not prime the collection set, which means we will not preselect any regions for promotion.

Given this reality, we can remove the call to add_preselected_regions_to_collection_SET() from ShenandoahGlobalHeuristics::choose_collection_set_from_regiondata().  Keeping the assertion in place is how we know it was safe to remove the call to add preselected regions.

We could do some archeology on the code to remind ourselves how we got here.  But maybe not necessary.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/484#discussion_r1727594983