RFR: 8302206: Factor out duplicate G1VerificationClosure [v4]
Kim Barrett
kbarrett at openjdk.org
Mon Feb 20 08:56:29 UTC 2023
On Mon, 13 Feb 2023 20:39:53 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
>
> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
>
> Minor removal of unnecessary code
Looks good except for the one simple change suggested.
src/hotspot/share/gc/g1/g1_globals.hpp line 226:
> 224: develop(size_t, G1MaxVerifyFailures, max_uintx, \
> 225: "The maximum number of verification failures to print.") \
> 226: range(1, max_uintx) \
s/max_uintx/SIZE_MAX/ in both places.
-------------
Marked as reviewed by kbarrett (Reviewer).
PR: https://git.openjdk.org/jdk/pull/12511
More information about the hotspot-gc-dev
mailing list