RFR: 8294752: G1: Remove redundant checks in check_obj_during_refinement
Thomas Schatzl
tschatzl at openjdk.org
Tue Oct 4 18:42:50 UTC 2022
On Tue, 4 Oct 2022 09:30:48 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
> Simple change of removing redundant assertions.
>
> Test: hotspot_gc
Changes requested by tschatzl (Reviewer).
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).
-------------
PR: https://git.openjdk.org/jdk/pull/10551
More information about the hotspot-gc-dev
mailing list