On Thu, 20 Jul 2023 15:32:09 GMT, Pavel Rappo <pra...@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 seven additional
> commits since the last revision:
>
> - 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
> - Use fewer range checks to improve performance
> - Improve
> - Initial commit
Made benchmarks more sensible. Bit length, which is a sole benchmark parameter,
is now based on stats I gathered from running tests in the security area, which
seems to be the biggest client of BigInteger in JDK. `equals` and `compareTo`
now benchmark the worst-case scenario: each method compares two numbers whose
binary representations differ minimally.
<details>
<summary>Here's how to replicate my stats, if required...</summary>
1. Add logging to `equals`, `hashCode`, and `compareTo`:
```
diff --git a/src/java.base/share/classes/java/math/BigInteger.java
b/src/java.base/share/classes/java/math/BigInteger.java
index 1803b77df54..214a9ce4e4f 100644
--- a/src/java.base/share/classes/java/math/BigInteger.java
+++ b/src/java.base/share/classes/java/math/BigInteger.java
@@ -3924,6 +3924,7 @@ public class BigInteger extends Number implements
Comparable<BigInteger> {
*/
@Override
public int compareTo(BigInteger val) {
+ System.out.println("#cmp#" + Math.min(this.bitLength(),
val.bitLength()));
if (signum == val.signum) {
return switch (signum) {
case 1 -> compareMagnitude(val);
@@ -4016,6 +4017,8 @@ public class BigInteger extends Number implements
Comparable<BigInteger> {
if (!(x instanceof BigInteger xInt))
return false;
+ System.out.println("#eq#" + Math.min(this.bitLength(),
xInt.bitLength()));
+
if (xInt.signum != signum)
return false;
@@ -4055,6 +4058,7 @@ public class BigInteger extends Number implements
Comparable<BigInteger> {
*/
@Override
public int hashCode() {
+ System.out.println("#hc#" + bitLength());
return ArraysSupport.vectorizedHashCode(mag, 0, mag.length, 0,
ArraysSupport.T_INT) * signum;
}
```
2. Run tests:
```
jtreg -J-Djavatest.maxOutputSize=999999999 -retain:all -verbose:summary
-testjdk:build/macosx-aarch64/images/jdk/ test/jdk/:jdk_security
```
3. Ensure that the output is complete, that is, there are no "Output overflow:"
in the jtr files. If there are, you might not be seeing the full data; increase
the `maxOutputValue`, repeat step 2.
**Note: besides taking considerable time to run, this test configuration
results in the JTwork directory whose size is 4G.**
</details>
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14630#issuecomment-1649878980
