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

[Thomas Schatzl]

On Tue, 9 Apr 2024 15:14:23 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 pull request now contains three commits:
> 
>  - Merge branch 'master' into submit/8329764-verification-null-refs-first
>  - 8329764
>  - 8329570: G1: Excessive is_obj_dead_cond calls in verification

> > I was think of something similar, also makes the iteration actually stop at G1MaxVerifyFailures instead of at some related but arbitrary larger number.
> 
> From #18595
> 
> Stopping field-iteration at the specified limit sounds much nicer; can that be done instead of (or in addition to) this reordering?

I am not completely sure what the exact goal of stopping field-iteration at specified limit is, however I think both options are orthonal: this change speeds up the common case (no error, null references), while the other would presumably make the VM exit faster in the failure case and/or guarantees to write only a particular amount of failure messages?

Just brainstorming:
  * making the VM exit faster after an error would require some way to abort iteration (or at least split iteration into parts for large objarrays?),
  * making the VM observe `G1MaxVerifyFailure` better (at the moment it is like `#verification threads * G1MaxVerifyFailure`) would need some global synchronization, which would make it much slower again I guess?

In any case these two efforts seem to be completely independent.

As mentioned initially, I am okay to just close this PR, but it seems low hanging fruit obtained with little effort.

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

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