Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Mon, 3 Jun 2024 14:38:03 GMT, Alexey Semenyuk wrote: >> I am yet to see anything that actually explains the cause of the `dlerror` >> crash here ??? > > @dholmes-ora there is no fix for the cause of the `dlerror` crash in this PR. > The PR fixes jpackage tests to rerun a launcher if it crashes. This > workaround for jpackage tests was first introduced in > [JDK-8269403](https://bugs.openjdk.org/browse/JDK-8269403) fix but > MainClassTest was left unfixed back then. This PR complements > [JDK-8269403](https://bugs.openjdk.org/browse/JDK-8269403). @alexeysemenyukoracle But now there is no bug to investigate the dlerrror crash! If you wanted to make the test more robust a new JBS issue should have been filed for that. - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2146669916
Integrated: 8332547: Unloaded signature classes in DirectMethodHandles
On Mon, 20 May 2024 21:29:20 GMT, Vladimir Ivanov wrote: > JVM routinely installs loader constraints for unloaded signature classes when > method resolution takes place. MethodHandle resolution took a different route > and eagerly resolves signature classes instead (see > `java.lang.invoke.MemberName$Factory::resolve` and > `sun.invoke.util.VerifyAccess::isTypeVisible` for details). > > There's a micro-optimization which bypasses eager resolution for `java.*` > classes. The downside is that `java.*` signature classes can show up as > unloaded. It manifests as inlining failures during JIT-compilation and may > cause severe performance issues. > > Proposed fix removes the aforementioned special case logic during > `MethodHandle` resolution. > > In some cases it may slow down `MethodHandle` construction a bit (e.g., when > repeatedly constructing `DirectMethodHandle`s with lots of arguments), but > `MethodHandle` construction step is not performance critical. > > Testing: hs-tier1 - hs-tier4 This pull request has now been integrated. Changeset: 29e10e45 Author:Vladimir Ivanov URL: https://git.openjdk.org/jdk/commit/29e10e4582c1a844a6db4c42ba01bd1d6d4dfd52 Stats: 83 lines in 4 files changed: 68 ins; 0 del; 15 mod 8332547: Unloaded signature classes in DirectMethodHandles Reviewed-by: jvernee, liach - PR: https://git.openjdk.org/jdk/pull/19319
Re: RFR: 8332547: Unloaded signature classes in DirectMethodHandles [v2]
On Mon, 3 Jun 2024 19:36:58 GMT, Vladimir Ivanov wrote: >> JVM routinely installs loader constraints for unloaded signature classes >> when method resolution takes place. MethodHandle resolution took a different >> route and eagerly resolves signature classes instead (see >> `java.lang.invoke.MemberName$Factory::resolve` and >> `sun.invoke.util.VerifyAccess::isTypeVisible` for details). >> >> There's a micro-optimization which bypasses eager resolution for `java.*` >> classes. The downside is that `java.*` signature classes can show up as >> unloaded. It manifests as inlining failures during JIT-compilation and may >> cause severe performance issues. >> >> Proposed fix removes the aforementioned special case logic during >> `MethodHandle` resolution. >> >> In some cases it may slow down `MethodHandle` construction a bit (e.g., when >> repeatedly constructing `DirectMethodHandle`s with lots of arguments), but >> `MethodHandle` construction step is not performance critical. >> >> Testing: hs-tier1 - hs-tier4 > > Vladimir Ivanov has updated the pull request incrementally with one > additional commit since the last revision: > > Renaming: isTypeVisible -> ensureTypeVisible Thanks for the reviews, Jorn and Chen. - PR Comment: https://git.openjdk.org/jdk/pull/19319#issuecomment-2146542294
Re: RFR: 8319457: Update jpackage to support WiX Toolset 4 on Windows
On Tue, 4 Jun 2024 01:29:15 GMT, Alexey Semenyuk wrote: > > Is there plan to add support for WiX 5? > > This patch supports WiX5 as well. There is not much difference between WiX4 > and WiX5 from jpackage perspective. I'll update the description I assume WiX5 will just work if installed instead of WiX4? Do you think it will make sense to introduce Wix5 ToolsetType in case if we need to do something different between 4 and 5? - PR Comment: https://git.openjdk.org/jdk/pull/19318#issuecomment-2146460825
RFR: 8333462: Performance regression of new DecimalFormat() when compare to jdk11
Run the below benchmark test ,it show the average time of new DecimalFormat() increase 18% when compare to jdk 11. the result with jdk 11: Benchmark Mode CntScore Error Units JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op the result with current jdk: Benchmark Mode CntScore Error Units JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op @BenchmarkMode(Mode.AverageTime) @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS) @State(Scope.Thread) @OutputTimeUnit(TimeUnit.NANOSECONDS) public class JmhDecimalFormat { @Setup(Level.Trial) public void setup() { } @Benchmark public void testNewOnly() throws InterruptedException { new DecimalFormat("#0.0"); } } Compare the flame graph it shows the java.text.DecimalFormatSymbols#findNonFormatChar takes a significant time. After replacing the lambda implementation with a simple loop , it shows nearly the same performance as jdk 11. Benchmark Mode CntScore Error Units JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op [flame-graph-jdk11-jdk21.zip](https://github.com/user-attachments/files/15541764/flame-graph-jdk11-jdk21.zip) - Commit messages: - 8333462: Performance regression of new DecimalFormat() when compare to jdk11 Changes: https://git.openjdk.org/jdk/pull/19534/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19534=00 Issue: https://bugs.openjdk.org/browse/JDK-8333462 Stats: 6 lines in 1 file changed: 2 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/19534.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19534/head:pull/19534 PR: https://git.openjdk.org/jdk/pull/19534
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v3]
> ### Performance regression of DecimalFormat.format > From the output of perf, we can see the hottest regions contain atomic > instructions. But when run with JDK 11, there is no such problem. The reason > is the removed biased locking. > The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself > contains many synchronized methods. > So I added support for some new methods that accept StringBuilder which is > lock-free. > > ### Test result > > @BenchmarkMode(Mode.AverageTime) > @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) > @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS) > @State(Scope.Thread) > @OutputTimeUnit(TimeUnit.NANOSECONDS) > public class JmhDecimalFormat { > > private DecimalFormat format; > > @Setup(Level.Trial) > public void setup() { > format = new DecimalFormat("#0.0"); > } > > @Benchmark > public void testNewAndFormat() throws InterruptedException { > new DecimalFormat("#0.0").format(9524234.1236457); > } > > @Benchmark > public void testNewOnly() throws InterruptedException { > new DecimalFormat("#0.0"); > } > > @Benchmark > public void testFormatOnly() throws InterruptedException { > format.format(9524234.1236457); > } > } > > Current JDK before optimize > > Benchmark Mode CntScore Error Units > JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op > JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op > JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op > > > > Current JDK after optimize > > Benchmark Mode CntScore Error Units > JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op > JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op > JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op > > > ### JDK 11 > > Benchmark Mode CntScore Error Units > JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op > JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op > JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op lingjun-cg has updated the pull request incrementally with one additional commit since the last revision: 896: Performance regression of DecimalFormat.format - Changes: - all: https://git.openjdk.org/jdk/pull/19513/files - new: https://git.openjdk.org/jdk/pull/19513/files/b48962b5..0a581428 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19513=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19513=01-02 Stats: 6 lines in 1 file changed: 0 ins; 2 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/19513.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513 PR: https://git.openjdk.org/jdk/pull/19513
Integrated: 8333398: Uncomment the commented test in test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java
On Mon, 3 Jun 2024 04:25:38 GMT, Jaikiran Pai wrote: > Can I please get a review of this test-only change which uncomments an > additional test that was commented out in > `test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java`? > > As noted in https://bugs.openjdk.org/browse/JDK-898, the original issue > due to which this line was commented out in the test, has been fixed several > releases back and thus this line can now be uncommented. > > I've verified that this test continues to pass with these changes. This pull request has now been integrated. Changeset: d230b303 Author:Jaikiran Pai URL: https://git.openjdk.org/jdk/commit/d230b30353f59135287436b09949b80e9fd73a93 Stats: 5 lines in 1 file changed: 0 ins; 3 del; 2 mod 898: Uncomment the commented test in test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java Reviewed-by: iris, lancea - PR: https://git.openjdk.org/jdk/pull/19514
Re: RFR: 8333398: Uncomment the commented test in test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java
On Mon, 3 Jun 2024 04:25:38 GMT, Jaikiran Pai wrote: > Can I please get a review of this test-only change which uncomments an > additional test that was commented out in > `test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java`? > > As noted in https://bugs.openjdk.org/browse/JDK-898, the original issue > due to which this line was commented out in the test, has been fixed several > releases back and thus this line can now be uncommented. > > I've verified that this test continues to pass with these changes. Thank you Iris and Lance for the reviews. > We should look at at a later date to convert this to junit and when we do, > look to rework isMultiReleaseJar() as it actually runs multiple tests within > the test itself and if one fails, the rest will not be run I have added it to my TODO to update this test in future. - PR Comment: https://git.openjdk.org/jdk/pull/19514#issuecomment-2146416470
Re: RFR: 8333396: Performance regression of new DecimalFormat and DecimalFormat.format [v2]
On Mon, 3 Jun 2024 06:13:35 GMT, lingjun-cg wrote: >> ### Performance regression of DecimalFormat.format >> From the output of perf, we can see the hottest regions contain atomic >> instructions. But when run with JDK 11, there is no such problem. The >> reason is the removed biased locking. >> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself >> contains many synchronized methods. >> So I added support for some new methods that accept StringBuilder which is >> lock-free. >> >> ### Performance regression of new DecimalFormat >> After comparing the flame graph between current jdk and jdk 11, the method >> java.text.DecimalFormatSymbols#findNonFormatChar takes a significant time. >> The performance becomes as good as jdk11 after replacing it with a simple >> loop implementation. >> >> >> >> ### Test result >> >> @BenchmarkMode(Mode.AverageTime) >> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) >> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS) >> @State(Scope.Thread) >> @OutputTimeUnit(TimeUnit.NANOSECONDS) >> public class JmhDecimalFormat { >> >> private DecimalFormat format; >> >> @Setup(Level.Trial) >> public void setup() { >> format = new DecimalFormat("#0.0"); >> } >> >> @Benchmark >> public void testNewAndFormat() throws InterruptedException { >> new DecimalFormat("#0.0").format(9524234.1236457); >> } >> >> @Benchmark >> public void testNewOnly() throws InterruptedException { >> new DecimalFormat("#0.0"); >> } >> >> @Benchmark >> public void testFormatOnly() throws InterruptedException { >> format.format(9524234.1236457); >> } >> } >> >> Current JDK before optimize >> >> Benchmark Mode CntScore Error Units >> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op >> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op >> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op >> >> >> >> Current JDK after optimize >> >> Benchmark Mode CntScore Error Units >> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op >> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op >> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op >> >> >> ### JDK 11 >> >> Benchmark Mode CntScore Error Units >> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op >> JmhDecimalFormat.testNewAndForma... > > lingjun-cg has updated the pull request incrementally with one additional > commit since the last revision: > > 896: Performance regression of new DecimalFormat and > DecimalFormat.format Hi naotaoj, What I mean "performance regression" is compare to JDK 11. We have an server side application that use DecimalFormat.format API seriously. When migrate it from JDK 11 to JDK 21, we found a performance degradation. So I write the JMH test case "JmhDecimalFormat". It show that there a performance regression since JDK 21. These are the perfasm output for running JMH on both JDK 11 and JDK21. There are some hot regions around the atomic instructions in JDK 21, but no such problem in JDK 11. [jdk11.txt](https://github.com/user-attachments/files/15541278/jdk11.txt) [jdk21.txt](https://github.com/user-attachments/files/15541279/jdk21.txt) Maybe the [JEP 374: Deprecate and Disable Biased Locking](https://openjdk.org/jeps/374) is the reason? So I run the benchmark on JDK 11 again but with option '-XX:-UseBiasedLocking', there only a minor gap between jdk11 and jdk 21. OK, return to my patch. java.text use StringBuffer internally, but nearly all methods in StringBuffer are synchronized: @Override public synchronized StringBuffer append(Object obj) { } @Override @IntrinsicCandidate public synchronized StringBuffer append(String str) { ... } >From the above analysis, the atomic instructions slow down >DecimalFormat.format, and StringBuffer's synchronized methods generate there >atomic instructions. So If remove these synchronized methods, it will get a >better performance. So I replace StringBuffer with StringBuilder in java.text.NumberFormat. public final String format(double number) { // Use fast-path for double result if that works String result = fastFormat(number); if (result != null) return result; -return format(number, new StringBuffer(), +return format(number, new StringBuilder(), DontCareFieldPosition.INSTANCE).toString(); } @@ -367,7 +367,7 @@ public final String format(double number) { * @see java.text.Format#format */ public final String format(long number) { - return format(number, new StringBuffer(), + return
Re: RFR: 8319457: Update jpackage to support WiX Toolset 4 on Windows
On Tue, 4 Jun 2024 01:24:15 GMT, Alexander Matveev wrote: > Is there plan to add support for WiX 5? This patch supports WiX5 as well. There is not much difference between WiX4 and WiX5 from jpackage perspective. I'll update the description - PR Comment: https://git.openjdk.org/jdk/pull/19318#issuecomment-2146395368
Re: RFR: 8326951: since-checker - missing @ since tags [v9]
On Mon, 13 May 2024 20:39:14 GMT, Nizar Benalla wrote: >> I added `@since` tags for methods/constructors that do not match the >> `@since` of the enclosing class. >> >> The `write` method already existed in `PrintStream` in earlier versions and >> instances of it could always call this method, since it extends >> `FilterOutputStream` [which has the >> method](https://github.com/openjdk/jdk6/blob/3e49aa876353eaa215cde71eb21acc9b7f9872a0/jdk/src/share/classes/java/io/FilterOutputStream.java#L96). >> >> for `MappedByteBuffer slice()` and `MappedByteBuffer slice(int index, int >> length)`, the return type changed from `ByteBuffer ` to `MappedByteBuffer`. >> And the checker tool differentiates between them because of that. >> >> This is similar to #18032 and #18373 >> >> For context, I am writing tests to check for accurate use of `@since` tags >> in documentation comments in source code. >> We're following these rules for now: >> >> ### Rule 1: Introduction of New Elements >> >> - If an element is new in JDK N, with no equivalent in JDK N-1, it must >> include `@since N`. >> - Exception: Member elements (fields, methods, nested classes) may omit >> `@since` if their version matches the value specified for the enclosing >> class or interface. >> >> ### Rule 2: Existing Elements in Subsequent JDK Versions >> >> - If an element exists in JDK N, with an equivalent in JDK N-1, it should >> not include `@since N`. >> >> ### Rule 3: Handling Missing `@since` Tags in methods if there is no `@since` >> >> - When inspecting methods, prioritize the `@since` annotation of the >> supertype's overridden method. >> - If unavailable or if the enclosing class's `@since` is newer, use the >> enclosing element's `@since`. >> >> I.e. if A extends B, and we add a method to B in JDK N, and add an >> override of the method to A in JDK M (M > N), we will use N as the effective >> `@since` for the method. > > Nizar Benalla has updated the pull request incrementally with one additional > commit since the last revision: > > empty commit and merge Here is a snippet from the report of the checker with the relevant bits for this PR STDERR: method: void java.io.PrintStream.write(byte[]): `@since` version is 14 but the element exists before JDK 10 method: java.lang.invoke.MethodHandle java.lang.invoke.MethodHandles.tableSwitch(java.lang.invoke.MethodHandle,java.lang.invoke.MethodHandle[]): `@since` version is 9 instead of 17 method: void java.lang.reflect.MalformedParameterizedTypeException.(java.lang.String): `@since` version is 9 instead of 10 method: java.nio.MappedByteBuffer java.nio.MappedByteBuffer.slice(): `@since` version is 9 instead of 17 method: java.nio.MappedByteBuffer java.nio.MappedByteBuffer.slice(int,int): `@since` version is 9 instead of 17 method: java.nio.MappedByteBuffer java.nio.MappedByteBuffer.duplicate(): `@since` version is 9 instead of 17 method: java.nio.MappedByteBuffer java.nio.MappedByteBuffer.compact(): `@since` version is 9 instead of 17 method: void java.util.Properties.(int): `@since` version is 9 instead of 10 method: float java.util.concurrent.ThreadLocalRandom.nextFloat(float): `@since` version is 9 instead of 17 method: float java.util.concurrent.ThreadLocalRandom.nextFloat(float,float): `@since` version is 9 instead of 17 method: void java.util.zip.Deflater.setDictionary(java.nio.ByteBuffer): `@since` version is 9 instead of 11 Here is the full report STDERR: method: void java.io.PrintStream.write(byte[]): `@since` version is 14 but the element exists before JDK 10 method: byte[] java.io.FileInputStream.readNBytes(int): `@since` version is 9 instead of 11 method: java.lang.classfile.Signature.ClassTypeSig java.lang.classfile.ClassSignature.superclassSignature(): `@since` version is 22 instead of 23 method: java.lang.classfile.constantpool.PoolEntry java.lang.classfile.ClassReader.readEntryOrNull(int,java.lang.Class): `@since` version is 24 instead of 23 method: java.lang.classfile.constantpool.PoolEntry java.lang.classfile.constantpool.ConstantPool.entryByIndex(int,java.lang.Class): `@since` version is 24 instead of 23 method: java.lang.Class java.lang.constant.ClassDesc.resolveConstantDesc(java.lang.invoke.MethodHandles.Lookup): `@since` version is 12 instead of 21 method: java.lang.invoke.MethodType java.lang.constant.MethodTypeDesc.resolveConstantDesc(java.lang.invoke.MethodHandles.Lookup): `@since` version is 12 instead of 21 method: java.lang.invoke.MethodHandle java.lang.constant.MethodHandleDesc.resolveConstantDesc(java.lang.invoke.MethodHandles.Lookup): `@since` version is 12 instead of 21 method: long java.lang.foreign.MemorySegment.maxByteAlignment(): `@since` version is 22 instead of 23 method: java.lang.invoke.MethodHandle java.lang.invoke.MethodHandles.tableSwitch(java.lang.invoke.MethodHandle,java.lang.invoke.MethodHandle[]): `@since` version is 9 instead of 17 method: java.lang.Object
Re: RFR: 8333396: Performance regression of new DecimalFormat and DecimalFormat.format [v2]
On Mon, 3 Jun 2024 06:13:35 GMT, lingjun-cg wrote: >> ### Performance regression of DecimalFormat.format >> From the output of perf, we can see the hottest regions contain atomic >> instructions. But when run with JDK 11, there is no such problem. The >> reason is the removed biased locking. >> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself >> contains many synchronized methods. >> So I added support for some new methods that accept StringBuilder which is >> lock-free. >> >> ### Performance regression of new DecimalFormat >> After comparing the flame graph between current jdk and jdk 11, the method >> java.text.DecimalFormatSymbols#findNonFormatChar takes a significant time. >> The performance becomes as good as jdk11 after replacing it with a simple >> loop implementation. >> >> >> >> ### Test result >> >> @BenchmarkMode(Mode.AverageTime) >> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) >> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS) >> @State(Scope.Thread) >> @OutputTimeUnit(TimeUnit.NANOSECONDS) >> public class JmhDecimalFormat { >> >> private DecimalFormat format; >> >> @Setup(Level.Trial) >> public void setup() { >> format = new DecimalFormat("#0.0"); >> } >> >> @Benchmark >> public void testNewAndFormat() throws InterruptedException { >> new DecimalFormat("#0.0").format(9524234.1236457); >> } >> >> @Benchmark >> public void testNewOnly() throws InterruptedException { >> new DecimalFormat("#0.0"); >> } >> >> @Benchmark >> public void testFormatOnly() throws InterruptedException { >> format.format(9524234.1236457); >> } >> } >> >> Current JDK before optimize >> >> Benchmark Mode CntScore Error Units >> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op >> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op >> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op >> >> >> >> Current JDK after optimize >> >> Benchmark Mode CntScore Error Units >> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op >> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op >> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op >> >> >> ### JDK 11 >> >> Benchmark Mode CntScore Error Units >> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op >> JmhDecimalFormat.testNewAndForma... > > lingjun-cg has updated the pull request incrementally with one additional > commit since the last revision: > > 896: Performance regression of new DecimalFormat and > DecimalFormat.format Hi, Can you please provide more details? As to StringBuffer, I think it is being used since those classes in `java.text` package have been created. I am not sure why that contributes to what you described as the "performance regression". Separately, please split this PR into two, as combining two different issues into a single JBS issue/PR is not right. The second issue is likely due to loading stream classes for the first time at JVM startup. - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2146264799
RFR: 8333456: CompactNumberFormat integer parsing fails when string has no suffix
Please review this PR which handles incorrect CompactNumberFormat integer only parsing when there is no suffix. See the following snippet, var fmt = NumberFormat.getCompactNumberInstance(Locale.US, NumberFormat.Style.SHORT) fmt.setParseIntegerOnly(true) fmt.parse("5K") // returns 5000 fmt.parse("50.00") // returns 50 fmt.parse("5") // unexpectedly throws StringIndexOutOfBoundsException fmt.parse("5 ") // returns 5 Within the `parse` method, there is code that advances `position` past the fractional portion to find the suffix when `parseIntegerOnly()` is true. However, if a valid string input is given with no suffix, `DecimalFormat.subparseNumber()` will fully iterate through the string and `position` will be equal to the length of the string, throwing StringIndexOutOfBoundsException when `charAt` is invoked (line 1740). We should check to make sure position does not exceed the string length before deciding to check for a decimal symbol. - Commit messages: - init Changes: https://git.openjdk.org/jdk/pull/19533/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19533=00 Issue: https://bugs.openjdk.org/browse/JDK-8333456 Stats: 69 lines in 2 files changed: 68 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19533.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19533/head:pull/19533 PR: https://git.openjdk.org/jdk/pull/19533
Re: RFR: 8333452: Make logging in jdk.jpackage.internal.OverridableResource class configurable
On Mon, 3 Jun 2024 21:53:06 GMT, Alexander Matveev wrote: > This added functionality is not used. It is completely unclear from PR and/or > issue description why we need this and how it will be used in the future? It was supposed to be used in [WiX4 project](https://bugs.openjdk.org/browse/JDK-8319457). In this project the same resource was supposed to be saved multiple times, producing duplicated messages in the log. This was the case at the early stage of the project. I looked closer and it is not needed any more. I'll withdraw this PR. - PR Comment: https://git.openjdk.org/jdk/pull/19532#issuecomment-2146217269
Withdrawn: 8333452: Make logging in jdk.jpackage.internal.OverridableResource class configurable
On Mon, 3 Jun 2024 20:29:47 GMT, Alexey Semenyuk wrote: > 8333452: Make logging in jdk.jpackage.internal.OverridableResource class > configurable This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/19532
Re: RFR: 8332547: Unloaded signature classes in DirectMethodHandles [v2]
On Mon, 3 Jun 2024 19:36:58 GMT, Vladimir Ivanov wrote: >> JVM routinely installs loader constraints for unloaded signature classes >> when method resolution takes place. MethodHandle resolution took a different >> route and eagerly resolves signature classes instead (see >> `java.lang.invoke.MemberName$Factory::resolve` and >> `sun.invoke.util.VerifyAccess::isTypeVisible` for details). >> >> There's a micro-optimization which bypasses eager resolution for `java.*` >> classes. The downside is that `java.*` signature classes can show up as >> unloaded. It manifests as inlining failures during JIT-compilation and may >> cause severe performance issues. >> >> Proposed fix removes the aforementioned special case logic during >> `MethodHandle` resolution. >> >> In some cases it may slow down `MethodHandle` construction a bit (e.g., when >> repeatedly constructing `DirectMethodHandle`s with lots of arguments), but >> `MethodHandle` construction step is not performance critical. >> >> Testing: hs-tier1 - hs-tier4 > > Vladimir Ivanov has updated the pull request incrementally with one > additional commit since the last revision: > > Renaming: isTypeVisible -> ensureTypeVisible Marked as reviewed by liach (Author). - PR Review: https://git.openjdk.org/jdk/pull/19319#pullrequestreview-2094960904
Integrated: 8332424: Update IANA Language Subtag Registry to Version 2024-05-16
On Fri, 17 May 2024 18:04:02 GMT, Justin Lu wrote: > Please review this PR which is a trivial update to the IANA sub tag registry > to version 2024-05-16. Locale tests pass as expected after update. > > Associated announcement -> > https://mm.icann.org/pipermail/ietf-languages-announcements/2024-May/91.html This pull request has now been integrated. Changeset: 6dac8d64 Author:Justin Lu URL: https://git.openjdk.org/jdk/commit/6dac8d64527b4e9ade783b99f82fbecd81c426a6 Stats: 9 lines in 2 files changed: 6 ins; 0 del; 3 mod 8332424: Update IANA Language Subtag Registry to Version 2024-05-16 Reviewed-by: naoto, iris - PR: https://git.openjdk.org/jdk/pull/19286
Re: RFR: 8333452: Make logging in jdk.jpackage.internal.OverridableResource class configurable
On Mon, 3 Jun 2024 20:29:47 GMT, Alexey Semenyuk wrote: > 8333452: Make logging in jdk.jpackage.internal.OverridableResource class > configurable OverridableResource uses Log.verbose, which will log if -verbose is specified. What do you mean by "OverridableResource class unconditionally writes in the log."? Why "In some situations emitting log messages is not desired."? If we do not need to log them when "-verbose" is specified then lets just remove them. `NoLogging` class is only used by test. Can we modify `OverridableResource` in such way it does not require additional code which is used by test only? This added functionality is not used. It is completely unclear from PR and/or issue description why we need this and how it will be used in the future? - PR Comment: https://git.openjdk.org/jdk/pull/19532#issuecomment-2146189724
Re: RFR: 8332072: Convert package.html files in `java.naming` to package-info.java
On Mon, 3 Jun 2024 17:26:52 GMT, Nizar Benalla wrote: > Can I please get a review for this small change? The motivation is that javac > does not recognize `package.html` files. > > The conversion was simple, I used a script to rename the files, append "*" on > the left and remove some HTML tags like `` and ``. I did the > conversion in place, renaming them in git but with the big amount of change > `git` thinks it's a new file. > > I also added a new `package-info.java` file to `javax.naming.ldap.spi`. I > hope that's fine. I recommend having the new package-info.java files follow the now-current javadoc conventions including: * Using `{@code foo}` in preference to `foo` * Using the `@snippet` tag in preference to `` * etc. - PR Comment: https://git.openjdk.org/jdk/pull/19529#issuecomment-2146157205
Instant.now(Clock) and InstantSource
Hi folks, It feels a bit strange that you can't pass an `InstantSource` to `Instant.now(...)`, but you _can_ pass a `Clock` (which of course has a "useless" `ZoneId` when creating an `Instant`). Therefore, I'd like to propose one of the following API changes: 1) adding `Instant.now(InstantSource)` 2) deprecating `Instant.now(Clock)` in favor of `clock.instant()` (I believe removing `Instant.now(Clock)` would be binary compatible, so that's not an option, right?) There's also a related discussion about adding an InstantSource-based static factory method to the "local" types (e.g., `LocalDate.now(InstantSource, ZoneId)`). However, I always avoid using the static factories on those types and have been quite happy with the fluent pattern instead (e.g., `instantSource.instant().atZone(...).toLocalDate()`). Thoughts? -- kak
Re: RFR: 8332547: Unloaded signature classes in DirectMethodHandles [v2]
On Mon, 3 Jun 2024 19:36:58 GMT, Vladimir Ivanov wrote: >> JVM routinely installs loader constraints for unloaded signature classes >> when method resolution takes place. MethodHandle resolution took a different >> route and eagerly resolves signature classes instead (see >> `java.lang.invoke.MemberName$Factory::resolve` and >> `sun.invoke.util.VerifyAccess::isTypeVisible` for details). >> >> There's a micro-optimization which bypasses eager resolution for `java.*` >> classes. The downside is that `java.*` signature classes can show up as >> unloaded. It manifests as inlining failures during JIT-compilation and may >> cause severe performance issues. >> >> Proposed fix removes the aforementioned special case logic during >> `MethodHandle` resolution. >> >> In some cases it may slow down `MethodHandle` construction a bit (e.g., when >> repeatedly constructing `DirectMethodHandle`s with lots of arguments), but >> `MethodHandle` construction step is not performance critical. >> >> Testing: hs-tier1 - hs-tier4 > > Vladimir Ivanov has updated the pull request incrementally with one > additional commit since the last revision: > > Renaming: isTypeVisible -> ensureTypeVisible Thanks - Marked as reviewed by jvernee (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19319#pullrequestreview-2094825208
Re: RFR: 8333452: Make logging in jdk.jpackage.internal.OverridableResource class configurable
On Mon, 3 Jun 2024 20:29:47 GMT, Alexey Semenyuk wrote: > 8333452: Make logging in jdk.jpackage.internal.OverridableResource class > configurable @sashamatveev, please review - PR Comment: https://git.openjdk.org/jdk/pull/19532#issuecomment-2146097131
RFR: 8333452: Make logging in jdk.jpackage.internal.OverridableResource class configurable
8333452: Make logging in jdk.jpackage.internal.OverridableResource class configurable - Commit messages: - 8333452: Make logging in jdk.jpackage.internal.OverridableResource class configurable Changes: https://git.openjdk.org/jdk/pull/19532/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19532=00 Issue: https://bugs.openjdk.org/browse/JDK-8333452 Stats: 67 lines in 2 files changed: 59 ins; 0 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/19532.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19532/head:pull/19532 PR: https://git.openjdk.org/jdk/pull/19532
Re: RFR: 8332547: Unloaded signature classes in DirectMethodHandles [v2]
> JVM routinely installs loader constraints for unloaded signature classes when > method resolution takes place. MethodHandle resolution took a different route > and eagerly resolves signature classes instead (see > `java.lang.invoke.MemberName$Factory::resolve` and > `sun.invoke.util.VerifyAccess::isTypeVisible` for details). > > There's a micro-optimization which bypasses eager resolution for `java.*` > classes. The downside is that `java.*` signature classes can show up as > unloaded. It manifests as inlining failures during JIT-compilation and may > cause severe performance issues. > > Proposed fix removes the aforementioned special case logic during > `MethodHandle` resolution. > > In some cases it may slow down `MethodHandle` construction a bit (e.g., when > repeatedly constructing `DirectMethodHandle`s with lots of arguments), but > `MethodHandle` construction step is not performance critical. > > Testing: hs-tier1 - hs-tier4 Vladimir Ivanov has updated the pull request incrementally with one additional commit since the last revision: Renaming: isTypeVisible -> ensureTypeVisible - Changes: - all: https://git.openjdk.org/jdk/pull/19319/files - new: https://git.openjdk.org/jdk/pull/19319/files/805d42fc..058cdba3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19319=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19319=00-01 Stats: 9 lines in 3 files changed: 0 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/19319.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19319/head:pull/19319 PR: https://git.openjdk.org/jdk/pull/19319
Re: RFR: 8332547: Unloaded signature classes in DirectMethodHandles
On Tue, 21 May 2024 20:14:41 GMT, Jorn Vernee wrote: >> Class loading triggered by `Class.forName()` call is at the core of >> `isTypeVisible`. (The rest is fast path checks.) It's what makes >> `isTypeVisible` query idempotent. >> >> I can definitely name it differently (e.g, `ensureTypeVisible`), but making >> a separate class loading pass across signature classes doesn't make much >> sense. > >> I can definitely name it differently (e.g, ensureTypeVisible), but making a >> separate class loading pass across signature classes doesn't make much sense. > > Ok, in that case I suggest also renaming `MemberName::checkForTypeAlias`, > maybe to `ensureTypeVisible` as well. @JornVernee ok, renamed. - PR Comment: https://git.openjdk.org/jdk/pull/19319#issuecomment-2145967665
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]
On Mon, 3 Jun 2024 15:40:10 GMT, Severin Gehwolf wrote: > Does that proposal sound good? That table is useful, I think it's right. No change to default behavior. If someone configures with --enable-runtime-image then they get a JDK run-time image that supports jlink (with some limitations) but is significantly small as it doesn't include the packaged modules. I've read through most of the changes now. Overall I think it's looking good, just a few terminology and minor points that I'll add as comments. - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2145932080
Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations
On Mon, 3 Jun 2024 17:07:28 GMT, Aleksey Shipilev wrote: >> Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless >> cloning of Object[0] instances. This cloning is intended to prevent callers >> from changing array contents, but many `CopyOnWriteArrayList`s are allocated >> to size zero, or are otherwise maintained empty, so cloning is unnecessary. >> >> Results from the included JMH benchmark: >> Before: >> >> BenchmarkMode Cnt >> Score Error Units >> CopyOnWriteArrayListBenchmark.clear avgt5 >> 74.487 ± 1.793 ns/op >> CopyOnWriteArrayListBenchmark.clearEmpty avgt5 >> 27.918 ± 0.759 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 >> 16.656 ± 0.375 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 >> 15.415 ± 0.489 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 >> 21.608 ± 0.363 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 >> 15.374 ± 0.260 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 >> 15.688 ± 0.350 ns/op >> >> >> After: >> >> BenchmarkMode Cnt >> Score Error Units >> CopyOnWriteArrayListBenchmark.clear avgt5 >> 75.365 ± 2.092 ns/op >> CopyOnWriteArrayListBenchmark.clearEmpty avgt5 >> 20.803 ± 0.539 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 >> 16.808 ± 0.582 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 >> 12.980 ± 0.418 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 >> 21.627 ± 0.173 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 >> 12.864 ± 0.408 ns/op >> CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 >> 12.931 ± 0.255 ns/op > > src/java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java > line 338: > >> 336: */ >> 337: public Object[] toArray() { >> 338: return getArray().length == 0 ? getArray() : getArray().clone(); > > Unfortunately, I think this is against the spec for this method, which > explicitly says the method must allocate a new array. Yes, this change would > be within the spirit of the spec ("You can modify the array", which you > cannot if the object is empty), but is against the letter of it. I think you're right, I'll remove the change to toArray(). - PR Review Comment: https://git.openjdk.org/jdk/pull/19527#discussion_r1624873832
Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations
On Mon, 3 Jun 2024 16:47:20 GMT, jengebr wrote: > Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless > cloning of Object[0] instances. This cloning is intended to prevent callers > from changing array contents, but many `CopyOnWriteArrayList`s are allocated > to size zero, or are otherwise maintained empty, so cloning is unnecessary. > > Results from the included JMH benchmark: > Before: > > BenchmarkMode Cnt > Score Error Units > CopyOnWriteArrayListBenchmark.clear avgt5 > 74.487 ± 1.793 ns/op > CopyOnWriteArrayListBenchmark.clearEmpty avgt5 > 27.918 ± 0.759 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 > 16.656 ± 0.375 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 > 15.415 ± 0.489 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 > 21.608 ± 0.363 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 > 15.374 ± 0.260 ns/op > CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 > 15.688 ± 0.350 ns/op > > > After: > > BenchmarkMode Cnt > Score Error Units > CopyOnWriteArrayListBenchmark.clear avgt5 > 75.365 ± 2.092 ns/op > CopyOnWriteArrayListBenchmark.clearEmpty avgt5 > 20.803 ± 0.539 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 > 16.808 ± 0.582 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 > 12.980 ± 0.418 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 > 21.627 ± 0.173 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 > 12.864 ± 0.408 ns/op > CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 > 12.931 ± 0.255 ns/op The jmh benchmark checks only the empty case, you need to also show lack of impact on non-empty cases. Assuming you demonstrate this, it seems basically OK, (Deja vu previous cases including hash maps). It is only a small band-aid -- programs generating lots of them still have to allocate the COWAL object, so the savings are small compared to not generating them at all unless needed. - PR Comment: https://git.openjdk.org/jdk/pull/19527#issuecomment-2145896233
RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations
Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless cloning of Object[0] instances. This cloning is intended to prevent callers from changing array contents, but many `CopyOnWriteArrayList`s are allocated to size zero, or are otherwise maintained empty, so cloning is unnecessary. Results from the included JMH benchmark: Before: BenchmarkMode Cnt Score Error Units CopyOnWriteArrayListBenchmark.clear avgt5 74.487 ± 1.793 ns/op CopyOnWriteArrayListBenchmark.clearEmpty avgt5 27.918 ± 0.759 ns/op CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 16.656 ± 0.375 ns/op CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 15.415 ± 0.489 ns/op CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 21.608 ± 0.363 ns/op CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 15.374 ± 0.260 ns/op CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 15.688 ± 0.350 ns/op After: BenchmarkMode Cnt Score Error Units CopyOnWriteArrayListBenchmark.clear avgt5 75.365 ± 2.092 ns/op CopyOnWriteArrayListBenchmark.clearEmpty avgt5 20.803 ± 0.539 ns/op CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 16.808 ± 0.582 ns/op CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 12.980 ± 0.418 ns/op CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 21.627 ± 0.173 ns/op CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 12.864 ± 0.408 ns/op CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 12.931 ± 0.255 ns/op - Commit messages: - Fixing whitespace - Removing toArray from benchmark - Reverting change to clone() - Merge branch 'openjdk:master' into CopyOnWriteArrayList_optimization - Adding JMH benchmark - Merge branch 'openjdk:master' into CopyOnWriteArrayList_optimization - Merge branch 'openjdk:master' into CopyOnWriteArrayList_optimization - Revert "Updating copyright yeaer" - Merge branch 'openjdk:master' into CopyOnWriteArrayList_optimization - Updating copyright yeaer - ... and 2 more: https://git.openjdk.org/jdk/compare/9686e804...ea7a0042 Changes: https://git.openjdk.org/jdk/pull/19527/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19527=00 Issue: https://bugs.openjdk.org/browse/JDK-8332842 Stats: 107 lines in 2 files changed: 104 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19527.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19527/head:pull/19527 PR: https://git.openjdk.org/jdk/pull/19527
Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations
On Mon, 3 Jun 2024 16:47:20 GMT, jengebr wrote: > Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless > cloning of Object[0] instances. This cloning is intended to prevent callers > from changing array contents, but many `CopyOnWriteArrayList`s are allocated > to size zero, or are otherwise maintained empty, so cloning is unnecessary. > > Results from the included JMH benchmark: > Before: > > BenchmarkMode Cnt > Score Error Units > CopyOnWriteArrayListBenchmark.clear avgt5 > 74.487 ± 1.793 ns/op > CopyOnWriteArrayListBenchmark.clearEmpty avgt5 > 27.918 ± 0.759 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 > 16.656 ± 0.375 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 > 15.415 ± 0.489 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 > 21.608 ± 0.363 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 > 15.374 ± 0.260 ns/op > CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 > 15.688 ± 0.350 ns/op > > > After: > > BenchmarkMode Cnt > Score Error Units > CopyOnWriteArrayListBenchmark.clear avgt5 > 75.365 ± 2.092 ns/op > CopyOnWriteArrayListBenchmark.clearEmpty avgt5 > 20.803 ± 0.539 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayavgt5 > 16.808 ± 0.582 ns/op > CopyOnWriteArrayListBenchmark.createInstanceArrayEmpty avgt5 > 12.980 ± 0.418 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollection avgt5 > 21.627 ± 0.173 ns/op > CopyOnWriteArrayListBenchmark.createInstanceCollectionEmpty avgt5 > 12.864 ± 0.408 ns/op > CopyOnWriteArrayListBenchmark.createInstanceDefault avgt5 > 12.931 ± 0.255 ns/op Some initial comments. @DougLea might want to give it a sanity check. Note there is a jcheck failure due to whitespace issues. Plus, I think the PR should still be named "8332842: Optimize empty CopyOnWriteArrayList allocations"? src/java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java line 338: > 336: */ > 337: public Object[] toArray() { > 338: return getArray().length == 0 ? getArray() : getArray().clone(); Unfortunately, I think this is against the spec for this method, which explicitly says the method must allocate a new array. Yes, this change would be within the spirit of the spec ("You can modify the array", which you cannot if the object is empty), but is against the letter of it. - PR Review: https://git.openjdk.org/jdk/pull/19527#pullrequestreview-2094435376 PR Comment: https://git.openjdk.org/jdk/pull/19527#issuecomment-2145853813 PR Review Comment: https://git.openjdk.org/jdk/pull/19527#discussion_r1624791561
RFR: 8332072: Convert package.html files in `java.naming` to package-info.java
Can I please get a review for this small change? The motivation is that javac does not recognize `package.html` files. The conversion was simple, I used a script to rename the files, append "*" on the left and remove some HTML tags like and . I did the conversion in place, renaming them in git but with the big amount of change git thinks it's a new file. I also added a new `package-info.java` file to `javax.naming.ldap.spi`. I hope that's fine. - Commit messages: - remove whitespace - 8332072: Convert package.html files in `java.naming` to package-info.java Changes: https://git.openjdk.org/jdk/pull/19529/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19529=00 Issue: https://bugs.openjdk.org/browse/JDK-8332072 Stats: 1438 lines in 11 files changed: 724 ins; 714 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19529.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19529/head:pull/19529 PR: https://git.openjdk.org/jdk/pull/19529
Re: RFR: 8332249: Micro-optimize Method.hashCode
On Tue, 28 May 2024 17:25:34 GMT, Sean Gwizdak wrote: > Improve the speed of Method.hashCode by caching the hashcode on first use. > I've seen an application where Method.hashCode is a hot path, and this is a > fairly simple speedup. The memory overhead is low. > > This addresses issue > [JDK-8332249](https://bugs.openjdk.org/browse/JDK-8332249). > > Before: > > Benchmark Mode Cnt Score Error Units > # Intel Skylake > MethodHashCode.benchmarkHashCode avgt5 1.843 ± 0.149 ns/op > # Arm Neoverse N1 > MethodHashCode.benchmarkHashCode avgt5 2.363 ± 0.091 ns/op > > > > After: > > > Benchmark Mode Cnt Score Error Units > # Intel Skylake > MethodHashCode.benchmarkHashCode avgt5 1.121 ± 1.189 ns/op > # Arm Neoverse N1 > MethodHashCode.benchmarkHashCode avgt5 1.001 ± 0.001 ns/op Minor change after draft review -- moved Method hashCode benchmark into the newly created MethodBenchmark file. - PR Comment: https://git.openjdk.org/jdk/pull/19433#issuecomment-2145806129
Re: RFR: 8332249: Micro-optimize Method.hashCode [v2]
> Improve the speed of Method.hashCode by caching the hashcode on first use. > I've seen an application where Method.hashCode is a hot path, and this is a > fairly simple speedup. The memory overhead is low. > > This addresses issue > [JDK-8332249](https://bugs.openjdk.org/browse/JDK-8332249). > > Before: > > Benchmark Mode Cnt Score Error Units > # Intel Skylake > MethodHashCode.benchmarkHashCode avgt5 1.843 ± 0.149 ns/op > # Arm Neoverse N1 > MethodHashCode.benchmarkHashCode avgt5 2.363 ± 0.091 ns/op > > > > After: > > > Benchmark Mode Cnt Score Error Units > # Intel Skylake > MethodHashCode.benchmarkHashCode avgt5 1.121 ± 1.189 ns/op > # Arm Neoverse N1 > MethodHashCode.benchmarkHashCode avgt5 1.001 ± 0.001 ns/op Sean Gwizdak 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 six additional commits since the last revision: - Remove trailing whitespace. - Move hashCode benchmark into the newly created MethodBenchmark file - Merge branch 'master' into method-hashcode-JDK-8332249 - Remove changes to JavaDoc per guidance. - Fix whitespace issues pointed by the bot - Micro-optimize Method.hashCode - Changes: - all: https://git.openjdk.org/jdk/pull/19433/files - new: https://git.openjdk.org/jdk/pull/19433/files/2e4c0c94..4364d99d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19433=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19433=00-01 Stats: 30025 lines in 868 files changed: 18155 ins; 7339 del; 4531 mod Patch: https://git.openjdk.org/jdk/pull/19433.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19433/head:pull/19433 PR: https://git.openjdk.org/jdk/pull/19433
Re: RFR: 8330954: since-checker - Fix remaining `@since` tags in `java.base` [v4]
On Mon, 13 May 2024 20:14:38 GMT, Nizar Benalla wrote: >> Please review this PR that aims to add all the remaining needed `@since` >> tags in `java.base`, and group them into a single fix. >> This is related to #18934 and my work around the `@since` checker feature. >> Explicit `@since` tags are needed for some overriding methods for the >> purpose of the checker. >> >> I will only give the example with the `CompletableFuture` class but here is >> the before where the methods only appeared in "Methods declared in interface >> N" >> >> > src="https://github.com/openjdk/jdk/assets/96922791/1749a355-133b-4a38-bffe-51ac415b2aac;> >> >> >> >> and after where the method has it's own javadoc, the main description is >> added and the `@since` tags are added as intended. >> >> I don't have an account on `https://cr.openjdk.org/` but I could host the >> generated docs somewhere if that is needed. >> >> > src="https://github.com/openjdk/jdk/assets/96922791/89b92288-9b5e-48ed-8fa1-9342ea43e043;> >> >> > src="https://github.com/openjdk/jdk/assets/96922791/9aef08ff-5030-4189-a996-582a7eef849b;> >> >> > src="https://github.com/openjdk/jdk/assets/96922791/fdd75a26-0396-4c5e-8f59-a3717b7d7ec8;> >> >> >> TIA > > Nizar Benalla 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 11 additional > commits since the last revision: > > - Merge remote-tracking branch 'upstream/master' into 8330954 > - (C) > - (C) > - Move security classes changes to pr18373 > - remove couple extra lines > - Pull request is now only about `@since` tags, might add an other commit > - add one more `{inheritDoc}` to `CompletableFuture.state` > - add `@throws` declarations to javadoc > - add ``{@inheritDoc}`` to new javadoc comments > - remove two extra spaces > - ... and 1 more: https://git.openjdk.org/jdk/compare/39767aaa...b3574b97 I disagree somewhat with the statements in the comments that the checker should follow the javadoc rules, whatever they are. The important thing is to decide what the rules for `@since` should be, in terms of changes to the set of signatures in the class. Generally, I think the rule should be that _every_ declaration should have `@since` _except_ that members need not have the tag if it would be the same as for the enclosing class or interface. As far as adding an overriding method is concerned, if it has the same VM descriptor as the overridden method, it is not a "new" method in the class; if it has a covariant return type, that is a significant change to the descriptor and so such methods should have `@since`. As a practical rule for deciding whether any declaration is new or not, imagine writing a test program that refers to the most specific form of the declaration. If that test program does not compile on JDK version N-1 and does compile on version N, then it warrants having `@since N`. Put another way, `@since N` should identify the first release in which the declaration can be used in the given form. Note, these rules are stated without reference to what `javadoc` does. `javadoc` should follow these rules as well; it is a bug if the tool generates incorrect documentation based on `@since` tags following these rules. Also, while the proposed new Since Checker should follow these rules when analysing declarations, it may go further when making suggestions to correct errors that it finds. For example, instead of simply saying _No `@since` tag found here_, it might analyze the history and say _No `@since` tag found here; the declaration was introduced in X_ for an appropriate X. - PR Comment: https://git.openjdk.org/jdk/pull/18954#issuecomment-2145605004
Integrated: 8333103: Re-examine the console provider loading
On Wed, 29 May 2024 19:51:36 GMT, Naoto Sato wrote: > There is an initialization code in `Console` class that searches for the > Console implementations. Refactoring the init code not to use lambda/stream > would reduce the (initial) number of loaded classes by about 100 for > java.base implementations. This would become relevant when the java.io.IO > (JEP 477) uses Console as the underlying framework. This pull request has now been integrated. Changeset: 9686e804 Author:Naoto Sato URL: https://git.openjdk.org/jdk/commit/9686e804a2b058955ff88149c54a0a7896c0a2eb Stats: 23 lines in 1 file changed: 5 ins; 0 del; 18 mod 8333103: Re-examine the console provider loading Reviewed-by: redestad, jpai - PR: https://git.openjdk.org/jdk/pull/19467
Re: RFR: 8333103: Re-examine the console provider loading [v2]
On Thu, 30 May 2024 19:48:25 GMT, Naoto Sato wrote: >> There is an initialization code in `Console` class that searches for the >> Console implementations. Refactoring the init code not to use lambda/stream >> would reduce the (initial) number of loaded classes by about 100 for >> java.base implementations. This would become relevant when the java.io.IO >> (JEP 477) uses Console as the underlying framework. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Clarify the comment for multiple impls in the module case Thank you for your reviews! - PR Comment: https://git.openjdk.org/jdk/pull/19467#issuecomment-2145561992
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]
On Mon, 3 Jun 2024 12:55:54 GMT, Alan Bateman wrote: > So I think we may have the wrong default. Yes, they are separate configure > and jlink options but I'm wondering if it would be more sensible to > opt-in(not opt-out) to keep the packaged modules when configured with > --enable-runtime-link-image. OK. I'll rework it so that we'll have: | config opts | effect | equivalent to | | -|--|-| | `--enable-runtime-link-image` | produces a linkable runtime **without** packaged modules even though the default of `--enable-packaged-modules` is otherwise `true`.| `--enable-runtime-link-image --disable-packaged-modules`| | `--enable-runtime-link-image --enable-packaged-modules`| produces a linkable runtime **with** packaged modules, overriding the default of packaged modules not being enabled when `--enable-runtime-link-image` is being used otherwise | N/A| | `--disable-runtime-link-image` | Default as of today. Adds packaged modules, with no run time link supporting jimage | `--disable-runtime-link-image --enable-packaged-modules`| | `--disable-runtime-link-image --disable-packaged-modules` | No linkable jimage runtime, no packaged modules in the resulting JDK | N/A | Does that proposal sound good? - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2145540168
Status of project "Brisbane"?
Hi, What's the status of Project Brisbane? According to [1], the Project was approved two month ago on April 4th, but until now I can't find it listed on openjdk.org nor can I find a corresponding mailing list? Best regards, Volker [1] https://mail.openjdk.org/pipermail/announce/2024-April/000350.html
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v18]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > The generated proxy is migrated from static initialization to CONDY bootstrap. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: MethodTypeDescImpl::ofValidated changed to varargs - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/fb3cdcdd..6ca164bc Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=17 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=16-17 Stats: 14 lines in 2 files changed: 0 ins; 0 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Mon, 3 Jun 2024 08:37:50 GMT, David Holmes wrote: >> Fix MainClassTest class to use HelloApp.AppOutputVerifier class to run app >> launcher instead of raw Executor. This makes MainClassTest test run app >> launchers with retries. This change addresses the primary issue. >> >> Fix inconsistencies in HelloApp.AppOutputVerifier class. It used to provide >> API allowing to run launchers without retries. It inconsistently allowed the >> execution of launchers with suppressed output (stdout and stderr). It >> inconsistently executed launchers with/without PATH removed from the >> environment. >> >> These loopholes were eliminated: >> >> - stdout and stderr of app launchers is never suppressed; >> - PATH env variable is always deleted for app launchers on Windows. It is >> not deleted on other platforms. This change sets the correct scope of >> [JDK-8254920](https://bugs.openjdk.org/browse/JDK-8254920) fix that >> introduced the removal of PATH env variable for app launchers; >> - app launchers are always executed with retries unless the launcher is >> executed with `jpackage.test.noexit` system property set to `true` >> indicating the test app will not terminate on its own. >> >> Other changes are due to changes in HelloApp.AppOutputVerifier class. > > I am yet to see anything that actually explains the cause of the `dlerror` > crash here ??? @dholmes-ora there is no fix for the cause of the `dlerror` crash in this PR. The PR fixes jpackage tests to rerun a launcher if it crashes. This workaround for jpackage tests was first introduced in [JDK-8269403](https://bugs.openjdk.org/browse/JDK-8269403) fix but MainClassTest was left unfixed back then. This PR complements [JDK-8269403](https://bugs.openjdk.org/browse/JDK-8269403). - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2145373568
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v3]
On Mon, 27 May 2024 12:20:20 GMT, Chen Liang wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> performance improvements > > src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java line 76: > >> 74: >> 75: private static final MethodTypeDesc >> 76: MTD_boolean = MethodTypeDescImpl.ofValidated(CD_boolean, >> ConstantUtils.EMPTY_CLASSDESC), > > Maybe we can change the `MethodTypeDescImpl.ofValidated` to varargs so we > don't need explicit array initializations. Yes, I like that idea. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1624576109
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v17]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > The generated proxy is migrated from static initialization to CONDY bootstrap. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: ClassFile context made static - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/29e269f5..fb3cdcdd Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=16 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=15-16 Stats: 12 lines in 1 file changed: 3 ins; 8 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Integrated: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor}
On Tue, 21 May 2024 13:49:18 GMT, jengebr wrote: > Improve `java/lang/reflect/Method.java` by eliminating needless cloning of > Class[0] instances. This cloning is intended to prevent callers from > changing array contents, but many Methods have zero exceptions or zero > parameters, and returning the original `Class[0]` is sufficient. > > Results from the included JMH benchmark: > Before: > > BenchmarkMode Cnt Score Error Units > ConstructorBenchmark.getExceptionTypes avgt5 6.526 ± 0.183 ns/op > ConstructorBenchmark.getExceptionTypesEmpty avgt5 5.803 ± 0.073 ns/op > ConstructorBenchmark.getParameterTypes avgt5 6.521 ± 0.188 ns/op > ConstructorBenchmark.getParameterTypesEmpty avgt5 5.747 ± 0.087 ns/op > MethodBenchmark.getExceptionTypesavgt5 6.525 ± 0.163 ns/op > MethodBenchmark.getExceptionTypesEmpty avgt5 5.783 ± 0.130 ns/op > MethodBenchmark.getParameterTypesavgt5 6.518 ± 0.195 ns/op > MethodBenchmark.getParameterTypesEmpty avgt5 5.742 ± 0.028 ns/op > > > After: > > BenchmarkMode Cnt Score Error Units > ConstructorBenchmark.getExceptionTypes avgt5 6.590 ± 0.124 ns/op > ConstructorBenchmark.getExceptionTypesEmpty avgt5 1.351 ± 0.061 ns/op > ConstructorBenchmark.getParameterTypes avgt5 6.651 ± 0.132 ns/op > ConstructorBenchmark.getParameterTypesEmpty avgt5 1.353 ± 0.150 ns/op > MethodBenchmark.getExceptionTypesavgt5 6.701 ± 0.151 ns/op > MethodBenchmark.getExceptionTypesEmpty avgt5 1.422 ± 0.025 ns/op > MethodBenchmark.getParameterTypesavgt5 6.629 ± 0.142 ns/op > MethodBenchmark.getParameterTypesEmpty avgt5 1.273 ± 0.169 ns/op This pull request has now been integrated. Changeset: 27af19d9 Author:John Engebretson Committer: Aleksey Shipilev URL: https://git.openjdk.org/jdk/commit/27af19d921a5cf15f5146471b58961815690b4f2 Stats: 177 lines in 4 files changed: 170 ins; 1 del; 6 mod 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor} Reviewed-by: shade, rriggs, liach - PR: https://git.openjdk.org/jdk/pull/19327
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]
On Sun, 2 Jun 2024 17:41:55 GMT, Alan Bateman wrote: >> Severin Gehwolf has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 110 commits: >> >> - Merge branch 'master' into jdk-8311302-jmodless-link >> - Fix new line in jlink.properties >> - Merge branch 'master' into jdk-8311302-jmodless-link >> - Simplify JLINK_JDK_EXTRA_OPTS var handling >> - Only add runtime track files for linkable runtimes >> - Generate the runtime image link diff at jlink time >> >>But only do that once the plugin-pipeline has run. The generation is >>enabled with the hidden option '--generate-linkable-runtime' and >>the resource pools available at jlink time are being used to generate >>the diff. >> - Merge branch 'master' into jdk-8311302-jmodless-link >> - Use shorter name for the build tool >> - Review feedback from Erik J. >> - Remove dependency on CDS which isn't needed anymore >> - ... and 100 more: https://git.openjdk.org/jdk/compare/4f1a10f8...e1e3f02f > > I've been looking through the changes. One thing that I'm wondering about is > whether --generate-runtime-link-image should disable the keeping of packaged > modules (set JLINK_KEEP_PACKAGED_MODULES to false). It seems surprising to > use this configure option and have the jlink command in the build also copy > the packaged modules into the image. > > (Doing that would of course mean that several existing jlink tests would need > some changes, either to `@requires` or to remove the checks for the `jmods` > directory) > @AlanBateman IMO those are orthogonal concepts. The purpose of --enable-runtime-link-image is to produce a run-time image that is capable of running jlink without the packaged modules. The benefit is is a reducing size on the file system but you don't get that benefit if the packages modules are copied in the image. So I think we may have the wrong default. Yes, they are separate configure and jlink options but I'm wondering if it would be more sensible to opt-in(not opt-out) to keep the packaged modules when configured with --enable-runtime-link-image. - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2145136017
Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v17]
On Mon, 3 Jun 2024 11:16:49 GMT, Doug Lea wrote: >> src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1345: >> >>> 1343: int b = base, p = top, cap; >>> 1344: if (p - b > 0 && a != null && (cap = a.length) > 0) { >>> 1345: for (int m = cap - 1, s, nb;;) { >> >> @DougLea I'm curious, what effect did you see if moving the `p - b > 0` out >> of the loop conditional? > > This was part of saving a read in the common case of empty local queue. > Barely but reliably worth doing. - PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1624352663
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v14]
On Mon, 3 Jun 2024 11:37:34 GMT, Claes Redestad wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> ProxyGenBench simplification > > test/micro/org/openjdk/bench/java/lang/reflect/Proxy/ProxyGenBench.java line > 23: > >> 21: * questions. >> 22: */ >> 23: package org.openjdk.bench.java.lang.reflect.Proxy; > > Package name needs to be lowercase. Not sure why the folder name is uppercase > Proxy, but the two pre-existing benchmarks both have lower case package > declarations. Uppercase letters in package names may subtly break a few tools Yes, I've moved it down to j/l/reflect. However the existing benchmarks probably need a separate treatment. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1624297815
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v16]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > The generated proxy is migrated from static initialization to CONDY bootstrap. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: fixed package - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/09baa376..29e269f5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=15 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=14-15 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v15]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > The generated proxy is migrated from static initialization to CONDY bootstrap. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: ProxyGenBench moved - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/7b00967d..09baa376 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=14 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=13-14 Stats: 0 lines in 1 file changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v14]
On Mon, 3 Jun 2024 11:09:31 GMT, Adam Sotona wrote: >> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use >> classfile API for reflection proxy-generation. Actual implementation of >> `ProxyGenerator` is focused on performance, however it causes JDK bootstrap >> regressions. `ProxyGenerator.TEMPLATE` class model is statically created and >> each proxy class is transformed from the template. >> >> This patch is intended to examine plain proxy generation impact on >> performance and JDK bootstrap (vs proxy transformation from template). >> >> The generated proxy is migrated from static initialization to CONDY >> bootstrap. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > ProxyGenBench simplification test/micro/org/openjdk/bench/java/lang/reflect/Proxy/ProxyGenBench.java line 23: > 21: * questions. > 22: */ > 23: package org.openjdk.bench.java.lang.reflect.Proxy; Package name needs to be lowercase. Not sure why the folder name is uppercase Proxy, but the two pre-existing benchmarks both have lower case package declarations. Uppercase letters in package names may subtly break a few tools - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1624270828
Re: RFR: 8322732: ForkJoinPool may underutilize cores in async mode [v17]
On Sun, 2 Jun 2024 14:33:45 GMT, Viktor Klang wrote: >> Doug Lea has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Reconcile changes > > src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 1345: > >> 1343: int b = base, p = top, cap; >> 1344: if (p - b > 0 && a != null && (cap = a.length) > 0) { >> 1345: for (int m = cap - 1, s, nb;;) { > > @DougLea I'm curious, what effect did you see if moving the `p - b > 0` out > of the loop conditional? This was part of saving a read in the common case of empty local queue. Barely but reliably worth doing. - PR Review Comment: https://git.openjdk.org/jdk/pull/19131#discussion_r1624246576
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v14]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > The generated proxy is migrated from static initialization to CONDY bootstrap. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: ProxyGenBench simplification - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/834d65c5..7b00967d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=13 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=12-13 Stats: 44 lines in 1 file changed: 1 ins; 33 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v13]
On Mon, 3 Jun 2024 10:30:03 GMT, Claes Redestad wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> added ProxyGenBench JMH micro benchmark > > test/micro/org/openjdk/bench/java/lang/reflect/Proxy/ProxyGenBench.java line > 66: > >> 64: ClassDesc tempDesc = >> ClassDesc.ofDescriptor(Interfaze.class.descriptorString()); >> 65: loader = new ClsLoader(); >> 66: clsMap = new HashMap<>(100); > > Suggestion: > > clsMap = HashMap.newHashMap(100); I've simplified it a lot. It uses a new `ClassLoader` instance for each proxy generation, instead of preparation of 100 different interfaces. - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1624235034
Re: RFR: 8206447: InflaterInputStream.skip receives long but it's limited to Integer.MAX_VALUE [v2]
On Mon, 3 Jun 2024 05:46:30 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which updates the API specification >> of `java.util.zip.InflaterInputStream.skip()` method to match its current >> implementation? >> >> `InflaterInputStream.skip()`, just like `DeflaterInputStream.skip()`, takes >> a `long` value `n` representing the number of bytes to skip. When a value >> greater than `Integer.MAX_VALUE` is passed to this >> `InflaterInputStream.skip()` method, the current implementation in that >> method only skips at most `Integer.MAX_VALUE` bytes. >> `DeflaterInputStream.skip()` too has the same behaviour. However, in the >> case of `DeflaterInputStream.skip()`, this semantic is clearly noted in that >> method's API documentation. `InflaterInputStream.skip()` currently doesn't >> specify this behaviour. >> >> The commit in this PR updates the `InflaterInputStream.skip()` to specify >> the current implementation. >> >> I'll open a CSR for this change. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > also note that the method may block, just like DeflaterInputStream > specifies it Marked as reviewed by lancea (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19515#pullrequestreview-2093515783
Re: RFR: 8333398: Uncomment the commented test in test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java
On Mon, 3 Jun 2024 04:25:38 GMT, Jaikiran Pai wrote: > Can I please get a review of this test-only change which uncomments an > additional test that was commented out in > `test/jdk/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java`? > > As noted in https://bugs.openjdk.org/browse/JDK-898, the original issue > due to which this line was commented out in the test, has been fixed several > releases back and thus this line can now be uncommented. > > I've verified that this test continues to pass with these changes. Change looks fine. We should look at at a later date to convert this to junit and when we do, look to rework isMultiReleaseJar() as it actually runs multiple tests within the test itself and if one fails, the rest will not be run - Marked as reviewed by lancea (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19514#pullrequestreview-2093508099
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v13]
On Mon, 3 Jun 2024 10:11:34 GMT, Adam Sotona wrote: >> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use >> classfile API for reflection proxy-generation. Actual implementation of >> `ProxyGenerator` is focused on performance, however it causes JDK bootstrap >> regressions. `ProxyGenerator.TEMPLATE` class model is statically created and >> each proxy class is transformed from the template. >> >> This patch is intended to examine plain proxy generation impact on >> performance and JDK bootstrap (vs proxy transformation from template). >> >> The generated proxy is migrated from static initialization to CONDY >> bootstrap. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > added ProxyGenBench JMH micro benchmark test/micro/org/openjdk/bench/java/lang/reflect/Proxy/ProxyGenBench.java line 66: > 64: ClassDesc tempDesc = > ClassDesc.ofDescriptor(Interfaze.class.descriptorString()); > 65: loader = new ClsLoader(); > 66: clsMap = new HashMap<>(100); Suggestion: clsMap = HashMap.newHashMap(100); - PR Review Comment: https://git.openjdk.org/jdk/pull/19410#discussion_r1624192732
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v29]
On Sun, 2 Jun 2024 17:41:55 GMT, Alan Bateman wrote: > I've been looking through the changes. One thing that I'm wondering about is > whether --generate-runtime-link-image should disable the keeping of packaged > modules (set JLINK_KEEP_PACKAGED_MODULES to false). It seems surprising to > use this configure option and have the jlink command in the build also copy > the packaged modules into the image. @AlanBateman IMO those are orthogonal concepts. Note that the configure options are `--enable-packaged-modules` and `--enable-runtime-link-image`. The corresponding `jlink` options are `--keep-packaged-modules` and `--generate-linkable-runtime`. My mental model is that with this patch it allows a more flexible distribution of the JDK build. The testing story also seems easier in its current form. All non-linkable runtime tests run as-is - with a linkable runtime build - but also run linkable runtime tests (those have appropriate `@requires` tags). We've had some discussion around this already in this review thread. I'm arguing that both configure options make sense independently and in combination. The user can configure it to their liking. What I'd try to avoid is needing to produce two different builds whether or not the JDK is runtime linkable or not. That is because our prime use-case is to make `jmods` optional when `jlink` is being used post build and test. I've tried to explain it earlier [here](https://github.com/openjdk/jdk/pull/14787#issuecomment-1999307995) and [here](https://github.com/openjdk/jdk/pull/14787#issuecomment-2003848668). @mlchung seemed OK with it [here](https://github.com/openjdk/jdk/pull/14787#issuecomment-2004605507) and @erikj79 was ok with it as well [here](https://github.com/openjdk/jdk/pull/14787#issuecomment-2004761747). Is there a specific reason this needs to be done? With the current patch `--enable-runtime-link-image` influences how the `jimage` in `lib/modules` looks like (adds some metadata) nothing else. `--enable-packaged-modules` influences copying of the packaged modules. Right now, what you are suggesting could be achieved with these configure options: `--enable-runtime-link-image --disable-packaged-modules` Is that not sufficient? Thoughts? - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2144808992
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v13]
> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use > classfile API for reflection proxy-generation. Actual implementation of > `ProxyGenerator` is focused on performance, however it causes JDK bootstrap > regressions. `ProxyGenerator.TEMPLATE` class model is statically created and > each proxy class is transformed from the template. > > This patch is intended to examine plain proxy generation impact on > performance and JDK bootstrap (vs proxy transformation from template). > > The generated proxy is migrated from static initialization to CONDY bootstrap. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: added ProxyGenBench JMH micro benchmark - Changes: - all: https://git.openjdk.org/jdk/pull/19410/files - new: https://git.openjdk.org/jdk/pull/19410/files/942342d5..834d65c5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19410=12 - incr: https://webrevs.openjdk.org/?repo=jdk=19410=11-12 Stats: 114 lines in 1 file changed: 114 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19410.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19410/head:pull/19410 PR: https://git.openjdk.org/jdk/pull/19410
Re: RFR: 8332457: Examine startup overheads from JDK-8294961 [v12]
On Wed, 29 May 2024 07:17:38 GMT, Adam Sotona wrote: >> [JDK-8294961](https://bugs.openjdk.org/browse/JDK-8294961) changed to use >> classfile API for reflection proxy-generation. Actual implementation of >> `ProxyGenerator` is focused on performance, however it causes JDK bootstrap >> regressions. `ProxyGenerator.TEMPLATE` class model is statically created and >> each proxy class is transformed from the template. >> >> This patch is intended to examine plain proxy generation impact on >> performance and JDK bootstrap (vs proxy transformation from template). >> >> The generated proxy is migrated from static initialization to CONDY >> bootstrap. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona 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 13 additional > commits since the last revision: > > - obsolete import > - Merge branch 'master' into JDK-8332457-proxy-startup > - missing bracket > - Update src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java > >Co-authored-by: liach <7806504+li...@users.noreply.github.com> > - removed obsolete entry > - MTD_ cleanup > - fixed javadoc > - CONDY implementation of ProxyGenerator > - simplification of the proxy class loading > - more improvements > - ... and 3 more: https://git.openjdk.org/jdk/compare/2ed35129...942342d5 Added `ProxyGenBenchmark` measuring generation time of 100 proxies. Results for master branch: BenchmarkMode Cnt Score Error Units Proxy.ProxyGenBench.generateProxiesss 10 12.266 ? 2.571 ms/op Results for this PR: BenchmarkMode Cnt Score Error Units Proxy.ProxyGenBench.generateProxiesss 10 9.851 ? 2.424 ms/op - PR Comment: https://git.openjdk.org/jdk/pull/19410#issuecomment-2144806554
Integrated: 8333301: Remove static builds using --enable-static-build
On Thu, 30 May 2024 19:14:43 GMT, Magnus Ihse Bursie wrote: > The original way of building static libraries in the JDK was to use the > configure argument --enable-static-build, which set the value of the make > variable STATIC_BUILD. (Note that this is not the same as the source code > definition STATIC_BUILD, which will be set by other means as well.) > > This method only ever worked on macOS, and has long since stopped working. It > was originally introduced for the Mobile Project, but I've now learn that not > even they use it anymore. > > It just adds clutter to the build system, and should be removed. This pull request has now been integrated. Changeset: f0bffbce Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/f0bffbce35bb06e724857e8651dd429c4f9df284 Stats: 218 lines in 14 files changed: 4 ins; 206 del; 8 mod 801: Remove static builds using --enable-static-build Reviewed-by: sgehwolf, erikj - PR: https://git.openjdk.org/jdk/pull/19487
Re: RFR: 8307818: Convert Indify tool to Classfile API [v10]
On Wed, 22 May 2024 22:38:16 GMT, Oussama Louati wrote: >> test/jdk/java/lang/invoke/CallStaticInitOrder.java line 54: >> >>> 52: >>> 53: static int Init1Tick; >>> 54: public static class Init1 { >> >> Is there a reason to change all the classes and methods to public? > > the test throws a java.lang.IllegalAccessError Changed to original access modifier - PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1624137975
Re: RFR: 8261242: [Linux] OSContainer::is_containerized() returns true when run outside a container [v3]
On Fri, 3 May 2024 16:05:30 GMT, Severin Gehwolf wrote: >> Please review this enhancement to the container detection code which allows >> it to figure out whether the JVM is actually running inside a container >> (`podman`, `docker`, `crio`), or with some other means that enforces >> memory/cpu limits by means of the cgroup filesystem. If neither of those >> conditions hold, the JVM runs in not containerized mode, addressing the >> issue described in the JBS tracker. For example, on my Linux system >> `is_containerized() == false" is being indicated with the following trace >> log line: >> >> >> [0.001s][debug][os,container] OSContainer::init: is_containerized() = false >> because no cpu or memory limit is present >> >> >> This state is being exposed by the Java `Metrics` API class using the new >> (still JDK internal) `isContainerized()` method. Example: >> >> >> java -XshowSettings:system --version >> Operating System Metrics: >> Provider: cgroupv1 >> System not containerized. >> openjdk 23-internal 2024-09-17 >> OpenJDK Runtime Environment (fastdebug build >> 23-internal-adhoc.sgehwolf.jdk-jdk) >> OpenJDK 64-Bit Server VM (fastdebug build >> 23-internal-adhoc.sgehwolf.jdk-jdk, mixed mode, sharing) >> >> >> The basic property this is being built on is the observation that the cgroup >> controllers typically get mounted read only into containers. Note that the >> current container tests assert that `OSContainer::is_containerized() == >> true` in various tests. Therefore, using the heuristic of "is any memory or >> cpu limit present" isn't sufficient. I had considered that in an earlier >> iteration, but many container tests failed. >> >> Overall, I think, with this patch we improve the current situation of >> claiming a containerized system being present when it's actually just a >> regular Linux system. >> >> Testing: >> >> - [x] GHA (risc-v failure seems infra related) >> - [x] Container tests on Linux x86_64 of cgroups v1 and cgroups v2 >> (including gtests) >> - [x] Some manual testing using cri-o >> >> Thoughts? > > Severin Gehwolf 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 12 additional > commits since the last revision: > > - Add doc for mountinfo scanning. > - Unify naming of variables > - Merge branch 'master' into jdk-8261242-is-containerized-fix > - Merge branch 'master' into jdk-8261242-is-containerized-fix > - jcheck fixes > - Fix tests > - Implement Metrics.isContainerized() > - Some clean-up > - Drop cgroups testing on plain Linux > - Implement fall-back logic for non-ro controller mounts > - ... and 2 more: https://git.openjdk.org/jdk/compare/88976cae...434430ca Keep open bot. Thanks. - PR Comment: https://git.openjdk.org/jdk/pull/18201#issuecomment-2144706137
Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Fri, 31 May 2024 14:05:07 GMT, Alexey Semenyuk wrote: > Fix MainClassTest class to use HelloApp.AppOutputVerifier class to run app > launcher instead of raw Executor. This makes MainClassTest test run app > launchers with retries. This change addresses the primary issue. > > Fix inconsistencies in HelloApp.AppOutputVerifier class. It used to provide > API allowing to run launchers without retries. It inconsistently allowed the > execution of launchers with suppressed output (stdout and stderr). It > inconsistently executed launchers with/without PATH removed from the > environment. > > These loopholes were eliminated: > > - stdout and stderr of app launchers is never suppressed; > - PATH env variable is always deleted for app launchers on Windows. It is > not deleted on other platforms. This change sets the correct scope of > [JDK-8254920](https://bugs.openjdk.org/browse/JDK-8254920) fix that > introduced the removal of PATH env variable for app launchers; > - app launchers are always executed with retries unless the launcher is > executed with `jpackage.test.noexit` system property set to `true` indicating > the test app will not terminate on its own. > > Other changes are due to changes in HelloApp.AppOutputVerifier class. I am yet to see anything that actually explains the cause of the `dlerror` crash here ??? - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2144616488
Re: RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor} [v6]
On Fri, 31 May 2024 18:39:33 GMT, jengebr wrote: >> Improve `java/lang/reflect/Method.java` by eliminating needless cloning of >> Class[0] instances. This cloning is intended to prevent callers from >> changing array contents, but many Methods have zero exceptions or zero >> parameters, and returning the original `Class[0]` is sufficient. >> >> Results from the included JMH benchmark: >> Before: >> >> BenchmarkMode Cnt Score Error Units >> ConstructorBenchmark.getExceptionTypes avgt5 6.526 ± 0.183 ns/op >> ConstructorBenchmark.getExceptionTypesEmpty avgt5 5.803 ± 0.073 ns/op >> ConstructorBenchmark.getParameterTypes avgt5 6.521 ± 0.188 ns/op >> ConstructorBenchmark.getParameterTypesEmpty avgt5 5.747 ± 0.087 ns/op >> MethodBenchmark.getExceptionTypesavgt5 6.525 ± 0.163 ns/op >> MethodBenchmark.getExceptionTypesEmpty avgt5 5.783 ± 0.130 ns/op >> MethodBenchmark.getParameterTypesavgt5 6.518 ± 0.195 ns/op >> MethodBenchmark.getParameterTypesEmpty avgt5 5.742 ± 0.028 ns/op >> >> >> After: >> >> BenchmarkMode Cnt Score Error Units >> ConstructorBenchmark.getExceptionTypes avgt5 6.590 ± 0.124 ns/op >> ConstructorBenchmark.getExceptionTypesEmpty avgt5 1.351 ± 0.061 ns/op >> ConstructorBenchmark.getParameterTypes avgt5 6.651 ± 0.132 ns/op >> ConstructorBenchmark.getParameterTypesEmpty avgt5 1.353 ± 0.150 ns/op >> MethodBenchmark.getExceptionTypesavgt5 6.701 ± 0.151 ns/op >> MethodBenchmark.getExceptionTypesEmpty avgt5 1.422 ± 0.025 ns/op >> MethodBenchmark.getParameterTypesavgt5 6.629 ± 0.142 ns/op >> MethodBenchmark.getParameterTypesEmpty avgt5 1.273 ± 0.169 ns/op > > jengebr has updated the pull request incrementally with one additional commit > since the last revision: > > Fixing nits in benchmark I believe GHA failure (serviceability/jvmti/ObjectMonitorUsage/ObjectMonitorUsage, x86_32) is unrelated to this change, it is a known issue fixed in current master. We can proceed with integration. - PR Comment: https://git.openjdk.org/jdk/pull/19327#issuecomment-2144546824
Re: RFR: 8333396: Performance regression of new DecimalFormat and DecimalFormat.format [v2]
> ### Performance regression of DecimalFormat.format > From the output of perf, we can see the hottest regions contain atomic > instructions. But when run with JDK 11, there is no such problem. The reason > is the removed biased locking. > The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself > contains many synchronized methods. > So I added support for some new methods that accept StringBuilder which is > lock-free. > > ### Performance regression of new DecimalFormat > After comparing the flame graph between current jdk and jdk 11, the method > java.text.DecimalFormatSymbols#findNonFormatChar takes a significant time. > The performance becomes as good as jdk11 after replacing it with a simple > loop implementation. > > > > ### Test result > > @BenchmarkMode(Mode.AverageTime) > @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) > @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS) > @State(Scope.Thread) > @OutputTimeUnit(TimeUnit.NANOSECONDS) > public class JmhDecimalFormat { > > private DecimalFormat format; > > @Setup(Level.Trial) > public void setup() { > format = new DecimalFormat("#0.0"); > } > > @Benchmark > public void testNewAndFormat() throws InterruptedException { > new DecimalFormat("#0.0").format(9524234.1236457); > } > > @Benchmark > public void testNewOnly() throws InterruptedException { > new DecimalFormat("#0.0"); > } > > @Benchmark > public void testFormatOnly() throws InterruptedException { > format.format(9524234.1236457); > } > } > > Current JDK before optimize > > Benchmark Mode CntScore Error Units > JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op > JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op > JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op > > > > Current JDK after optimize > > Benchmark Mode CntScore Error Units > JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op > JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op > JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op > > > ### JDK 11 > > Benchmark Mode CntScore Error Units > JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op > JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op > JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op lingjun-cg has updated the pull request incrementally with one additional commit since the last revision: 896: Performance regression of new DecimalFormat and DecimalFormat.format - Changes: - all: https://git.openjdk.org/jdk/pull/19513/files - new: https://git.openjdk.org/jdk/pull/19513/files/a6755f8f..b48962b5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19513=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19513=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19513.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513 PR: https://git.openjdk.org/jdk/pull/19513