RFR: 8352869: Verify.checkEQ: extension for NaN, VectorAPI and arbitrary Objects [v5]

[Christian Hagedorn]

On Tue, 1 Apr 2025 07:02:11 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> test/hotspot/jtreg/compiler/lib/verify/Verify.java line 209:
>> 
>>> 207:                     print(a, b, field, aParent, bParent);
>>> 208:                     throw new VerifyException("Object type not supported: " + ca.getName() + " -- did you mean to 'enableCheckWithArbitraryClasses'?");
>>> 209:                 }
>> 
>> What's the reason behind throwing instead of just comparing two arbitrary objects by default? If a user calls `Verify.checkEQ()` and sees this exception, I would guess he then just passes the additional option and we have the same result. But maybe I'm missing something.
>
> Good question. I think my reasoning was that comparing arbitrary classes requires reflection. And that is rather slow. So by default it would be good if that feature is not enabled, so the user tries to avoid it, and is aware when they enable it explicitly.
> 
> But if you think that is not useful, I can remove the feature.
> 
> @chhagedorn what do you think?

I think the intention to let the user double check is good. I'm not sure though if the user is really aware of the potential slow down without diving deeper into the implementation. All they know is that `checkEQ` somehow does not support their some objects but there is a simple workaround to still use it. So, the real question is: How many users will then consider doing something different when facing this exception and not just enable it anyway? I guess enabling is probably the most natural thing to do.

Given that, I would probably just drop this. It would also simplify the API usage in the following way:

We would only have checks with NaNs being all equals and comparing raw bits (i.e. NaNs not equal). Then you could offer `checkEQ()` (default) and `checkRawBitsEQ()` or something like that. Then users do not need to worry about creating and passing in an `Options`.

What do you think about these suggestions?

What we could do either way at the `checkEQ()` API method: Describe the potential slow down with reflection when not using certain classes.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24224#discussion_r2022461294