8312164: Refactor Arrays.hashCode for long, boolean, double, float, and Object arrays #14900

pavelrappo · 2023-07-17T09:36:35Z

Please review this PR to refactor Arrays.hashCode for long[], boolean[], and Object[]. I've been told elsewhere that it shouldn't have significant performance implications.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8312164: Refactor Arrays.hashCode for long, boolean, double, float, and Object arrays (Bug - P4)

Reviewers

Roger Riggs (@RogerRiggs - Reviewer)
Vyom Tewari (@vyommani - Committer) ⚠️ Review applies to 5fd3bb86

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/14900/head:pull/14900
$ git checkout pull/14900

Update a local copy of the PR:
$ git checkout pull/14900
$ git pull https://git.openjdk.org/jdk.git pull/14900/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 14900

View PR using the GUI difftool:
$ git pr show -t 14900

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14900.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2023-07-17T09:37:45Z

👋 Welcome back prappo! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2023-07-17T09:39:20Z

@pavelrappo The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2023-07-17T09:43:43Z

Webrevs

openjdk · 2023-07-17T14:35:52Z

@pavelrappo This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8312164: Refactor Arrays.hashCode for long, boolean, double, float, and Object arrays

Reviewed-by: rriggs, vtewari

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 48 new commits pushed to the master branch:

14cf035: 8302987: Add uniform and spatially equidistributed bounded double streams to RandomGenerator
d1c788c: 8312392: ARM32 build broken since 8311035
c119037: 8311971: SA's ConstantPool.java uses incorrect computation to read long value in the constant pool
028068a: 8312166: (dc) DatagramChannel's socket adaptor does not release carrier thread when blocking in receive
e7adbdb: 8311923: TestIRMatching.java fails on RISC-V
c6ab9c2: 8308103: Massive (up to ~30x) increase in C2 compilation time since JDK 17
d33e8e6: 8312200: Fix Parse::catch_call_exceptions memory leak
f677793: 8312190: Fix c++11-narrowing warnings in hotspot code
82612e2: 8312329: Minimal build failure after JDK-8311541
702fea8: 8312147: Dynamic Exception Specification warnings are no longer required after JDK-8311380
... and 38 more: https://git.openjdk.org/jdk/compare/f6e23ae4519f8eb877ae28a0bfbb4820c9718f91...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

vyommani

Looks OK to me.

pavelrappo · 2023-07-19T13:08:32Z

Please (re-)review this PR. The recent three commits improve readability of hashCode(double[]) and hashCode(float[]).

pavelrappo · 2023-07-19T17:19:28Z

/integrate

openjdk · 2023-07-19T17:21:22Z

Going to push as commit b5b6f4e.
Since your change was applied there have been 48 commits pushed to the master branch:

14cf035: 8302987: Add uniform and spatially equidistributed bounded double streams to RandomGenerator
d1c788c: 8312392: ARM32 build broken since 8311035
c119037: 8311971: SA's ConstantPool.java uses incorrect computation to read long value in the constant pool
028068a: 8312166: (dc) DatagramChannel's socket adaptor does not release carrier thread when blocking in receive
e7adbdb: 8311923: TestIRMatching.java fails on RISC-V
c6ab9c2: 8308103: Massive (up to ~30x) increase in C2 compilation time since JDK 17
d33e8e6: 8312200: Fix Parse::catch_call_exceptions memory leak
f677793: 8312190: Fix c++11-narrowing warnings in hotspot code
82612e2: 8312329: Minimal build failure after JDK-8311541
702fea8: 8312147: Dynamic Exception Specification warnings are no longer required after JDK-8311380
... and 38 more: https://git.openjdk.org/jdk/compare/f6e23ae4519f8eb877ae28a0bfbb4820c9718f91...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-07-19T17:21:31Z

@pavelrappo Pushed as commit b5b6f4e.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Glavo

I ran a JMH benchmark for this PR:

@Warmup(iterations = 5, time = 3)
@Measurement(iterations = 5, time = 2)
@Fork(value = 1, jvmArgsAppend = {"-XX:+UseG1GC", "-Xms8g", "-Xmx8g"})
@BenchmarkMode(Mode.Throughput)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
@State(Scope.Benchmark)
public class HashCode {
    @Param({"1", "10", "100", "10000"})
    private int size;

    private long[] longs;
    private boolean[] booleans;
    private float[] floats;
    private double[] doubles;
    private Object[] objects;

    @Setup
    public void setup() {
        longs = new long[size];
        booleans = new boolean[size];
        floats = new float[size];
        doubles = new double[size];
        objects = new Object[size];

        Random rnd = new Random(0);
        for (int i = 0; i < size; i++) {
            long next = rnd.nextLong();

            longs[i] = next;
            booleans[i] = next % 2 == 0;
            floats[i] = Float.intBitsToFloat((int) next);
            doubles[i] = Double.longBitsToDouble(next);
            objects[i] = (int) next;
        }
    }

    @Benchmark
    public int longs() {
        return Arrays.hashCode(longs);
    }

    @Benchmark
    public int booleans() {
        return Arrays.hashCode(booleans);
    }

    @Benchmark
    public int floats() {
        return Arrays.hashCode(floats);
    }

    @Benchmark
    public int doubles() {
        return Arrays.hashCode(doubles);
    }

    @Benchmark
    public int objects() {
        return Arrays.hashCode(objects);
    }
}

Result:

                                                 (This PR)                        (Baseline)   

Benchmark          (size)   Mode  Cnt       Score       Error   Units         Score       Error   Units
HashCode.booleans       1  thrpt    5  472358.594 ±  5037.260  ops/ms    473356.084 ±  1780.600  ops/ms
HashCode.booleans      10  thrpt    5  118457.276 ±   848.886  ops/ms    118470.818 ±   342.023  ops/ms
HashCode.booleans     100  thrpt    5   13140.346 ±    46.869  ops/ms     13153.643 ±     1.226  ops/ms
HashCode.booleans   10000  thrpt    5     133.090 ±     0.027  ops/ms       133.041 ±     0.032  ops/ms
HashCode.doubles        1  thrpt    5  438869.947 ±  1806.730  ops/ms    441611.989 ±   692.622  ops/ms
HashCode.doubles       10  thrpt    5   76418.134 ±    42.834  ops/ms     76432.141 ±   368.953  ops/ms
HashCode.doubles      100  thrpt    5    7364.920 ±     6.941  ops/ms      7339.776 ±    28.448  ops/ms
HashCode.doubles    10000  thrpt    5      76.689 ±     0.037  ops/ms        76.787 ±     0.188  ops/ms
HashCode.floats         1  thrpt    5  443741.068 ±  1773.736  ops/ms    444085.364 ±   557.545  ops/ms
HashCode.floats        10  thrpt    5   86047.878 ±   306.709  ops/ms     86306.681 ±   908.752  ops/ms
HashCode.floats       100  thrpt    5    8263.785 ±    37.091  ops/ms      8254.055 ±     3.427  ops/ms
HashCode.floats     10000  thrpt    5      86.146 ±     0.018  ops/ms        86.121 ±     0.022  ops/ms
HashCode.longs          1  thrpt    5  462198.590 ± 27224.002  ops/ms    460673.957 ± 26979.630  ops/ms
HashCode.longs         10  thrpt    5  124744.207 ±   262.268  ops/ms    130367.753 ±   257.766  ops/ms
HashCode.longs        100  thrpt    5   13552.607 ±     2.545  ops/ms     13454.362 ±     8.991  ops/ms
HashCode.longs      10000  thrpt    5     133.122 ±     0.038  ops/ms       133.114 ±     0.016  ops/ms
HashCode.objects        1  thrpt    5  230303.967 ±   919.637  ops/ms    647597.213 ±  2652.002  ops/ms
HashCode.objects       10  thrpt    5   25724.486 ±   111.422  ops/ms    121222.501 ±   270.073  ops/ms
HashCode.objects      100  thrpt    5    2497.018 ±     6.201  ops/ms     11889.717 ±   264.730  ops/ms
HashCode.objects    10000  thrpt    5      27.673 ±     0.012  ops/ms       130.903 ±     0.714  ops/ms

I think there is a performance issue with this PR. For object arrays, the performance is reduced by about 80%.

(I got the result reversed in a previous comment, so I hid it.)

Glavo · 2023-07-20T01:14:59Z

I created #14944 to revert changes to Arrays.hashCode(Object[])

Initial commit

5fd3bb8

openjdk bot added the rfr Pull request is ready for review label Jul 17, 2023

openjdk bot added the core-libs core-libs-dev@openjdk.org label Jul 17, 2023

RogerRiggs approved these changes Jul 17, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jul 17, 2023

vyommani approved these changes Jul 17, 2023

View reviewed changes

Pavel Rappo added 3 commits July 19, 2023 13:59

Fix hashCode(double[]): part 1

0f33244

Fix hashCode(double[]): part 2

81d0f0b

Fix hashCode(float[])

92d0947

RogerRiggs approved these changes Jul 19, 2023

View reviewed changes

pavelrappo changed the title ~~8312164: Refactor Arrays.hashCode for long, boolean, and Object arrays~~ 8312164: Refactor Arrays.hashCode for long, boolean, double, float, and Object arrays Jul 19, 2023

openjdk bot added the integrated Pull request has been integrated label Jul 19, 2023

openjdk bot closed this Jul 19, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jul 19, 2023

This comment was marked as resolved.

Sign in to view

Glavo reviewed Jul 20, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8312164: Refactor Arrays.hashCode for long, boolean, double, float, and Object arrays #14900

8312164: Refactor Arrays.hashCode for long, boolean, double, float, and Object arrays #14900

pavelrappo commented Jul 17, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jul 17, 2023

openjdk bot commented Jul 17, 2023

mlbridge bot commented Jul 17, 2023 •

edited

Loading

openjdk bot commented Jul 17, 2023 •

edited

Loading

vyommani left a comment

pavelrappo commented Jul 19, 2023

pavelrappo commented Jul 19, 2023

openjdk bot commented Jul 19, 2023

openjdk bot commented Jul 19, 2023

This comment was marked as resolved.

Glavo left a comment •

edited

Loading

Glavo commented Jul 20, 2023

8312164: Refactor Arrays.hashCode for long, boolean, double, float, and Object arrays #14900

8312164: Refactor Arrays.hashCode for long, boolean, double, float, and Object arrays #14900

Conversation

pavelrappo commented Jul 17, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Jul 17, 2023

openjdk bot commented Jul 17, 2023

mlbridge bot commented Jul 17, 2023 • edited Loading

Webrevs

openjdk bot commented Jul 17, 2023 • edited Loading

vyommani left a comment

Choose a reason for hiding this comment

pavelrappo commented Jul 19, 2023

pavelrappo commented Jul 19, 2023

openjdk bot commented Jul 19, 2023

openjdk bot commented Jul 19, 2023

This comment was marked as resolved.

Glavo left a comment • edited Loading

Choose a reason for hiding this comment

Glavo commented Jul 20, 2023

pavelrappo commented Jul 17, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Jul 17, 2023 •

edited

Loading

openjdk bot commented Jul 17, 2023 •

edited

Loading

Glavo left a comment •

edited

Loading