On Thu, 10 Aug 2023 11:38:08 GMT, Pavel Rappo <pra...@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
