RFR: 8293210: G1: Remove redundant check in G1FreeHumongousRegionClosure

[Kim Barrett]

On Fri, 9 Sep 2022 09:51:43 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp line 236:
>> 
>>> 234: 
>>> 235: inline bool G1CollectedHeap::is_humongous_reclaim_candidate(uint region) {
>>> 236:   assert(_hrm.at(region)->is_starts_humongous(), "Must start a humongous object");
>> 
>> I think I like the semantic requirement that this only be applied to starts-humongous regions.  Unless there is some future change that needs this removal (and not just because of the change to G1FreeHumongousRegionClosure::do_heap_region that I don't like), then I'd prefer it stay.
>
> There are no other uses in the near future. However, this method doesn't really require that as a pre-condition, even before this PR.

That it doesn't (currently) require that is arguably an implementation detail.  I'm speaking of the intended semantics of the function.  I think it's a query about the region that corresponds to a humongous object.  It's placed near other predicates on objects.  That its argument is a region rather than an oop is an inconsistency that arises from how it's being used.  Maybe it's misplaced here?

>> src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp line 187:
>> 
>>> 185:     if (!r->is_starts_humongous()) {
>>> 186:       return false;
>>> 187:     }
>> 
>> I think this check is a fast-path to quickly filter out definitely uninteresting regions, which will be nearly all of them.  I'm not convinced it should be removed.
>
> It's the same argument as [JDK-8292858](https://bugs.openjdk.org/browse/JDK-8292858); the attr-table can (or should) be used instead of the region-type.

Yeah, and I thought about objecting to that one on the same grounds.  In retrospect I regret not doing so.

I see no issue with not using the attr-table.  The attr-table is not the UR-source of the information.  It is instead a cache of a non-trivial calculation.  Avoiding it by directly using one of the inputs to that calculation because doing so gives us a fast-path for what is by far the most common case seems perfectly fine.

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

PR: https://git.openjdk.org/jdk/pull/10119