RFR: 8311170: Simplify and modernize equals and hashCode in security area [v3]

[Pavel Rappo]

On Wed, 5 Jul 2023 12:31:09 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> Pavel Rappo has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add even more cases and tidy up
>
> src/java.base/share/classes/java/security/spec/ECFieldF2m.java line 235:
> 
>> 233:     public int hashCode() {
>> 234:         int value = m << 5;
>> 235:         // consider simplifying using Objects.hashCode(rp) after JDK-8015417
> 
> Could you explain this comment? Why does it apply here, and not to other methods like CodeSigner.hashCode?

You are absolutely right: adding that comment, while actively disregarding concerns described in it elsewhere, warrants some explanation.

While working on this and the related PRs, I discovered JDK-8015417. hashCode and, to a lesser extent, equals in ECFieldF2m seemed lean and performance-sensitive so as to apply the smarts of JDK-8015417. Frankly, performance sensitivity was my guess; anyway, I kept the original behaviour for that site.

Since then, I learned that JDK-8015417 is quite complicated and well beyond my level of understanding of how JVM works. Until I have clearer understanding of the effects of that issue on this and similar refactoring efforts, I won't integrate this or any other PRs where using the java.util.Objects static methods is a primary goal.

That said, I'll remove the comment and refactor that `?:` conditional to use `Objects.equals(Object, Object)`, to be consistent with the rest of this PR.

FWIW, we've accidentally started discussion on those effects in another PR: https://github.com/openjdk/jdk/pull/14752#pullrequestreview-1511190453. It might as well have been this PR, but that PR was first. While either PR is good, let's keep it confined and organized. So, please follow that discussion, if you're interested.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14738#discussion_r1253214761