RFR: 8302206: Factor out duplicate G1VerificationClosure

[Kim Barrett]

On Fri, 10 Feb 2023 12:43:04 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Hi all,
> 
>   can I have reviews for this cleanup that merges `G1VerifyLiveClosure` from `g1FullGCMarker.cpp` and `VerifyLiveClosure` from `heapRegion.cpp` into a single one as they are virtually identical.
> 
> There is probably one change that needs some discussion, and that is about changing the type and meaning of `G1MaxVerifyFailures`: after changing the type of `G1VerificationClosure::_num_failures` to uint, the given comparison does not work any more due to differing types.
> However the change of the default value to `0` meaning "show all messages" does not seem too bad: even before this change, a value of `0` at least showed one failure as this check (now it shows all messages) is located after the error printing in the closures.
> It's a develop flag too, so instead of casting around I chose to simply change the meaning.
> 
> Testing: gha
> 
> Thanks,
>   Thomas

Changes requested by kbarrett (Reviewer).

src/hotspot/share/gc/g1/heapRegion.cpp line 482:

> 480:       from != to &&
> 481:       !to->is_pinned() &&
> 482:       to->rem_set()->is_complete()) {

Mis-indented multi-line tests.  These are part of the if-condition, so should be lined up inside the `()`.
With the indentation being used it's not so easy to spot where the condition stops and the body ends.
Although I see it was like that before the change too, except this change adds a blank line separator.
I don't think the blank line is enough (easy to miss it's importance, esp. in a review diff), and these
lines should be indented further.

src/hotspot/share/gc/g1/heapRegion.cpp line 499:

> 497:         LogStream ls(log.error());
> 498: 
> 499:         MutexLocker x(ParGCRareEvent_lock, Mutex::_no_safepoint_check_flag);

[pre-existing] That's a weird name for a mutex that seems to only be used by G1.

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

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