Re: RFR: 8331977: Crash: SIGSEGV in dlerror()

[David Holmes]

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

[Vladimir Ivanov]

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]

[Vladimir Ivanov]

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

[Alexander Matveev]

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

[lingjun-cg]

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]

[lingjun-cg]

> ### 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

[Jaikiran Pai]

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

[Jaikiran Pai]

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]

[lingjun-cg]

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

[Alexey Semenyuk]

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]

[Nizar Benalla]

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]

[Naoto Sato]

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

[Justin Lu]

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

[Alexey Semenyuk]

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

[Alexey Semenyuk]

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]

[Chen Liang]

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

[Justin Lu]

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

[Alexander Matveev]

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

[Joe Darcy]

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

[Kurt Alfred Kluever]

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]

[Jorn Vernee]

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

[Alexey Semenyuk]

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

[Alexey Semenyuk]

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]

[Vladimir Ivanov]

> 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

[Vladimir Ivanov]

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]

[Alan Bateman]

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

[jengebr]

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

[Doug Lea]

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

[jengebr]

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

[Aleksey Shipilev]

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

[Nizar Benalla]

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

[Sean Gwizdak]

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]

[Sean Gwizdak]

> 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]

[Jonathan Gibbons]

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

[Naoto Sato]

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]

[Naoto Sato]

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]

[Severin Gehwolf]

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"?

[Volker Simonis]

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]

[Adam Sotona]

> [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()

[Alexey Semenyuk]

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]

[Adam Sotona]

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]

[Adam Sotona]

> [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}

[jengebr]

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]

[Alan Bateman]

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]

[Viktor Klang]

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]

[Adam Sotona]

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]

[Adam Sotona]

> [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]

[Adam Sotona]

> [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]

[Claes Redestad]

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]

[Doug Lea]

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]

[Adam Sotona]

> [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]

[Adam Sotona]

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]

[Lance Andersen]

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

[Lance Andersen]

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]

[Claes Redestad]

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]

[Severin Gehwolf]

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]

[Adam Sotona]

> [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]

[Adam Sotona]

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

[Magnus Ihse Bursie]

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]

[Oussama Louati]

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]

[Severin Gehwolf]

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()

[David Holmes]

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]

[Aleksey Shipilev]

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]

[lingjun-cg]

> ### 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