8331934: [s390x] Add support for primitive array C1 clone intrinsic
Adds JDK-8302850 Port for s390x.
Testing:
make test TEST="hotspot_compiler" JTREG="JAVA_OPTIONS=-XX:TieredStopAtLevel=1"
==============================
Test summary
==============================
TEST TOTAL PASS FAIL ERROR
jtreg:test/hotspot/jtreg:hotspot_compiler 1166 1166 0 0
==============================
TEST SUCCESS
* Tier1 Test with Fast debug build.
BenchMarking:
Without Patch:
Benchmark (size) Mode Cnt Score Error Units
ArrayClone.byteArraycopy 0 avgt 15 10.838 ± 0.461 ns/op
ArrayClone.byteArraycopy 10 avgt 15 28.919 ± 1.695 ns/op
ArrayClone.byteArraycopy 100 avgt 15 48.815 ± 0.901 ns/op
ArrayClone.byteArraycopy 1000 avgt 15 256.357 ± 7.901 ns/op
ArrayClone.byteClone 0 avgt 15 90.398 ± 3.119 ns/op
ArrayClone.byteClone 10 avgt 15 103.774 ± 4.468 ns/op
ArrayClone.byteClone 100 avgt 15 126.628 ± 6.952 ns/op
ArrayClone.byteClone 1000 avgt 15 326.409 ± 31.635 ns/op
ArrayClone.intArraycopy 0 avgt 15 10.450 ± 0.509 ns/op
ArrayClone.intArraycopy 10 avgt 15 36.903 ± 0.753 ns/op
ArrayClone.intArraycopy 100 avgt 15 85.964 ± 1.806 ns/op
ArrayClone.intArraycopy 1000 avgt 15 841.512 ± 40.335 ns/op
ArrayClone.intClone 0 avgt 15 89.332 ± 3.695 ns/op
ArrayClone.intClone 10 avgt 15 110.639 ± 2.476 ns/op
ArrayClone.intClone 100 avgt 15 195.781 ± 8.622 ns/op
ArrayClone.intClone 1000 avgt 15 1058.479 ± 92.468 ns/op
Finished running test 'micro:java.lang.ArrayClone'
with patch:
Benchmark (size) Mode Cnt Score Error Units
ArrayClone.byteArraycopy 0 avgt 15 10.526 ± 0.289 ns/op
ArrayClone.byteArraycopy 10 avgt 15 27.110 ± 0.656 ns/op
ArrayClone.byteArraycopy 100 avgt 15 49.872 ± 1.562 ns/op
ArrayClone.byteArraycopy 1000 avgt 15 269.518 ± 4.567 ns/op
ArrayClone.byteClone 0 avgt 15 10.766 ± 0.899 ns/op
ArrayClone.byteClone 10 avgt 15 18.341 ± 0.394 ns/op
ArrayClone.byteClone 100 avgt 15 40.986 ± 0.674 ns/op
ArrayClone.byteClone 1000 avgt 15 227.512 ± 7.643 ns/op
ArrayClone.intArraycopy 0 avgt 15 10.320 ± 0.294 ns/op
ArrayClone.intArraycopy 10 avgt 15 36.557 ± 0.860 ns/op
ArrayClone.intArraycopy 100 avgt 15 89.837 ± 2.364 ns/op
ArrayClone.intArraycopy 1000 avgt 15 836.678 ± 27.920 ns/op
ArrayClone.intClone 0 avgt 15 10.043 ± 0.216 ns/op
ArrayClone.intClone 10 avgt 15 29.149 ± 0.723 ns/op
ArrayClone.intClone 100 avgt 15 88.046 ± 2.211 ns/op
ArrayClone.intClone 1000 avgt 15 840.163 ± 58.748 ns/op
Finished running test 'micro:java.lang.ArrayClone'
Progress
- [x] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
Issue
- JDK-8331934: [s390x] Add support for primitive array C1 clone intrinsic (Enhancement - P4)
Reviewers
- Martin Doerr (@TheRealMDoerr - Reviewer)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19220/head:pull/19220
$ git checkout pull/19220
Update a local copy of the PR:
$ git checkout pull/19220
$ git pull https://git.openjdk.org/jdk.git pull/19220/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 19220
View PR using the GUI difftool:
$ git pr show -t 19220
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19220.diff
Webrev
:wave: Welcome back amitkumar! A progress list of the required criteria for merging this PR into pr/17667 will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
@offamitkumar 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:
8331934: [s390x] Add support for primitive array C1 clone intrinsic
Reviewed-by: mdoerr, sjayagond
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 67 new commits pushed to the master branch:
- 9bfae8891e6efa58c557bd6dac61de111a16f71e: 8332297: annotation processor that generates records sometimes fails due to NPE in javac
- 4e169d1ed7501d1de8fd4ea326f84b6c1a34270d: 8332401: G1: TestFromCardCacheIndex.java with -XX:GCCardSizeInBytes=128 triggers underflow assertion
- 7ffc9997bd4a93cefe30f672a5f0e9c49215d2c7: 8332498: [aarch64, x86] improving OpToAssembly output for partialSubtypeCheckConstSuper Instruct
- e529101ea30b49a6601088ce5ab81df590fc52f0: 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed as argument 1, which is declared to never be null
- 414a7fdc5e4aae4cec25b0847bb7c163f271b4e0: 8311175: Move BufWriter::asByteBuffer to BufWriterImpl
- 451cc239050f097060be927171fe0e46962f3356: 8332486: ClassFile API ArrayIndexOutOfBoundsException with label metadata
- 5f2b8d0224868d09ff54e93fabe4a6db177aef8f: 8332448: Make SpaceMangler inherit AllStatic
- 8a49d47cf3e845ddccaaeafeee9dfe6ab3180ded: 8332462: ubsan: c1_ValueStack.hpp:229:49: runtime error: load of value 171, which is not a valid value for type 'bool'
- ce99198e3a6dc81865c518b1fe4a67e93b8ebdd1: 8332181: Deprecate for removal the MulticastSocket.send(DatagramPacket, byte) and setTTL/getTTL methods on DatagramSocketImpl and MulticastSocket
- f5ab7dff402a3152f5d5736cc6521b4be617eccf: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\A\Z' missing from stderr
- ... and 57 more: https://git.openjdk.org/jdk/compare/2f10a316ff0c5a4c124b94f6fabb38fb119d2c82...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.
@offamitkumar The following label will be automatically applied to this pull request:
hotspot-compiler
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.
@RealLucy @TheRealMDoerr Would you please review this one. :-)
Testing seems clear on s390x. I have posted Benchmark result as well.
Please let me know if any further testing is required.
The parent pull request that this pull request depends on has now been integrated and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork:
git checkout c1_clone_intr
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# if there are conflicts, follow the instructions given by git merge
git commit -m "Merge master"
git push
@offamitkumar this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:
git checkout c1_clone_intr
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push
Result from the LPAR:
without patch:
Benchmark (size) Mode Cnt Score Error Units
ArrayClone.byteArraycopy 0 avgt 15 4.021 ± 0.032 ns/op
ArrayClone.byteArraycopy 10 avgt 15 15.931 ± 0.171 ns/op
ArrayClone.byteArraycopy 100 avgt 15 16.201 ± 0.076 ns/op
ArrayClone.byteArraycopy 1000 avgt 15 45.268 ± 0.213 ns/op
ArrayClone.byteClone 0 avgt 15 70.300 ± 0.244 ns/op
ArrayClone.byteClone 10 avgt 15 77.112 ± 0.558 ns/op
ArrayClone.byteClone 100 avgt 15 79.860 ± 0.606 ns/op
ArrayClone.byteClone 1000 avgt 15 112.834 ± 0.526 ns/op
ArrayClone.intArraycopy 0 avgt 15 4.007 ± 0.012 ns/op
ArrayClone.intArraycopy 10 avgt 15 15.378 ± 0.055 ns/op
ArrayClone.intArraycopy 100 avgt 15 25.387 ± 0.102 ns/op
ArrayClone.intArraycopy 1000 avgt 15 161.278 ± 0.719 ns/op
ArrayClone.intClone 0 avgt 15 70.341 ± 0.265 ns/op
ArrayClone.intClone 10 avgt 15 78.209 ± 0.514 ns/op
ArrayClone.intClone 100 avgt 15 89.845 ± 0.571 ns/op
ArrayClone.intClone 1000 avgt 15 257.037 ± 2.809 ns/op
Finished running test 'micro:java.lang.ArrayClone'
with patch:
Benchmark (size) Mode Cnt Score Error Units
ArrayClone.byteArraycopy 0 avgt 15 4.021 ± 0.027 ns/op
ArrayClone.byteArraycopy 10 avgt 15 16.106 ± 0.859 ns/op
ArrayClone.byteArraycopy 100 avgt 15 16.212 ± 0.045 ns/op
ArrayClone.byteArraycopy 1000 avgt 15 45.147 ± 0.137 ns/op
ArrayClone.byteClone 0 avgt 15 3.570 ± 0.010 ns/op
ArrayClone.byteClone 10 avgt 15 6.033 ± 0.018 ns/op
ArrayClone.byteClone 100 avgt 15 6.868 ± 0.020 ns/op
ArrayClone.byteClone 1000 avgt 15 33.437 ± 0.114 ns/op
ArrayClone.intArraycopy 0 avgt 15 4.008 ± 0.010 ns/op
ArrayClone.intArraycopy 10 avgt 15 15.373 ± 0.044 ns/op
ArrayClone.intArraycopy 100 avgt 15 29.543 ± 3.687 ns/op
ArrayClone.intArraycopy 1000 avgt 15 161.554 ± 0.414 ns/op
ArrayClone.intClone 0 avgt 15 3.571 ± 0.010 ns/op
ArrayClone.intClone 10 avgt 15 6.184 ± 0.016 ns/op
ArrayClone.intClone 100 avgt 15 13.304 ± 0.043 ns/op
ArrayClone.intClone 1000 avgt 15 133.755 ± 0.362 ns/op
Finished running test 'micro:java.lang.ArrayClone'
@RealLucy @TheRealMDoerr Would you please review this one. :-)
pinging you again, if you got bandwidth then please review it.
@galderz if possible can you review this ?
Maybe this could ease you a bit while review: Testing for {tier1} X {fastdebug, slowdebug, release} and {tier1 -XX:TieredStopAtLevel=1} X {fastdebug, slowdebug, release} was clean. Benchmarking also shows that we are good on this. You can check the result here :-)
@offamitkumar I'm no s390 expert, so I can only do a light review on the code. The changes look good to me and the benchmark results show improvements. One thing I would suggest is maybe expanding the testing a bit, e.g. hotspot_compiler, hotspot_gc, hotspot_serviceability, hotspot_runtime, and tier1-3 see https://github.com/openjdk/jdk/pull/17667#issuecomment-2071741244 for further details.
Ran this command: make test TEST="hotspot_compiler hotspot_gc hotspot_serviceability hotspot_runtime tier1 tier2 tier3" JTREG="JAVA_OPTIONS=-XX:TieredStopAtLevel=1 -Xcomp" and didn't see any new failure appearing due to my changes.
Thanks Martin & Sid for approval ;-)
/integrate
Going to push as commit ae9ad862ee54e119553efec919f1061dca36b954.
Since your change was applied there have been 68 commits pushed to the master branch:
- 3479b46c5bea3afd92b6ab4acd2fe7f274df38aa: 8332595: Serial: Remove unused TenuredGeneration::should_collect
- 9bfae8891e6efa58c557bd6dac61de111a16f71e: 8332297: annotation processor that generates records sometimes fails due to NPE in javac
- 4e169d1ed7501d1de8fd4ea326f84b6c1a34270d: 8332401: G1: TestFromCardCacheIndex.java with -XX:GCCardSizeInBytes=128 triggers underflow assertion
- 7ffc9997bd4a93cefe30f672a5f0e9c49215d2c7: 8332498: [aarch64, x86] improving OpToAssembly output for partialSubtypeCheckConstSuper Instruct
- e529101ea30b49a6601088ce5ab81df590fc52f0: 8332473: ubsan: growableArray.hpp:290:10: runtime error: null pointer passed as argument 1, which is declared to never be null
- 414a7fdc5e4aae4cec25b0847bb7c163f271b4e0: 8311175: Move BufWriter::asByteBuffer to BufWriterImpl
- 451cc239050f097060be927171fe0e46962f3356: 8332486: ClassFile API ArrayIndexOutOfBoundsException with label metadata
- 5f2b8d0224868d09ff54e93fabe4a6db177aef8f: 8332448: Make SpaceMangler inherit AllStatic
- 8a49d47cf3e845ddccaaeafeee9dfe6ab3180ded: 8332462: ubsan: c1_ValueStack.hpp:229:49: runtime error: load of value 171, which is not a valid value for type 'bool'
- ce99198e3a6dc81865c518b1fe4a67e93b8ebdd1: 8332181: Deprecate for removal the MulticastSocket.send(DatagramPacket, byte) and setTTL/getTTL methods on DatagramSocketImpl and MulticastSocket
- ... and 58 more: https://git.openjdk.org/jdk/compare/2f10a316ff0c5a4c124b94f6fabb38fb119d2c82...master
Your commit was automatically rebased without conflicts.
@offamitkumar Pushed as commit ae9ad862ee54e119553efec919f1061dca36b954.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.