June 2021 - core-libs-dev

RFR: 8266054: VectorAPI rotate operation optimization
by Jatin Bhateja 28 Jul '21

28 Jul '21

Current VectorAPI Java side implementation expresses rotateLeft and rotateRight operation using following operations:- vec1 = lanewise(VectorOperators.LSHL, n) vec2 = lanewise(VectorOperators.LSHR, n) res = lanewise(VectorOperations.OR, vec1 , vec2) This patch moves above handling from Java side to C2 compiler which facilitates dismantling the rotate operation if target ISA does not support a direct rotate instruction. AVX512 added vector rotate instructions vpro[rl][v][dq] which operate over long and integer type vectors. For other cases (i.e. sub-word type vectors or for targets which do not support direct rotate operations ) instruction sequence comprising of vector SHIFT (LEFT/RIGHT) and vector OR is emitted. Please find below the performance data for included JMH benchmark. Machine: Intel(R) Xeon(R) Platinum 8280 CPU @ 2.70GHz (Cascaselake Server) `` Benchmark | (TESTSIZE) | Baseline (ops/min) | Withopt (ops/min) | Gain % -- | -- | -- | -- | -- RotateBenchmark.testRotateLeftI | 64 | 6813.033 | 6936.507 | 1.81 RotateBenchmark.testRotateLeftI | 64 | 11549.524 | 12109.845 | 4.85 RotateBenchmark.testRotateLeftI | 64 | 17780.166 | 18180.083 | 2.25 RotateBenchmark.testRotateLeftI | 128 | 3355.631 | 3426.436 | 2.11 RotateBenchmark.testRotateLeftI | 128 | 5925.534 | 6096.71 | 2.89 RotateBenchmark.testRotateLeftI | 128 | 8847.645 | 8964.224 | 1.32 RotateBenchmark.testRotateLeftI | 256 | 1704.192 | 1779.964 | 4.45 RotateBenchmark.testRotateLeftI | 256 | 2919.158 | 3000.392 | 2.78 RotateBenchmark.testRotateLeftI | 256 | 4386.134 | 4514.211 | 2.92 RotateBenchmark.testRotateLeftL | 64 | 3419.193 | 3500.522 | 2.38 RotateBenchmark.testRotateLeftL | 64 | 5982.87 | 6056.589 | 1.23 RotateBenchmark.testRotateLeftL | 64 | 8829.172 | 8949.157 | 1.36 RotateBenchmark.testRotateLeftL | 128 | 1684.745 | 1784.49 | 5.92 RotateBenchmark.testRotateLeftL | 128 | 2942.226 | 2947.684 | 0.19 RotateBenchmark.testRotateLeftL | 128 | 4385.488 | 4554.113 | 3.85 RotateBenchmark.testRotateLeftL | 256 | 834.195 | 856.939 | 2.73 RotateBenchmark.testRotateLeftL | 256 | 1463.802 | 1551.051 | 5.96 RotateBenchmark.testRotateLeftL | 256 | 2187.005 | 2268.596 | 3.73 RotateBenchmark.testRotateRightI | 64 | 6676.132 | 7077.587 | 6.01 RotateBenchmark.testRotateRightI | 64 | 11452.825 | 11855.45 | 3.52 RotateBenchmark.testRotateRightI | 64 | 17507.286 | 18164.757 | 3.76 RotateBenchmark.testRotateRightI | 128 | 3412.394 | 3519.829 | 3.15 RotateBenchmark.testRotateRightI | 128 | 5905.677 | 5875.698 | -0.51 RotateBenchmark.testRotateRightI | 128 | 8745.95 | 8890.757 | 1.66 RotateBenchmark.testRotateRightI | 256 | 1681.176 | 1708.54 | 1.63 RotateBenchmark.testRotateRightI | 256 | 3004.008 | 3005.606 | 0.05 RotateBenchmark.testRotateRightI | 256 | 4466.633 | 4548.232 | 1.83 RotateBenchmark.testRotateRightL | 64 | 3361.499 | 3461.121 | 2.96 RotateBenchmark.testRotateRightL | 64 | 5873.37 | 6072.209 | 3.39 RotateBenchmark.testRotateRightL | 64 | 8820.284 | 9016.172 | 2.22 RotateBenchmark.testRotateRightL | 128 | 1715.556 | 1720.67 | 0.30 RotateBenchmark.testRotateRightL | 128 | 2957.337 | 3149.193 | 6.49 RotateBenchmark.testRotateRightL | 128 | 4440.468 | 4473.221 | 0.74 RotateBenchmark.testRotateRightL | 256 | 851.391 | 875.371 | 2.82 RotateBenchmark.testRotateRightL | 256 | 1452.422 | 1522.942 | 4.86 RotateBenchmark.testRotateRightL | 256 | 2208.454 | 2263.349 | 2.49 `` ------------- Commit messages: - 8266054: Changing gen-src.sh file permissions - 8266054: VectorAPI rotate operation optimization Changes: https://git.openjdk.java.net/jdk/pull/3720/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3720&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266054 Stats: 1282 lines in 45 files changed: 1201 ins; 58 del; 23 mod Patch: https://git.openjdk.java.net/jdk/pull/3720.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3720/head:pull/3720 PR: https://git.openjdk.java.net/jdk/pull/3720

4 34

RFR: 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream
by Jaikiran Pai 27 Jul '21

27 Jul '21

Can I please get a review for this proposed fix for the issue reported in https://bugs.openjdk.java.net/browse/JDK-8190753? The commit here checks for the size of the zip entry before trying to create a `ByteArrayOutputStream` for that entry's content. A new jtreg test has been included in this commit to reproduce the issue and verify the fix. P.S: It's still a bit arguable whether it's a good idea to create the `ByteArrayOutputStream` with an initial size potentially as large as the `MAX_ARRAY_SIZE` or whether it's better to just use some smaller default value. However, I think that can be addressed separately while dealing with https://bugs.openjdk.java.net/browse/JDK-8011146 ------------- Commit messages: - 8190753: (zipfs): Accessing a large entry (> 2^31 bytes) leads to a negative initial size for ByteArrayOutputStream Changes: https://git.openjdk.java.net/jdk/pull/4607/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4607&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8190753 Stats: 139 lines in 2 files changed: 138 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4607.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4607/head:pull/4607 PR: https://git.openjdk.java.net/jdk/pull/4607

6 34

RFR: 8268873: Unnecessary Vector usage in java.base
by Andrey Turbanov 26 Jul '21

26 Jul '21

Usage of thread-safe collection `Vector` is unnecessary. It's recommended to use `ArrayList` if a thread-safe implementation is not needed. I checked only places where `Vector` was used as local variable. ------------- Commit messages: - [PATCH] Unnecessary Vector usage in java.base - [PATCH] Unnecessary Vector usage in JarIndex.read Changes: https://git.openjdk.java.net/jdk/pull/4482/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4482&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268873 Stats: 18 lines in 3 files changed: 1 ins; 4 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/4482.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4482/head:pull/4482 PR: https://git.openjdk.java.net/jdk/pull/4482

5 10

RFR: 8265474: Dubious 'null' assignment in CompactByteArray.expand
by Andrey Turbanov 26 Jul '21

26 Jul '21

I propose to remove 'null' assignment of field CompactByteArray.values in `expand` method. In the next statement this field is overridden with another value - `tempArray`. This code was there from initial load of OpenJDK sources. I believe it was just leftovers from development phase of this class. There is no practical reason to assign `null` to non-volatile field and then overwrite it with another value. Also I've removed unused method `getArray`. I hope it's ok to cleanup such unused stuff in the same PR. ------------- Commit messages: - 8265474: Dubious 'null' assignment in CompactByteArray.expand - [PATCH] Remove dubious 'null' assignment to CompactByteArray.values Changes: https://git.openjdk.java.net/jdk/pull/1880/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1880&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8265474 Stats: 9 lines in 1 file changed: 0 ins; 6 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/1880.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1880/head:pull/1880 PR: https://git.openjdk.java.net/jdk/pull/1880

3 4

RFR: 8199594: Add doc describing how (?x) ignores spaces in character classes
by Ian Graves 23 Jul '21

23 Jul '21

8199594: Add doc describing how (?x) ignores spaces in character classes ------------- Commit messages: - Updating documentation on comments mode and character class whitespace Changes: https://git.openjdk.java.net/jdk/pull/4618/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4618&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8199594 Stats: 12 lines in 1 file changed: 11 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4618.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4618/head:pull/4618 PR: https://git.openjdk.java.net/jdk/pull/4618

7 13

RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside
by Jaikiran Pai 23 Jul '21

23 Jul '21

Can I please get a review of this proposed fix for https://bugs.openjdk.java.net/browse/JDK-8251329? As noted in that issue, if a zip filesystem created on top of a jar containing a "./" entry is "walked" using the `Files.walkFileTree`, it leads to a infinite never ending iteration (which ultimately fails with Java heap space OOM). Alan notes in that issue that: > This is more likely an issue with the zipfs DirectoryStream implementation. A DirectoryStream is specified to not include elements that for the special links to the current or parent directory. It should be rare. This indeed turned out to be an issue in the `jdk.nio.zipfs.ZipDirectoryStream#iterator()` implementation where it calls the `jdk.nio.zipfs.ZipFileSystem#iteratorOf(...)` implementation. The implementation, unlike what the javadoc of `java.nio.file.DirectoryStream` states, was including the "." and ".." paths in its returned iterator: > The elements returned by the iterator are in no specific order. Some file systems maintain special links to the directory itself and the directory's parent directory. Entries representing these links are not returned by the iterator. The proposed fix in this patch checks the paths for "." and "..", similar to what the `sun.nio.fs.UnixDirectoryStream` does in its `isSelfOrParent` and skips those paths from being added into the returned iterator. The `jdk.nio.zipfs.ZipFileSystem#iteratorOf(...)` (where this change has been done) is currently only used by `jdk.nio.zipfs.ZipDirectoryStream#iterator()` and has package-private visibility, so this change shouldn't impact any other usage/expectations. A new jtreg test has been added to reproduce this issue and verify the fix. Local testing of the `test/jdk/jdk/nio/` (including this new test) went fine without any issues after this change. I will be triggering a `tier1` test locally in a while. ------------- Commit messages: - 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside Changes: https://git.openjdk.java.net/jdk/pull/4604/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4604&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8251329 Stats: 173 lines in 3 files changed: 170 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4604.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4604/head:pull/4604 PR: https://git.openjdk.java.net/jdk/pull/4604

6 19

RFR: 8247373: ArraysSupport.newLength doc, test, and exception message
by Stuart Marks 23 Jul '21

23 Jul '21

This rewrites the doc of ArraysSupport.newLength, adds detail to the exception message, and adds a test. In addition to some renaming and a bit of refactoring of the actual code, I also made two changes of substance to the code: 1. I fixed a problem with overflow checking. In the original code, if oldLength and prefGrowth were both very large (say, Integer.MAX_VALUE), this method could return a negative value. It turns out that writing tests helps find bugs! 2. Under the old policy, if oldLength and minGrowth required a length above SOFT_MAX_ARRAY_LENGTH but not above Integer.MAX_VALUE, this method would return Integer.MAX_VALUE. That doesn't make any sense, because attempting to allocate an array of that length will almost certainly cause the Hotspot to throw OOME because its implementation limit was exceeded. Instead, if the required length is in this range, this method returns that required length. Separately, I'll work on retrofitting various call sites around the JDK to use this method. ------------- Commit messages: - 8247373: ArraysSupport.newLength doc, test, and exception message Changes: https://git.openjdk.java.net/jdk/pull/1617/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1617&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8247373 Stats: 171 lines in 4 files changed: 137 ins; 3 del; 31 mod Patch: https://git.openjdk.java.net/jdk/pull/1617.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1617/head:pull/1617 PR: https://git.openjdk.java.net/jdk/pull/1617

7 31

RFR: 8268974: jpackage fails to handle --dest option containing "bin" folder
by Alexey Semenyuk 22 Jul '21

22 Jul '21

GetApplicationHomeFromDll() fails if the path to libjli.so contains "bin" component (/tmp/bin/HelloWorld/lib/runtime/lib/libjli.so). TruncatePath() looks for "/bin/" first in "/tmp/bin/HelloWorld/lib/runtime/lib/libjli.so" string and then it looks for "/lib/". But this is wrong order as it should look for "/lib/" first. I.e. TruncatePath() should look for "/bin/" and then for "/lib/" if called from GetApplicationHome() and for "/lib/" first and then for "/bin/" if called from GetApplicationHomeFromDll(). ------------- Commit messages: - 8268974: jpackage fails to handle --dest option containing "bin" folder Changes: https://git.openjdk.java.net/jdk/pull/4534/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4534&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268974 Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/4534.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4534/head:pull/4534 PR: https://git.openjdk.java.net/jdk/pull/4534

4 12

[jdk17] RFR: 8269240: java/foreign/stackwalk/TestAsyncStackWalk.java test failed with concurrent GC
by Jorn Vernee 17 Jul '21

17 Jul '21

This patch rewrites the prologue and epilogue of panama upcalls, in order to fix the test failure from the title. Previously, we did a call to potentially attach the current thread to the VM, and then afterwards did the same suspend and stack reguard checks that we do on the back-edge of a native downcall. Then, on the back edge of the upcall we did another conditional call to detach the thread. I've changed these 2 calls to mimic what is done by JavaCallWrapper instead (with attach and detach included), and removed the old suspend and stack reguard checks (now handled by the call). FWIW, this removes the JavaFrameAnchor save/restore MacroAssembler code. This is now written in C++. Also, MacroAssembler code was added to save/restore the result of the upcall around the call on the back-edge, which was previously missing. Since the new code allocates a handle block as well, I've added handling for those oops to frame & OptimizedUpcallBlob. Testing: local running of `jdk_foreign` on Windows and Linux (WSL). Tier 1-3 ------------- Commit messages: - Remove whitespace - Add Shenandoah test case as well - Add zgc requires tags - Typo in MACOS_AARCH64_ONLY part - Polish - Some more build fixes - build fixes - re-write upcall code to mimic JavaCallWrapper Changes: https://git.openjdk.java.net/jdk17/pull/149/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17&pr=149&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269240 Stats: 516 lines in 18 files changed: 338 ins; 147 del; 31 mod Patch: https://git.openjdk.java.net/jdk17/pull/149.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/149/head:pull/149 PR: https://git.openjdk.java.net/jdk17/pull/149

5 25

RFR: 8266936: Add a finalization JFR event
by Brent Christian 14 Jul '21

14 Jul '21

Please review this enhancement to add a new JFR event, generated whenever a finalizer is run. (The makeup is similar to the Deserialization event, [JDK-8261160](https://bugs.openjdk.java.net/browse/JDK-8261160).) The event's only datum (beyond those common to all jfr events) is the class of the object that was finalized. The Category for the event: `"Java Virtual Machine" / "GC" / "Finalization"` is what made sense to me, even though the event is generated from library code. Along with the new regtest, I added a run mode to the basic finalizer test to enable jfr. Automated testing looks good so far. Thanks, -Brent ------------- Commit messages: - Fix jcheck some more - Fix jcheck - Merge branch 'master' into 8266936 - Add jfr mode to test/jdk/java/lang/ref/FinalizeOverride.java - Fix spacing in new regtest - Remove extraneous comments from new regtest - Removed unintentional changes from FinalizeOverride.java - Actually check in new regtest - Update TestActiveSettingEvent.java - Update TestDefaultConfigurations.java and .jfc files - ... and 2 more: https://git.openjdk.java.net/jdk/compare/e6705c0e...200268ab Changes: https://git.openjdk.java.net/jdk/pull/4101/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4101&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266936 Stats: 191 lines in 11 files changed: 190 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4101.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4101/head:pull/4101 PR: https://git.openjdk.java.net/jdk/pull/4101

7 12