RFR: 8329858: G1: Make G1VerifyLiveAndRemSetClosure stateless

[Stefan Karlsson]

On Mon, 8 Apr 2024 12:50:59 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> In [JDK-8329570](https://bugs.openjdk.org/browse/JDK-8329570): 'G1: Excessive is_obj_dead_cond calls in verification' we moved the verification of the containing object so that it was done only once. We left the repeated null-check because of concerns that moving it would introduce the risk that someone in the future would miss calling `set_containing_obj` or calling it with a null object.
>> 
>> I propose that we restructure the code to make the closure's state const and remove the set_containing_obj function so that we can feel confident that we don't hit such a bug in the future.
>> 
>> The patch also combines the failures from verification of the oops with failures from checking the containing objects. I had a version that kept these two separate, but I don't see why we would want that. Tell me if you'd want me to reintroduce this separation.
>
> src/hotspot/share/gc/g1/g1HeapRegion.cpp line 651:
> 
>> 649:       _failures(failures) {
>> 650:     assert(containing_obj != nullptr, "must be");
>> 651:     assert(!G1CollectedHeap::heap()->is_obj_dead_cond(containing_obj, _vo), "Precondition");
> 
> In this new structure, these two asserts are not very useful any more, IMO.

Good point. The `is_obj_dead_cond` has already been checked by the caller. I can see why someone would like to keep the null-check. I'll see what @tschatzl says, and will make changes after that.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18677#discussion_r1555807912