RFR: 8310813: Simplify and modernize equals, hashCode, and compareTo for BigInteger [v6]

[Claes Redestad]

On Thu, 10 Aug 2023 11:38:08 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> 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/3619ef21...6fa3f694
>
> Here's a status update for this PR. I'm currently benchmarking baseline against this PR and this PR plus changes in https://github.com/openjdk/jdk/pull/15197. It's a 3-way benchmark, so to speak. Its purpose is to see whether performance degradation brought by this PR to `equals` and `compareTo` can be sufficiently offset by the improved `ArraysSupport.mismatch` mechanics.

Some context: On the `equals` and `compareTo` microbenchmarks @pavelrappo is adding in this PR there's a tiny regression from using `ArraysSupport.mismatch` when the `value` array has just a single element. Since such small `BigInteger`s appears to be exceedingly common (huh) it's been deemed hard to brush aside. Still this regression comes from the code having some extra branch, and is less than 2ns/op. All the while the speedup on huge `BigInteger`s is substantial. 

I think it's reasonable to move forward with this patch and accept the tiny regression on `equals/hashCode` with a magnitude of 1 (like we've done in other places such as `String.startsWith`) and separately continue investigating if `ArraysSupport.mismatch` can be improved to remove this deficiency.

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

PR Comment: https://git.openjdk.org/jdk/pull/14630#issuecomment-1674582483