RFR: 8278207: G1: Tighten verification in G1ResetSkipCompactingClosure

[Stefan Johansson]

On Mon, 6 Dec 2021 15:09:57 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1FullGCCompactTask.cpp line 64:
>> 
>>> 62:              "should be quite full");
>>> 63:     }
>>> 64: #endif
>> 
>> What do you think about extracting this to a function. Something like:
>> 
>> void assert_should_reset(HeapRegion*) PRODUCT_RETURN;
>> 
>> But you might be able to come up with a better name :)
>
> Considering this method is fairly short, I think having those asserts inlined doesn't hinder readability; instead, provides some documentation concerning in what context this method is invoked.

I think the fact that the method is short a good reason to extract the asserts, now the bulk of the method is asserts. If we want more documentation I would rather see a comment explaining in what situations regions might be marked as "skip compacting". We do have some documentation on this in `G1FullGCHeapRegionAttr`.

I'm fine with leaving the asserts here if you and others prefer it. One other question, can we ever end up here with a closed archive region? Those are marked `SkipMarking` so I guess they should be filtered out above?

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

PR: https://git.openjdk.java.net/jdk/pull/6698