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