8273681: Add Vector API vs Arrays.mismatch intrinsic benchmark #5459

amCap1712 · 2021-09-10T08:32:02Z

Hi all!

Please review this PR to add a benchmark comparing the performance of Arrays.mismatch intrinsic in the JDK with that of the Vector API. Kindly refer to this thread on panama-dev regarding some initial discussion about this benchmark. I have attached the results of the full benchmark run along with the assembly output of a shorter run I had done while analysing the results. The benchmarks were run against the latest build of panama-vector available from builds.shipilev.net.

Also, I have not added the copyright header to this file yet as I am an individual contributor (OCA signed) and do not know what to put there.

Regards,
Kartik

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8273681: Add Vector API vs Arrays.mismatch intrinsic benchmark

Reviewers

Paul Sandoz (@PaulSandoz - Reviewer) ⚠️ Review applies to b057780

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5459/head:pull/5459
$ git checkout pull/5459

Update a local copy of the PR:
$ git checkout pull/5459
$ git pull https://git.openjdk.java.net/jdk pull/5459/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5459

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5459.diff

bridgekeeper · 2021-09-10T08:33:09Z

👋 Welcome back amCap1712! 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 · 2021-09-10T08:35:56Z

@amCap1712 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.

PaulSandoz · 2021-09-13T17:47:53Z

/issue JDK-8273681

openjdk · 2021-09-13T17:48:09Z

@PaulSandoz Only the author (@amCap1712) is allowed to issue the /issue command.

amCap1712 · 2021-09-13T17:50:12Z

/issue JDK-8273681

openjdk · 2021-09-13T17:51:08Z

@amCap1712 The primary solved issue for a PR is set through the PR title. Since the current title does not contain an issue reference, it will now be updated.

mlbridge · 2021-09-13T17:57:23Z

Webrevs

PaulSandoz · 2021-09-13T18:05:55Z

The simplest approach is to copy the license from here https://github.com/openjdk/jdk/blob/master/test/micro/org/openjdk/bench/jdk/incubator/vector/RotateBenchmark.java

PaulSandoz

Benchmark looks good (assuming license is added).

PaulSandoz · 2021-09-13T18:19:24Z

test/micro/org/openjdk/bench/jdk/incubator/vector/ArrayMismatchBenchmark.java

+    static final VectorSpecies<Float> FLOAT_SPECIES_PREFERRED = FloatVector.SPECIES_PREFERRED;
+    static final VectorSpecies<Long> LONG_SPECIES_PREFERRED = LongVector.SPECIES_PREFERRED;
+
+    static final Random random = new Random();


We could use the recently added RandomGenerator instead in the spirit of encouraging the use of new and preferred APIs:

remove the static field

replace FLOAT_SPECIES with DOUBLE_SPECIES

in setup create an instance RandomGenerator rg = RandomGenerator.getDefault()

remove createRandomFloats and use rg.doubles(...).toArray()

PaulSandoz

Looks good, just a minor issue (that does not induce incorrect behavior), no need for re-approval to fix that.

test/micro/org/openjdk/bench/jdk/incubator/vector/ArrayMismatchBenchmark.java

openjdk · 2021-09-16T22:53:29Z

@amCap1712 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:

8273681: Add Vector API vs Arrays.mismatch intrinsic benchmark

Reviewed-by: psandoz

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 114 new commits pushed to the master branch:

8c022e2: 8270434: JDI+UT: Unexpected event in JDI tests
b982904: 8271073: Improve testing with VM option VerifyArchivedFields
bc48a0a: 8273902: Memory leak in OopStorage due to bug in OopHandle::release()
9c5441c: 8271569: Clean up the use of CDS constants and field offsets
12fa707: 8261941: Use ClassLoader for unregistered classes during -Xshare:dump
7e92abe: 8273710: Remove redundant stream() call before forEach in jdk.jdeps
59b2478: 8273659: Replay compilation crashes with SIGSEGV since 8271911
5e4d09c: 8273300: Check Mutex ranking during a safepoint
c86e24d: 8271954: C2: assert(false) failed: Bad graph detected in build_loop_late
14dc517: 8273372: Remove scavenge trace message in psPromotionManager
... and 104 more: https://git.openjdk.java.net/jdk/compare/267c61a16a916e35762e8df5737ec74b06defae8...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@PaulSandoz) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

Co-authored-by: Paul Sandoz <paul.d.sandoz@googlemail.com>

amCap1712 · 2021-09-17T04:49:31Z

Thanks for the review, @PaulSandoz
/integrate

openjdk · 2021-09-17T04:51:11Z

@amCap1712
Your change (at version 869e4f2) is now ready to be sponsored by a Committer.

PaulSandoz · 2021-09-17T15:10:51Z

/sponsor

openjdk · 2021-09-17T15:12:45Z

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

17f7a45: 8273913: Problem list some headful client jtreg tests that fail on macOS 12
27d747a: 8273877: os::unsetenv unused
35f6f1d: 8273808: Cleanup AddFontsToX11FontPath
1890d85: 8273872: ZGC: Explicitly use 2M large pages
54b4567: 8273880: Zero: Print warnings when unsupported intrinsics are enabled
e07ab82: 8273408: java.lang.AssertionError: typeSig ERROR on generated class property of record
8c022e2: 8270434: JDI+UT: Unexpected event in JDI tests
b982904: 8271073: Improve testing with VM option VerifyArchivedFields
bc48a0a: 8273902: Memory leak in OopStorage due to bug in OopHandle::release()
9c5441c: 8271569: Clean up the use of CDS constants and field offsets
... and 110 more: https://git.openjdk.java.net/jdk/compare/267c61a16a916e35762e8df5737ec74b06defae8...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-09-17T15:13:05Z

@PaulSandoz @amCap1712 Pushed as commit 2f8c221.

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

Add Vector API vs Arrays.mismatch intrinsic benchmark

cf9328a

openjdk bot added the core-libs core-libs-dev@openjdk.org label Sep 10, 2021

openjdk bot changed the title ~~Add Vector API vs Arrays.mismatch intrinsic benchmark~~ 8273681: Add Vector API vs Arrays.mismatch intrinsic benchmark Sep 13, 2021

openjdk bot added the rfr Pull request is ready for review label Sep 13, 2021

PaulSandoz reviewed Sep 13, 2021

View reviewed changes

Update benchmark according to code review

b057780

PaulSandoz approved these changes Sep 16, 2021

View reviewed changes

test/micro/org/openjdk/bench/jdk/incubator/vector/ArrayMismatchBenchmark.java Outdated Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Sep 16, 2021

Fix typo

869e4f2

Co-authored-by: Paul Sandoz <paul.d.sandoz@googlemail.com>

openjdk bot added the sponsor Pull request is ready to be sponsored label Sep 17, 2021

openjdk bot closed this Sep 17, 2021

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Sep 17, 2021

8273681: Add Vector API vs Arrays.mismatch intrinsic benchmark #5459

8273681: Add Vector API vs Arrays.mismatch intrinsic benchmark #5459

amCap1712 commented Sep 10, 2021 •

edited by openjdk bot

bridgekeeper bot commented Sep 10, 2021

openjdk bot commented Sep 10, 2021

PaulSandoz commented Sep 13, 2021

openjdk bot commented Sep 13, 2021

amCap1712 commented Sep 13, 2021

openjdk bot commented Sep 13, 2021

mlbridge bot commented Sep 13, 2021 •

edited

PaulSandoz commented Sep 13, 2021

PaulSandoz left a comment

PaulSandoz Sep 13, 2021

PaulSandoz left a comment

openjdk bot commented Sep 16, 2021 •

edited

amCap1712 commented Sep 17, 2021

openjdk bot commented Sep 17, 2021

PaulSandoz commented Sep 17, 2021

openjdk bot commented Sep 17, 2021

openjdk bot commented Sep 17, 2021

8273681: Add Vector API vs Arrays.mismatch intrinsic benchmark #5459

8273681: Add Vector API vs Arrays.mismatch intrinsic benchmark #5459

Conversation

amCap1712 commented Sep 10, 2021 • edited by openjdk bot

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Sep 10, 2021

openjdk bot commented Sep 10, 2021

PaulSandoz commented Sep 13, 2021

openjdk bot commented Sep 13, 2021

amCap1712 commented Sep 13, 2021

openjdk bot commented Sep 13, 2021

mlbridge bot commented Sep 13, 2021 • edited

Webrevs

PaulSandoz commented Sep 13, 2021

PaulSandoz left a comment

Choose a reason for hiding this comment

PaulSandoz Sep 13, 2021

Choose a reason for hiding this comment

PaulSandoz left a comment

Choose a reason for hiding this comment

openjdk bot commented Sep 16, 2021 • edited

amCap1712 commented Sep 17, 2021

openjdk bot commented Sep 17, 2021

PaulSandoz commented Sep 17, 2021

openjdk bot commented Sep 17, 2021

openjdk bot commented Sep 17, 2021

amCap1712 commented Sep 10, 2021 •

edited by openjdk bot

mlbridge bot commented Sep 13, 2021 •

edited

openjdk bot commented Sep 16, 2021 •

edited