RFR: 8310813: Simplify and modernize equals, hashCode, and compareTo for BigInteger [v6]
Claes Redestad
redestad at openjdk.org
Wed Aug 9 23:02:59 UTC 2023
On Wed, 9 Aug 2023 22:54:28 GMT, Pavel Rappo <prappo at openjdk.org> wrote:
>> Please review this PR to use modern APIs and language features to simplify equals, hashCode, and compareTo for BigInteger. If you have any performance concerns, please raise them.
>>
>> This PR is cherry-picked from a bigger, not-yet-published PR, to test the waters. That latter PR will be published soon.
>
> Pavel Rappo 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. The pull request contains 13 additional commits since the last revision:
>
> - Fix bugs in Shared.createSingle
> - Merge branch 'master' into 8310813
> - Group params coarser (suggested by @cl4es)
>
> - Splits 20 params into 3 groups: (S)mall, (M)edium and (L)arge.
> Every testXYZ method invokes M operations, where M is the maximum
> number of elements in a group. Shorter groups are cyclically padded.
> - Uses the org.openjdk.jmh.infra.Blackhole API and increases
> benchmark time.
> - Fixes a bug in Shared that precluded 0 from being in a pair.
> - Use better overloads (suggested by @cl4es)
>
> - Uses simpler, more suitable overloads for the subrange
> starting from 0
> - Improve benchmarks
> - Merge branch 'master' into 8310813
> - Restore hash code values
>
> BigInteger is old and ubiquitous enough so that there might be
> inadvertent dependencies on its hash code.
>
> This commit also includes a test, to make sure hash code is
> unchanged.
> - Merge branch 'master' into 8310813
> - Add a benchmark
> - Merge branch 'master' into 8310813
> - ... and 3 more: https://git.openjdk.org/jdk/compare/d5d285cb...6fa3f694
This looks good, thanks for the microbenchmark updates.
Your work on this PR was what prompted me to look at `ArraysSupport.mismatch` performance. The patch in #15197 should mitigate at least some of the overhead we've seen on small `BigInteger` objects on `equals` and `compareTo` micros
test/micro/org/openjdk/bench/java/math/Shared.java line 82:
> 80: }
> 81: int nBytes = (nBits + 7) / 8;
> 82: var r = new Random();
Some have a preference for providing a seed for `Random` instances in micros. Either hard-coded or through a `@Param` (I find this a bit excessive). Doing so might reduce run-to-run noise.
-------------
Marked as reviewed by redestad (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14630#pullrequestreview-1570712845
PR Review Comment: https://git.openjdk.org/jdk/pull/14630#discussion_r1289313113
More information about the core-libs-dev
mailing list