RFR: 8294752: G1: Remove redundant checks in check_obj_during_refinement

[Albert Mingkun Yang]

On Tue, 4 Oct 2022 17:28:55 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Simple change of removing redundant assertions.
>> 
>> Test: hotspot_gc
>
> src/hotspot/share/gc/g1/g1OopClosures.inline.hpp line 125:
> 
>> 123:   HeapRegion* from = g1h->heap_region_containing(p);
>> 124: 
>> 125:   assert(from->is_in_reserved(p) || from->is_humongous(),
> 
> I think this whole assert is broken almost meaningless (this is all pre-existing).
> 
> `heap_region_containing` always returns the `HeapRegion` `p` is in. So I would expect the first part of the disjunction (both in current old an new code) always true.
> 
> It made sense in very old code (before 52a1ad0) because `_from` has been set to the humongous start region before executing this closure. Here's the original code where `check_obj_during_refinement()` came from:
> 
> 
>   assert(_from != NULL, "from region must be non-NULL");
>   assert(_from->is_in_reserved(p) ||
>          (_from->is_humongous() &&
>           _g1->heap_region_containing(p)->is_humongous() &&
>           _from->humongous_start_region() == _g1->heap_region_containing(p)->humongous_start_region()),
>          "p " PTR_FORMAT " is not in the same region %u or part of the correct humongous object starting at region %u.",
>          p2i(p), _from->hrm_index(), _from->humongous_start_region()->hrm_index());
> 
> `_from` has been set at the instantiation of the closure, and for humongous objects may have been different than `heap_region_containing(p)` if `p` were in humongous continues regions.
> 
> I would suggest to reduce the code to
> 
> assert(g1h->is_in(p), "must be");
> 
> or `is_in_reserved()`.
> 
> (The former is what the call to `heap_region_containing` also verifies).

Yes, you are right. Additionally, I believe the same is true for `obj` as well, i.e. `assert(g1h->is_in(obj))`, instead of `is_in_reserved(obj)`.

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

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