On Wed, 5 Jul 2023 14:39:44 GMT, Pavel Rappo <pra...@openjdk.org> wrote:
>> 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.hashCode(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.
Refactored in 3f316b49f26.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14738#discussion_r1253222537
