RFR: 8329764: G1: Handle null references during verification first [v2]

[Thomas Schatzl]

On Fri, 5 Apr 2024 12:09:35 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   please review this change that suggests to move the null reference check in object iteration during heap verification first.
>> 
>> The reason is as stated, since null references are fairly common (not only in that test mentioned in https://bugs.openjdk.org/browse/JDK-8329314), it may make sense to put it first. Also, null references never fail anyway (and verification failure is supposed to be uncommon).
>> 
>> Improves total runtime of that test case from 3s (~3.9s cpu time) to 2.2s (~3.1s cpu time).
>> 
>> If you think it makes the code too ugly, I will retract it.
>> 
>> Testing: gha, local testing.
>
> Thomas Schatzl has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase.

> I integrated the simplest version of https://github.com/openjdk/jdk/pull/18595 now. I still think it would be nice to clean away the assert(_containing_obj...) with my proposed changes. Should I do that? Or do you have some other plan for it?

I am good with moving the `assert` too (not sure if it is worth an extra change or adding it here); however this change, i.e. moving the condition, would improve performance beyond moving the assert as shown above.

The other changes from you about extracting the failure counter could be done as well - when refactoring this verification code the last time the extra 30 LOC or so did not seem worth to me, but the code is already fairly large.

There are no further plans from me to improve verification performance further, apart from maybe filing a CR to parallelize verification of large object arrays.

Does that answer your questions?

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

PR Comment: https://git.openjdk.org/jdk/pull/18650#issuecomment-2039644297