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

[Stefan Karlsson]

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 #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?

I had hoped for a more clear yes or no. :)

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

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