Re: RFR: 8330182: Start of release updates for JDK 24 [v5]

[Iris Clark]

On Mon, 20 May 2024 22:42:20 GMT, Joe Darcy  wrote:

>> Get JDK 24 underway.
>
> Joe Darcy has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 11 commits:
> 
>  - Fix-up test.
>  - Merge branch 'master' into JDK-8330188
>  - Adjust test for deprecated options.
>  - Merge branch 'master' into JDK-8330188
>  - Update deprecated options test.
>  - Merge branch 'master' into JDK-8330188
>  - Merge branch 'master' into JDK-8330188
>  - Merge branch 'master' into JDK-8330188
>  - Correct release date as observed in review feedback.
>  - Improve javadoc of class file update.
>  - ... and 1 more: https://git.openjdk.org/jdk/compare/d6b7f9b1...128f53a3

Still looks good.

-

Marked as reviewed by iris (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18787#pullrequestreview-2067401131

Re: RFR: 8332086: Remove the usage of ServiceLoader in j.u.r.RandomGeneratorFactory [v7]

[Joe Darcy]

On Fri, 17 May 2024 08:28:15 GMT, Raffaello Giulietti  
wrote:

>> All random number generator algorithms are implemented in module 
>> `java.base`. The usage of `ServiceLoader` in `j.u.r.RandomGeneratorFactory` 
>> is no longer needed.
>
> Raffaello Giulietti has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   SecureRandom added to the table in package documentation.

src/java.base/share/classes/java/util/random/package-info.java line 129:

> 127:  *
> 128:  *  Random number generator algorithms are organized in groups,
> 129:  * as described below.

FYI, it is possible to link to an anchor in a package-info file using syntax 
like

`{@linkplain java.util.random##algorithms below}`

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19212#discussion_r1607475198

Re: RFR: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr [v2]

[David Holmes]

On Mon, 20 May 2024 11:41:48 GMT, Axel Boldt-Christmas  
wrote:

>> test/lib/jdk/test/lib/process/OutputAnalyzer.java line 691:
>> 
>>> 689:  * @throws RuntimeException If the pattern was not found
>>> 690:  */
>>> 691: public OutputAnalyzer 
>>> stderrShouldMatchIgnoreDeprecatedWarnings(String pattern) {
>> 
>> Given we have `...IgnoreVMWarnings` this special case should really be 
>> called `...IgnoreDeprecatedVMWarnings`.
>
> The name was chosen based on: 
> https://github.com/openjdk/jdk/blob/77c8516085225a04bd5a954197fc5ef7e5c5ee61/test/lib/jdk/test/lib/process/OutputAnalyzer.java#L184
> 
> Should I still change it?

No. Sorry I missed the fact the inconsistency had already been introduced. And 
given that function exists we may as well add the new one too.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19297#discussion_r1607458481

Re: RFR: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr [v2]

[David Holmes]

On Mon, 20 May 2024 07:27:19 GMT, Axel Boldt-Christmas  
wrote:

>> Improve `java/util/zip/EntryCount64k.java` stderr parsing by ignoring 
>> deprecated warnings. Testing non-generational ZGC requires the use of a 
>> deprecated option.
>
> Axel Boldt-Christmas has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright year

looks good. Thanks

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19297#pullrequestreview-2067288156

Re: RFR: 8332547: Unloaded signature classes in DirectMethodHandles

[Chen Liang]

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

src/java.base/share/classes/sun/invoke/util/VerifyAccess.java line 291:

> 289: // guarantees that classes with names beginning "java." cannot 
> be aliased,
> 290: // because class loaders cannot load them directly. However, it 
> is beneficial
> 291: // for JIT-compilers to ensure all signature classes are loaded.

Since we anticipate this method to perform side effects, can we rename all of 
these non-pure `isTypeVisible` to `seeType`/`accessType` to indicate this 
desired side effect?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19319#discussion_r1607426203

Re: RFR: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations

[Chen Liang]

On Mon, 20 May 2024 10:52:27 GMT, Claes Redestad  wrote:

> We can fold the call to `Objects.checkIndex` into the code generated in 
> generateTypeSwitchSkeleton instead of doing so by filtering the MH argument. 
> This loads 9 less classes (of which 8 generated LFs and Species classes) on a 
> minimal test, while being neutral on a throughput sanity test:
> 
> 
> Name   Cnt  Base   Error   Test   Error  Unit  Change
> SwitchSanity.switchSum  15 8,162 ± 0,117  8,152 ± 0,131 ns/op   1,00x (p = 
> 0,800 )
>   * = significant
>   ```
> 
> A few additional optimizations includes generating the switch method using 
> the precise type (to avoid the need for an explicitCast adaptation), and 
> moving some seldom used `findStatic` calls to a holder. All in all this means 
> a reduction by 33-34M cycles to bootstrap a trivial switch expression on my 
> M1.

Changes requested by liach (Author).

src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 630:

> 628:.withMethodBody("typeSwitch",
> 629:
> MethodTypeDescImpl.ofValidated(ConstantDescs.CD_int,
> 630: 
> ClassDesc.ofDescriptor(selectorType.descriptorString()),

`selectorType` can be primitive, therefore verification errors arise at `aload` 
in the generated code. This is why tests fail on GitHub actions.

-

PR Review: https://git.openjdk.org/jdk/pull/19307#pullrequestreview-2067245226
PR Review Comment: https://git.openjdk.org/jdk/pull/19307#discussion_r1607425026

Re: RFR: 8330182: Start of release updates for JDK 24 [v5]

[Joe Darcy]

> Get JDK 24 underway.

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The pull request now contains 11 commits:

 - Fix-up test.
 - Merge branch 'master' into JDK-8330188
 - Adjust test for deprecated options.
 - Merge branch 'master' into JDK-8330188
 - Update deprecated options test.
 - Merge branch 'master' into JDK-8330188
 - Merge branch 'master' into JDK-8330188
 - Merge branch 'master' into JDK-8330188
 - Correct release date as observed in review feedback.
 - Improve javadoc of class file update.
 - ... and 1 more: https://git.openjdk.org/jdk/compare/d6b7f9b1...128f53a3

-

Changes: https://git.openjdk.org/jdk/pull/18787/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=18787=04
  Stats: 107 lines in 37 files changed: 46 ins; 7 del; 54 mod
  Patch: https://git.openjdk.org/jdk/pull/18787.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18787/head:pull/18787

PR: https://git.openjdk.org/jdk/pull/18787

RFR: 8332547: Unloaded signature classes in DirectMethodHandles

[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

-

Commit messages:
 - Fix
 - Test

Changes: https://git.openjdk.org/jdk/pull/19319/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=19319=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332547
  Stats: 74 lines in 2 files changed: 68 ins; 0 del; 6 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: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations

[Claes Redestad]

On Mon, 20 May 2024 18:06:32 GMT, Chen Liang  wrote:

>> We can fold the call to `Objects.checkIndex` into the code generated in 
>> generateTypeSwitchSkeleton instead of doing so by filtering the MH argument. 
>> This loads 9 less classes (of which 8 generated LFs and Species classes) on 
>> a minimal test, while being neutral on a throughput sanity test:
>> 
>> 
>> Name   Cnt  Base   Error   Test   Error  Unit  Change
>> SwitchSanity.switchSum  15 8,162 ± 0,117  8,152 ± 0,131 ns/op   1,00x (p = 
>> 0,800 )
>>   * = significant
>>   ```
>> 
>> A few additional optimizations includes generating the switch method using 
>> the precise type (to avoid the need for an explicitCast adaptation), and 
>> moving some seldom used `findStatic` calls to a holder. All in all this 
>> means a reduction by 33-34M cycles to bootstrap a trivial switch expression 
>> on my M1.
>
> src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 406:
> 
>> 404: cb.iload(RESTART_IDX);
>> 405: cb.loadConstant(labelConstants.length + 1);
>> 406: cb.invokestatic(CD_Objects, "checkIndex", 
>> MethodTypeDesc.of(ConstantDescs.CD_int, ConstantDescs.CD_int, 
>> ConstantDescs.CD_int));
> 
> We should cache this MethodTypeDesc too.

`MethodTypeDesc.of` is actually quite cheap when inputs are `ClassDesc`s. 
Besides I think the main focus in these bootstraps should be improving the 
overhead of the first few calls - reduce dependencies, reduce runtime code 
generation - not treat this code as something that will be run many times over 
in a hot loop.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19307#discussion_r1607207823

Re: RFR: 8307818: Convert Indify tool to Classfile API [v8]

[Oussama Louati]

> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
> MethodType, and CallSite constants.
> It currently uses ad-hoc code to process class files and intends to migrate 
> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
> instead.

Oussama Louati has updated the pull request incrementally with one additional 
commit since the last revision:

  update: Added javadoc documentation

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18841/files
  - new: https://git.openjdk.org/jdk/pull/18841/files/91dbb74c..781c951d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18841=07
 - incr: https://webrevs.openjdk.org/?repo=jdk=18841=06-07

  Stats: 27 lines in 1 file changed: 26 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/18841.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18841/head:pull/18841

PR: https://git.openjdk.org/jdk/pull/18841

Re: RFR: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations

[Chen Liang]

On Mon, 20 May 2024 10:52:27 GMT, Claes Redestad  wrote:

> We can fold the call to `Objects.checkIndex` into the code generated in 
> generateTypeSwitchSkeleton instead of doing so by filtering the MH argument. 
> This loads 9 less classes (of which 8 generated LFs and Species classes) on a 
> minimal test, while being neutral on a throughput sanity test:
> 
> 
> Name   Cnt  Base   Error   Test   Error  Unit  Change
> SwitchSanity.switchSum  15 8,162 ± 0,117  8,152 ± 0,131 ns/op   1,00x (p = 
> 0,800 )
>   * = significant
>   ```
> 
> A few additional optimizations includes generating the switch method using 
> the precise type (to avoid the need for an explicitCast adaptation), and 
> moving some seldom used `findStatic` calls to a holder. All in all this means 
> a reduction by 33-34M cycles to bootstrap a trivial switch expression on my 
> M1.

src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 406:

> 404: cb.iload(RESTART_IDX);
> 405: cb.loadConstant(labelConstants.length + 1);
> 406: cb.invokestatic(CD_Objects, "checkIndex", 
> MethodTypeDesc.of(ConstantDescs.CD_int, ConstantDescs.CD_int, 
> ConstantDescs.CD_int));

We should cache this MethodTypeDesc too.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19307#discussion_r1607097291

Re: RFR: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations

[Claes Redestad]

On Mon, 20 May 2024 10:52:27 GMT, Claes Redestad  wrote:

> We can fold the call to `Objects.checkIndex` into the code generated in 
> generateTypeSwitchSkeleton instead of doing so by filtering the MH argument. 
> This loads 9 less classes (of which 8 generated LFs and Species classes) on a 
> minimal test, while being neutral on a throughput sanity test:
> 
> 
> Name   Cnt  Base   Error   Test   Error  Unit  Change
> SwitchSanity.switchSum  15 8,162 ± 0,117  8,152 ± 0,131 ns/op   1,00x (p = 
> 0,800 )
>   * = significant
>   ```
> 
> A few additional optimizations includes generating the switch method using 
> the precise type (to avoid the need for an explicitCast adaptation), and 
> moving some seldom used `findStatic` calls to a holder. All in all this means 
> a reduction by 33-34M cycles to bootstrap a trivial switch expression on my 
> M1.

To help evaluate and diagnose the startup overheads I added a `main` method to 
the provided `SwitchSanity` micro. This only runs a single invocation and is 
easy to run using your startup benchmarking script/tool of choice:


Name Cnt  Base Error   Test Error   
  Unit  Change
Perfstartup-JMH   2075,000 ±   4,455 62,000 ±   6,042   
 ms/op   1,21x (p = 0,000*)
  :.cycles   277393501,850 ± 4422757,249  253819501,350 ± 5482340,325   
cycles   0,92x (p = 0,000*)
  :.instructions 701618995,500 ± 3774721,992  622929549,250 ± 3634060,803 
instructions   0,89x (p = 0,000*)
  :.taskclock   82,000 ±   3,564 74,000 ±   4,365   
ms   0,90x (p = 0,000*)
  * = significant

-

PR Comment: https://git.openjdk.org/jdk/pull/19307#issuecomment-2121179185

RFR: 8332528: Generate code in SwitchBootstraps.generateTypeSwitch that require fewer adaptations

[Claes Redestad]

We can fold the call to `Objects.checkIndex` into the code generated in 
generateTypeSwitchSkeleton instead of doing so by filtering the MH argument. 
This loads 9 less classes (of which 8 generated LFs and Species classes) on a 
minimal test, while being neutral on a throughput sanity test:


Name   Cnt  Base   Error   Test   Error  Unit  Change
SwitchSanity.switchSum  15 8,162 ± 0,117  8,152 ± 0,131 ns/op   1,00x (p = 
0,800 )
  * = significant
  ```

A few additional optimizations includes generating the switch method using the 
precise type (to avoid the need for an explicitCast adaptation), and moving 
some seldom used `findStatic` calls to a holder. All in all this means a 
reduction by 33-34M cycles to bootstrap a trivial switch expression on my M1.

-

Commit messages:
 - Remove explicitCastArguments and refactor
 - Move rarely used findStatics to Holder class
 - Drive-by desugaring
 - Move Objects.checkIndex call to code generated by generateTypeSwitch

Changes: https://git.openjdk.org/jdk/pull/19307/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=19307=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332528
  Stats: 157 lines in 4 files changed: 113 ins; 18 del; 26 mod
  Patch: https://git.openjdk.org/jdk/pull/19307.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19307/head:pull/19307

PR: https://git.openjdk.org/jdk/pull/19307

Re: RFR: 8331196: vector api: Remove unnecessary index check in Byte/ShortVector.fromArray/fromArray0Template

[Paul Sandoz]

On Fri, 26 Apr 2024 14:06:02 GMT, Hamlin Li  wrote:

> Hi,
> Can you help to review this simple patch?
> Some index check in Byte/ShortVector.fromArray/fromArray0Template seems not 
> necessary, could be removed.
> Thanks

That does not look correct and will only check a prefix indexes. A `ByteVector` 
with a shape of 256 bits has 32 lanes, whereas an `IntVector` of the same shape 
has 8 lanes. The `mapOffset` array will hold 32 indexes that need checking, so 
we need to loop through `mapOffset` array four times.

-

PR Comment: https://git.openjdk.org/jdk/pull/18977#issuecomment-2121057883

Re: RFR: 8332486: ClassFile API ArrayIndexOutOfBoundsException with label metadata

[Paul Sandoz]

On Mon, 20 May 2024 08:37:49 GMT, Adam Sotona  wrote:

> Parsing of a specifically corrupted class file cause unexpected 
> `ArrayIndexOutOfBoundsException` during label inflation.
> This patch checks the valid range and throws expected 
> `IllegalArgumentException` instead.
> Relevant test is added.
> 
> Please review.
> 
> Thanks,
> Adam

Marked as reviewed by psandoz (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19304#pullrequestreview-2066846853

Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]

[Alan Bateman]

On Mon, 20 May 2024 18:39:31 GMT, Phil Race  wrote:

>> make/conf/module-loader-map.conf line 105:
>> 
>>> 103: java.smartcardio \
>>> 104: jdk.accessibility \
>>> 105: jdk.attach \
>> 
>> The list of allowed modules has been rewritten from scratch, by looking at 
>> the set of modules containing at least one `native` method declaration.
>
> Should I understand this list to be the set of modules exempt from needing to 
> specific that native access is allowed ?
> ie they always have native access without any warnings, and further that any 
> attempt to enable warnings, or to disable native access for these modules is 
> ignored ?

Yes, this was added via JDK-8327218. The changes in this PR are just trimming 
down the list to only the modules that have native code.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1607147983

Integrated: 8332154: Memory leak in SynchronousQueue

[Viktor Klang]

On Thu, 16 May 2024 14:54:52 GMT, Viktor Klang  wrote:

> Local testing seems to indicate that this fix (which mirrors what's done in 
> the FIFO mode) addresses the problem. Regression test added for JDK20+

This pull request has now been integrated.

Changeset: b78613b6
Author:Viktor Klang 
URL:   
https://git.openjdk.org/jdk/commit/b78613b6813a85662fb2af2004d0b68002fe471d
Stats: 99 lines in 4 files changed: 98 ins; 1 del; 0 mod

8332154: Memory leak in SynchronousQueue

Reviewed-by: alanb

-

PR: https://git.openjdk.org/jdk/pull/19271

Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]

[Phil Race]

On Fri, 17 May 2024 13:38:25 GMT, Maurizio Cimadamore  
wrote:

>> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting 
>> the use of JNI in the following ways:
>> 
>> * `System::load` and `System::loadLibrary` are now restricted methods
>> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods
>> * binding a JNI `native` method declaration to a native implementation is 
>> now considered a restricted operation
>> 
>> This PR slightly changes the way in which the JDK deals with restricted 
>> methods, even for FFM API calls. In Java 22, the single 
>> `--enable-native-access` was used both to specify a set of modules for which 
>> native access should be allowed *and* to specify whether illegal native 
>> access (that is, native access occurring from a module not specified by 
>> `--enable-native-access`) should be treated as an error or a warning. More 
>> specifically, an error is only issued if the `--enable-native-access flag` 
>> is used at least once.
>> 
>> Here, a new flag is introduced, namely 
>> `illegal-native-access=allow/warn/deny`, which is used to specify what 
>> should happen when access to a restricted method and/or functionality is 
>> found outside the set of modules specified with `--enable-native-access`. 
>> The default policy is `warn`, but users can select `allow` to suppress the 
>> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This 
>> aligns the treatment of restricted methods with other mechanisms, such as 
>> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`.
>> 
>> Some changes were required in the package-info javadoc for 
>> `java.lang.foreign`, to reflect the changes in the command line flags 
>> described above.
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address review comments

Have you looked into / thought about how this will work for jpackaged apps ?
I suspect that both the existing FFM usage and this will be options the 
application packager will need to supply when building the jpackaged app - the 
end user cannot pass in command line VM options.
Seems there should be some testing of this as some kind of native access could 
be a common case for jpackaged apps.

-

PR Review: https://git.openjdk.org/jdk/pull/19213#pullrequestreview-2066794950

Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]

[Phil Race]

On Mon, 13 May 2024 10:49:30 GMT, Maurizio Cimadamore  
wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address review comments
>
> make/conf/module-loader-map.conf line 105:
> 
>> 103: java.smartcardio \
>> 104: jdk.accessibility \
>> 105: jdk.attach \
> 
> The list of allowed modules has been rewritten from scratch, by looking at 
> the set of modules containing at least one `native` method declaration.

Should I understand this list to be the set of modules exempt from needing to 
specific that native access is allowed ?
ie they always have native access without any warnings, and further that any 
attempt to enable warnings, or to disable native access for these modules is 
ignored ?

> src/java.desktop/macosx/classes/com/apple/eio/FileManager.java line 61:
> 
>> 59: }
>> 60: 
>> 61: @SuppressWarnings({"removal", "restricted"})
> 
> There are several of these changes. One option might have been to just 
> disable restricted warnings when building. But on a deeper look, I realized 
> that in all these places we already disabled deprecation warnings for the use 
> of security manager, so I also added a new suppression instead.

Sounds reasonable.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1607136237
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1607136808

Re: RFR: 8331865: Consolidate size and alignment checks in LayoutPath [v2]

[Maurizio Cimadamore]

On Mon, 20 May 2024 16:42:19 GMT, Paul Sandoz  wrote:

> some additional overhead e.g.,
> 
> ```
> if (derefAdapters.length == 0) {
> // insert align check for the root layout on the initial MS + 
> offset
> List> coordinateTypes = handle.coordinateTypes();
> MethodHandle alignCheck = 
> MethodHandles.insertArguments(MH_CHECK_ENCL_LAYOUT, 2, rootLayout());
> handle = MethodHandles.collectCoordinates(handle, 0, alignCheck);
> int[] reorder = IntStream.concat(IntStream.of(0, 1), 
> IntStream.range(0, coordinateTypes.size())).toArray();
> handle = MethodHandles.permuteCoordinates(handle, 
> coordinateTypes, reorder);
> }
> ```

True, the chain for segment var handle is quite long (and that is not a result 
of this patch, it has always been more or less like that). Funnily, I was 
staring at this piece of code the other day, and I think this can be dealt with 
morally with a `foldArguments`, but we don't have the equivalent of that in the 
VH/coordinates world. Maybe we should add that (or at least implement 
internally) as that would simplify the adaptation, avoiding the permute step 
(which is typically rather heavy).

-

PR Comment: https://git.openjdk.org/jdk/pull/19251#issuecomment-2120979087

RFR: 8332161: Need a test for restoring echo in the Console implementation (java.base)

[Naoto Sato]

This test intends to verify the behavior of JdkConsole for the java.base 
module, wrt restoring the echo. The test assumes the internal methods that 
sets/gets the echo status of the platform.

-

Depends on: https://git.openjdk.org/jdk/pull/19184

Commit messages:
 - initial commit

Changes: https://git.openjdk.org/jdk/pull/19315/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=19315=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332161
  Stats: 196 lines in 2 files changed: 196 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/19315.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19315/head:pull/19315

PR: https://git.openjdk.org/jdk/pull/19315

Re: RFR: 8332084: Ensure JdkConsoleImpl.restoreEcho visibility in a shutdown hook [v7]

[Naoto Sato]

> Making sure `restoreEcho` correctly reflects the state in the shutdown 
> thread, which differs from the application's thread that issues the 
> `readPassword()` method.

Naoto Sato 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 ten additional commits since the 
last revision:

 - Merge branch 'master' into JDK-8332084-restoreEcho-readLock
 - Merge branch 'master' into JDK-8332084-restoreEcho-readLock
 - copyright year
 - Merge branch 'master' into JDK-8332084-restoreEcho-readLock
 - get/setEcho()
 - Addresses a review comment
 - Replaced another unused exception with _
 - Update src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java
   
   Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
 - initial commit

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19184/files
  - new: https://git.openjdk.org/jdk/pull/19184/files/82d30b3b..e58fbdcd

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19184=06
 - incr: https://webrevs.openjdk.org/?repo=jdk=19184=05-06

  Stats: 41521 lines in 532 files changed: 34514 ins; 5047 del; 1960 mod
  Patch: https://git.openjdk.org/jdk/pull/19184.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19184/head:pull/19184

PR: https://git.openjdk.org/jdk/pull/19184

Re: RFR: 8332340: Add JavacBench as a test case for CDS [v2]

[Ioi Lam]

On Fri, 17 May 2024 19:45:00 GMT, Calvin Cheung  wrote:

>> Ioi Lam has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   (1) comments from @liach; (2) added javadoc; (3) Use 
>> createTestJavaProcessBuilder() instead of 
>> createLimitedTestJavaProcessBuilder()
>
> test/hotspot/jtreg/runtime/cds/appcds/applications/JavacBench.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights 
>> reserved.
> 
> Since this is a new file, should the copyright year be only 2024?
> (same for other files)

The file was first added in the Leyden repo in 2023, so I think we should use 
that as the initial copyright year.

> test/hotspot/jtreg/runtime/cds/appcds/applications/JavacBench.java line 43:
> 
>> 41: import jdk.test.lib.cds.CDSAppTester;
>> 42: import jdk.test.lib.helpers.ClassFileInstaller;
>> 43: import jdk.test.lib.process.OutputAnalyzer;
> 
> Is this import needed?

Removed.

> test/hotspot/jtreg/runtime/cds/appcds/applications/JavacBenchApp.java line 
> 222:
> 
>> 220: }
>> 221: long elapsed = System.currentTimeMillis() - started;
>> 222: if (System.getProperty("JavacBenchApp.silent") == null) {
> 
> Should line 221 be inside the 'if' block?

I moved it inside the 'if' block.

> test/lib/jdk/test/lib/cds/CDSAppTester.java line 218:
> 
>> 216: }
>> 217: 
>> 218: throw new RuntimeException("Must have exactly one command line 
>> argument: STATIC or DYNAMIC");
> 
> Why not check the number of args at the beginning of the method?

Fixed.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1607047769
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1607047882
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1607047859
PR Review Comment: https://git.openjdk.org/jdk/pull/19256#discussion_r1607047805

Re: RFR: 8332340: Add JavacBench as a test case for CDS [v3]

[Ioi Lam]

> JavacBench is a test program that compiles 90 Java source files. It uses a 
> fair amount of invokedynamic callsites, so it's good for testing CDS support 
> for indy and lambda expressions.
> 
> This test was first integrated into the 
> [leyden](https://github.com/openjdk/leyden/tree/premain) repo. Hence some of 
> the files have copyrights in 2023.

Ioi Lam has updated the pull request incrementally with one additional commit 
since the last revision:

  @calvinccheung comments

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19256/files
  - new: https://git.openjdk.org/jdk/pull/19256/files/354665a3..ad97efa1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19256=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=19256=01-02

  Stats: 14 lines in 3 files changed: 5 ins; 6 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/19256.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19256/head:pull/19256

PR: https://git.openjdk.org/jdk/pull/19256

Re: RFR: 8331865: Consolidate size and alignment checks in LayoutPath [v2]

[Paul Sandoz]

On Mon, 20 May 2024 16:31:18 GMT, Maurizio Cimadamore  
wrote:

> 
> Ah, got it. You mean more support in VarHandleGuards. Yes, I have a separate 
> patch for that (actually had that for quite a while), but we're not super 
> sure how to evaluate what impact it has :-)

Ah, i did not realize that. Yes its tricky, it was designed for VHs to 
fields/arrays, to really minimize their overhead. With segments there is 
already some additional overhead e.g.,

if (derefAdapters.length == 0) {
// insert align check for the root layout on the initial MS + offset
List> coordinateTypes = handle.coordinateTypes();
MethodHandle alignCheck = 
MethodHandles.insertArguments(MH_CHECK_ENCL_LAYOUT, 2, rootLayout());
handle = MethodHandles.collectCoordinates(handle, 0, alignCheck);
int[] reorder = IntStream.concat(IntStream.of(0, 1), 
IntStream.range(0, coordinateTypes.size())).toArray();
handle = MethodHandles.permuteCoordinates(handle, coordinateTypes, 
reorder);
}

So perhaps it does not make much difference.

-

PR Comment: https://git.openjdk.org/jdk/pull/19251#issuecomment-2120813942

Re: RFR: 8331865: Consolidate size and alignment checks in LayoutPath [v2]

[Maurizio Cimadamore]

On Mon, 20 May 2024 09:45:31 GMT, Maurizio Cimadamore  
wrote:

> > Separately, we might be missing a few long argument accepting guard methods 
> > for simpler cases as I suspect they are still focused more on int index 
> > types.
> 
> Not sure I understand what guard methods you are referring to here?

Ah, got it. You mean more support in VarHandleGuards. Yes, I have a separate 
patch for that (actually had that for quite a while), but we're not super sure 
how to evaluate what impact it has :-)

-

PR Comment: https://git.openjdk.org/jdk/pull/19251#issuecomment-2120796446

Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]

[Joe Wang]

On Mon, 20 May 2024 13:03:36 GMT, Sean Mullan  wrote:

>> In XML parlance, a "processor" is an aggregation of parsers, serializers, 
>> and other things that contribute to the processing. So I think it could be 
>> either here, but you have a point and if it stays as "processor" then it 
>> should link #FacPro where the term is defined.
>
> It's the wording that doesn't sound right, before this you have "making" 
> which doesn't sound right with "process". Maybe it needs two sentences.

Thanks! Same as Alan, I thought you were talking about "processor" as well on 
the first glance, then realized you're referring to the parallel statements :-)

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606995925

Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v11]

[Joe Wang]

> Add two sample configuration files:
> 
>   jaxp-strict.properties: used to set strict configuration, stricter than 
> jaxp.properties in previous versions such as JDK 22
> 
>>   jaxp-compat.properties: used to regain compatibility from any more 
>> restricted configuration than previous versions such as JDK 22
> 
> Updated on 5/16/2024
> 
> Design change:
> The design is changed to include in the JDK two configuration files that are 
> the default jaxp.properties and jaxp-strict.properties, instead of three, 
> dropping jaxp-compat.properties.
> 
> Updated on 5/18/2024
> 
> Withdraw changes to jaxp.properties. The original idea was to match 
> jaxp-strict.properties with regard to the properties. However, that change 
> impact the configuration process, resulting in tests that verify the process 
> to fail.

Joe Wang has updated the pull request incrementally with one additional commit 
since the last revision:

  updated jaxp-strict; fixed typo in module-info.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18831/files
  - new: https://git.openjdk.org/jdk/pull/18831/files/dfc965c6..55a86db3

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18831=10
 - incr: https://webrevs.openjdk.org/?repo=jdk=18831=09-10

  Stats: 5 lines in 2 files changed: 0 ins; 2 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/18831.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831

PR: https://git.openjdk.org/jdk/pull/18831

Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]

[Joe Wang]

On Mon, 20 May 2024 07:13:02 GMT, Alan Bateman  wrote:

>> Joe Wang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   withdraw changes to jaxp.properties. The configuration process has not 
>> changed, changing the default configuration would result in many failures 
>> that test the process.
>
> src/java.xml/share/conf/jaxp-strict.properties line 17:
> 
>> 15: # java -Djava.xml.config.file=$JAVA_HOME/conf/jaxp-strict.properties
>> 16: #
>> 17: # The pathname to the configuration file must be valid. If it is not 
>> absolute,
> 
> I think it would be better to drop this paragraph or else just replace it 
> with a sentence to say that the java.xml module description specifies the 
> system property.

Thanks Alan. Replaced the paragraph with a reference to the module description.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606993156

Re: RFR: 8307818: Convert Indify tool to Classfile API [v7]

[Oussama Louati]

> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
> MethodType, and CallSite constants.
> It currently uses ad-hoc code to process class files and intends to migrate 
> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
> instead.

Oussama Louati has updated the pull request incrementally with one additional 
commit since the last revision:

  update: updates indify

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18841/files
  - new: https://git.openjdk.org/jdk/pull/18841/files/3f41a98f..91dbb74c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18841=06
 - incr: https://webrevs.openjdk.org/?repo=jdk=18841=05-06

  Stats: 80 lines in 1 file changed: 2 ins; 21 del; 57 mod
  Patch: https://git.openjdk.org/jdk/pull/18841.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18841/head:pull/18841

PR: https://git.openjdk.org/jdk/pull/18841

Re: RFR: 8307818: Convert Indify tool to Classfile API [v6]

[Oussama Louati]

> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
> MethodType, and CallSite constants.
> It currently uses ad-hoc code to process class files and intends to migrate 
> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
> instead.

Oussama Louati has updated the pull request incrementally with one additional 
commit since the last revision:

  update: updates indify

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18841/files
  - new: https://git.openjdk.org/jdk/pull/18841/files/f6c5bf0f..3f41a98f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18841=05
 - incr: https://webrevs.openjdk.org/?repo=jdk=18841=04-05

  Stats: 16 lines in 1 file changed: 0 ins; 4 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/18841.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18841/head:pull/18841

PR: https://git.openjdk.org/jdk/pull/18841

Withdrawn: 8323552: AbstractMemorySegmentImpl#mismatch returns -1 when comparing distinct areas of the same instance of MemorySegment

[duke]

On Wed, 10 Jan 2024 20:54:20 GMT, Peter Levart  wrote:

> I belive there is a bug in `AbstractMemorySegmentImpl#mismatch` method. It 
> returns `-1` (meaning that regions are equal) when passing the same instance 
> of MemorySegment as both `srcSegment` and `dstSegment` parameters regardless 
> of whether `srcFromOffset` and `dstFromOffset` as well as `srcToOffset` and 
> `dstToOffset` are also equal.
> 
> Am I right?

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.org/jdk/pull/17354

Re: RFR: 8307818: Convert Indify tool to Classfile API [v4]

[Chen Liang]

On Mon, 20 May 2024 15:34:28 GMT, Oussama Louati  wrote:

>> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
>> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
>> MethodType, and CallSite constants.
>> It currently uses ad-hoc code to process class files and intends to migrate 
>> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
>> instead.
>
> Oussama Louati has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update test/jdk/java/lang/invoke/indify/Indify.java
>   
>   Co-authored-by: Glavo 

test/jdk/java/lang/invoke/indify/Indify.java line 1362:

> 1360: List bsmArgs = new ArrayList<>();
> 1361: for (LoadableConstantEntry lce : 
> classModel.constantPool().bootstrapMethodEntry(i).arguments()){
> 1362: bsmArgs.add(lce);

Suggestion:

List bsmArgs = new 
ArrayList<>(classModel.constantPool().bootstrapMethodEntry(i).arguments());

Also remove the closing brace in next line.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1606944693

Re: RFR: 8307818: Convert Indify tool to Classfile API [v5]

[Oussama Louati]

> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
> MethodType, and CallSite constants.
> It currently uses ad-hoc code to process class files and intends to migrate 
> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
> instead.

Oussama Louati has updated the pull request incrementally with one additional 
commit since the last revision:

  Update test/jdk/java/lang/invoke/indify/Indify.java
  
  Co-authored-by: liach <7806504+li...@users.noreply.github.com>

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18841/files
  - new: https://git.openjdk.org/jdk/pull/18841/files/992c9773..f6c5bf0f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18841=04
 - incr: https://webrevs.openjdk.org/?repo=jdk=18841=03-04

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/18841.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18841/head:pull/18841

PR: https://git.openjdk.org/jdk/pull/18841

Re: RFR: 8307818: Convert Indify tool to Classfile API [v4]

[Chen Liang]

On Mon, 20 May 2024 15:34:28 GMT, Oussama Louati  wrote:

>> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
>> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
>> MethodType, and CallSite constants.
>> It currently uses ad-hoc code to process class files and intends to migrate 
>> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
>> instead.
>
> Oussama Louati has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update test/jdk/java/lang/invoke/indify/Indify.java
>   
>   Co-authored-by: Glavo 

test/jdk/java/lang/invoke/indify/Indify.java line 201:

> 199: } else if (ex != err) {
> 200: err.addSuppressed(ex);
> 201: }

Suggestion:

err.addSuppressed(ex);
}

test/jdk/java/lang/invoke/indify/Indify.java line 445:

> 443: final char[] poolMarks;
> 444: final Map Constants = new HashMap<>();
> 445: final Map IndySignatures = new HashMap<>();

Please use lowerCamelCase field names.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1606938989
PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1606941104

Re: RFR: 8307818: Convert Indify tool to Classfile API [v4]

[Oussama Louati]

> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
> MethodType, and CallSite constants.
> It currently uses ad-hoc code to process class files and intends to migrate 
> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
> instead.

Oussama Louati has updated the pull request incrementally with one additional 
commit since the last revision:

  Update test/jdk/java/lang/invoke/indify/Indify.java
  
  Co-authored-by: Glavo 

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18841/files
  - new: https://git.openjdk.org/jdk/pull/18841/files/040e925b..992c9773

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18841=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=18841=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/18841.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18841/head:pull/18841

PR: https://git.openjdk.org/jdk/pull/18841

Re: RFR: 8307818: Convert Indify tool to Classfile API [v3]

[Glavo]

On Mon, 20 May 2024 13:20:17 GMT, Oussama Louati  wrote:

>> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
>> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
>> MethodType, and CallSite constants.
>> It currently uses ad-hoc code to process class files and intends to migrate 
>> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
>> instead.
>
> Oussama Louati has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove: remove unnecessary ClassModel fields

test/jdk/java/lang/invoke/indify/Indify.java line 352:

> 350: }
> 351: 
> 352: public void indifyJar(File f, Object dest){

Suggestion:

public void indifyJar(File f, Object dest) {

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18841#discussion_r1606931520

Re: RFR: 8332154: Memory leak in SynchronousQueue [v5]

[Alan Bateman]

On Mon, 20 May 2024 15:23:36 GMT, Viktor Klang  wrote:

>> Local testing seems to indicate that this fix (which mirrors what's done in 
>> the FIFO mode) addresses the problem. Regression test added for JDK20+
>
> Viktor Klang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Turn the Runnables into callables to avoid having to catch any 
> InterruptedException

Marked as reviewed by alanb (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19271#pullrequestreview-2066466910

Re: RFR: 8332154: Memory leak in SynchronousQueue [v5]

[Viktor Klang]

> Local testing seems to indicate that this fix (which mirrors what's done in 
> the FIFO mode) addresses the problem. Regression test added for JDK20+

Viktor Klang has updated the pull request incrementally with one additional 
commit since the last revision:

  Turn the Runnables into callables to avoid having to catch any 
InterruptedException

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19271/files
  - new: https://git.openjdk.org/jdk/pull/19271/files/f32e9ea4..e0b2eb89

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19271=04
 - incr: https://webrevs.openjdk.org/?repo=jdk=19271=03-04

  Stats: 16 lines in 1 file changed: 0 ins; 12 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/19271.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19271/head:pull/19271

PR: https://git.openjdk.org/jdk/pull/19271

Re: RFR: 8332154: Memory leak in SynchronousQueue [v3]

[Viktor Klang]

On Mon, 20 May 2024 14:49:13 GMT, Alan Bateman  wrote:

>> Viktor Klang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Moving the memory leak test for SynchronousQueue into its own test and 
>> runs only for JDK20+, using VirtualThreads
>
> Looks okay, I just wonder how reliable assertDoesntLeak with concurrent 
> agents, debug builds, and VM options such as a Xcomp that will challenge the 
> await(10s).  Minimally the timed awaits should be untimed. Another idea is 
> try an ES like this:
> 
> try (var executor = Executors.newVirtualThreadPerTaskExecutor()) {
> for (int i = 0; i < NUMBER_OF_ITEMS; i++) {
> executor.submit( .. producer .. );
> executor.submit(.. consumer ..);
> }
> }
> 
> No need for the count down latches or the need to retry on 
> InterruptedException. The close method waits for the 400 virtual threads to 
> finish so you can assert that the queue is empty.
> 
> The loop that waits for survivors to empty can be unbounded too. If there is 
> a leak then the test will timeout.

@AlanBateman Great suggestions, Alan. I've updated the PR to reflect your 
improvements.

-

PR Comment: https://git.openjdk.org/jdk/pull/19271#issuecomment-2120665477

Re: RFR: 8332154: Memory leak in SynchronousQueue [v4]

[Viktor Klang]

> Local testing seems to indicate that this fix (which mirrors what's done in 
> the FIFO mode) addresses the problem. Regression test added for JDK20+

Viktor Klang has updated the pull request incrementally with one additional 
commit since the last revision:

  Addressing review feedback

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19271/files
  - new: https://git.openjdk.org/jdk/pull/19271/files/cc0a2014..f32e9ea4

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19271=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=19271=02-03

  Stats: 37 lines in 1 file changed: 2 ins; 10 del; 25 mod
  Patch: https://git.openjdk.org/jdk/pull/19271.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19271/head:pull/19271

PR: https://git.openjdk.org/jdk/pull/19271

Re: RFR: 8332154: Memory leak in SynchronousQueue [v3]

[Alan Bateman]

On Fri, 17 May 2024 13:19:19 GMT, Viktor Klang  wrote:

>> Local testing seems to indicate that this fix (which mirrors what's done in 
>> the FIFO mode) addresses the problem. Regression test added for JDK20+
>
> Viktor Klang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Moving the memory leak test for SynchronousQueue into its own test and runs 
> only for JDK20+, using VirtualThreads

Looks okay, I just wonder how reliable assertDoesntLeak will concurrent agents, 
debug builds, and VM options such as a Xcomp that will challenge the 
await(10s).  Minimally the timed awaits should be untimed. Another idea is try 
an ES like this:

try (var executor = Executors.newVirtualThreadPerTaskExecutor()) {
for (int i = 0; i < NUMBER_OF_ITEMS; i++) {
executor.submit( .. producer .. );
executor.submit(.. consumer ..);
}
}

No need for the count down latches or the need to retry on 
InterruptedException. The close method waits for the 400 virtual threads to 
finish so you can assert that the queue is empty.

The loop that waits for survivors to empty can be unbounded too. If there is a 
leak then the test will timeout.

-

PR Comment: https://git.openjdk.org/jdk/pull/19271#issuecomment-2120616967

Re: RFR: 8307818: Convert Indify tool to Classfile API [v3]

[Oussama Louati]

> An indify tool in j.l.i tests (also in vmTestBase) convert some source-code 
> private static methods with MT_ MH_, and INDY_ prefixes into MethodHandle, 
> MethodType, and CallSite constants.
> It currently uses ad-hoc code to process class files and intends to migrate 
> to ASM; but since we have the Classfile API, we can migrate to Classfile API 
> instead.

Oussama Louati has updated the pull request incrementally with one additional 
commit since the last revision:

  remove: remove unnecessary ClassModel fields

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18841/files
  - new: https://git.openjdk.org/jdk/pull/18841/files/96e6920a..040e925b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18841=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=18841=01-02

  Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/18841.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18841/head:pull/18841

PR: https://git.openjdk.org/jdk/pull/18841

Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]

[Sean Mullan]

On Mon, 20 May 2024 12:55:24 GMT, Alan Bateman  wrote:

>> src/java.xml/share/classes/module-info.java line 444:
>> 
>>> 442:  *
>>> 443:  * Deploying with this configuration prevents processors from 
>>> unknowingly making
>>> 444:  * outbound network connections to fetch DTDs, or process XML that 
>>> makes use of
>> 
>> s/process/processing/
>
> In XML parlance, a "processor" is an aggregation of parsers, serializers, and 
> other things that contribute to the processing. So I think it could be either 
> here, but you have a point and if it stays as "processor" then it should link 
> #FacPro where the term is defined.

It's the wording that doesn't sound right, before this you have "making" which 
doesn't sound right with "process". Maybe it needs two sentences.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606754542

Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]

[Alan Bateman]

On Mon, 20 May 2024 12:48:01 GMT, Sean Mullan  wrote:

>> Joe Wang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   withdraw changes to jaxp.properties. The configuration process has not 
>> changed, changing the default configuration would result in many failures 
>> that test the process.
>
> src/java.xml/share/classes/module-info.java line 444:
> 
>> 442:  *
>> 443:  * Deploying with this configuration prevents processors from 
>> unknowingly making
>> 444:  * outbound network connections to fetch DTDs, or process XML that 
>> makes use of
> 
> s/process/processing/

In XML parlance, a "processor" is an aggregation of parsers, serializers, and 
other things that contribute to the processing. So I think it could be either 
here, but you have a point and if it stays as "processor" then it should link 
#FacPro where the term is defined.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606745477

Re: Deterministic naming of subclasses of `java/lang/reflect/Proxy`

[Aman Sharma]

Hi David,


> I would not expect any class load
events.


I understand. I also haven't tried to intercept them but I see only one 
approach right now to include them in an allowlist - 1) statically look for 
invocations of "Lookup::defineHiddenClass". 2) Instrument them so that its 
first argument "bytes" can be looked into upon. I haven't looked into it much 
because I did not have much idea about it. And they are hidden so it made it 
worse.  Thanks for sharing the JEP!


>

java.lang.reflect.Proxy could define hidden classes to act as the proxy classes 
which implement proxy interfaces; from JEP 317


It says that Proxy classes will also become hidden classes. Is it underway? 
Right now one can intercept, transform them, and include them in an allowlist. 
What do you think of naming them independent of AtomicLong so that a proxy 
class generated at runtime is easy to lookup in the allowlist?



Regards,
Aman Sharma

PhD Student
KTH Royal Institute of Technology
School of Electrical Engineering and Computer Science (EECS)
Department of Theoretical Computer Science (TCS)

https://algomaster99.github.io/

From: David Holmes 
Sent: Monday, May 20, 2024 2:30:37 PM
To: Aman Sharma; liangchenb...@gmail.com
Cc: core-libs-dev@openjdk.org; leyden-...@openjdk.org
Subject: Re: Deterministic naming of subclasses of `java/lang/reflect/Proxy`

On 20/05/2024 10:12 pm, Aman Sharma wrote:
> Hi David,
>
>
>  > How did you try to intercept them? Hidden classes are not "loaded" in
> the normal sense so won't trigger class load events.
>
>
> I could not intercept them. I only see them when I pass `-verbose:class`
> in the Java CLI.

Yes that is why I asked how you tried to intercept them.

>
> I also couldn't intercept them using JVMTI Class File Load Hook
> 
>  event. However JEP 371 suggests that it should be possible to intercept them 
> using JVMTI Class Load 
>  
> event, but I won't have the bytecode at this stage. So is there no way to get 
> its bytecode before it is linked and initialized in the JVM?

Hidden classes are not loaded so I would not expect any class load
events. However the exact nature of the JVMTI class load event is
unclear as it talks about "class or interface creation" which is neither
loading or defining per se. But a class prepare event sounds like it
should be issued. However neither give you access to the bytecode of the
class AFAICS.

David
-


>
> Regards,
> Aman Sharma
>
> PhD Student
> KTH Royal Institute of Technology
> School of Electrical Engineering and Computer Science (EECS)
> Department of Theoretical Computer Science (TCS)
> 
> https://algomaster99.github.io/
> 
> 
> *From:* David Holmes 
> *Sent:* Monday, May 20, 2024 2:59:17 AM
> *To:* Aman Sharma; liangchenb...@gmail.com
> *Cc:* core-libs-dev@openjdk.org; leyden-...@openjdk.org
> *Subject:* Re: Deterministic naming of subclasses of
> `java/lang/reflect/Proxy`
> On 17/05/2024 9:43 pm, Aman Sharma wrote:
>> Hi Chen,
>>
>>  > java.lang.invoke.LambdaForm$MH/0x0200cc000400
>>
>> I do see this as output when I pass -verbose:class. However, based on my
>> experiments, I have seen that neither an agent passed via 'javaagent'
>> nor an agent passed via 'agentpath' is able to intercept this hidden class.
>
> How did you try to intercept them? Hidden classes are not "loaded" in
> the normal sense so won't trigger class load events.
>
>> Also, I was a bit confused since I saw somewhere that the names of
>> hidden classes are null. But thanks for clarifying here.
>
> The JEP clearly defines the name format for hidden classes - though the
> final component is VM specific (and typically a hashcode).
>
> https://openjdk.org/jeps/371 
>
> Cheers,
> David
> -
>
>>  > avoid dynamic class loading
>>
>> I don't see dynamic class loading as a problem. I only mind some
>> unstable generation aspects of them which make it hard to verify them
>> based on an allowlist.
>>
>> For example, if this hidden class is generated with the exact same name
>> and the exact same bytecode during runtime as well, it would be easy to
>> verify it. However, I do see the names are based on some sort of memory
>> address so and I don't know what bytecode it has so I don't have
>> suggestions to make them stable as of now. For Proxy classes, I feel it
>> can be addressed unless you disagree or some involved in Project Leyden
>> does. :) Thank you for forwarding my mail there.
>>
>> Regards,

Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]

[Sean Mullan]

On Sun, 19 May 2024 05:01:32 GMT, Joe Wang  wrote:

>> Add two sample configuration files:
>> 
>>   jaxp-strict.properties: used to set strict configuration, stricter than 
>> jaxp.properties in previous versions such as JDK 22
>> 
>>>   jaxp-compat.properties: used to regain compatibility from any more 
>>> restricted configuration than previous versions such as JDK 22
>> 
>> Updated on 5/16/2024
>> 
>> Design change:
>> The design is changed to include in the JDK two configuration files that are 
>> the default jaxp.properties and jaxp-strict.properties, instead of three, 
>> dropping jaxp-compat.properties.
>> 
>> Updated on 5/18/2024
>> 
>> Withdraw changes to jaxp.properties. The original idea was to match 
>> jaxp-strict.properties with regard to the properties. However, that change 
>> impact the configuration process, resulting in tests that verify the process 
>> to fail.
>
> Joe Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   withdraw changes to jaxp.properties. The configuration process has not 
> changed, changing the default configuration would result in many failures 
> that test the process.

src/java.xml/share/classes/module-info.java line 444:

> 442:  *
> 443:  * Deploying with this configuration prevents processors from 
> unknowingly making
> 444:  * outbound network connections to fetch DTDs, or process XML that makes 
> use of

s/process/processing/

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606737006

Re: Deterministic naming of subclasses of `java/lang/reflect/Proxy`

[David Holmes]

On 20/05/2024 10:12 pm, Aman Sharma wrote:

Hi David,


 > How did you try to intercept them? Hidden classes are not "loaded" in
the normal sense so won't trigger class load events.


I could not intercept them. I only see them when I pass `-verbose:class` 
in the Java CLI.


Yes that is why I asked how you tried to intercept them.



I also couldn't intercept them using JVMTI Class File Load Hook 
 event. However JEP 371 suggests that it should be possible to intercept them using JVMTI Class Load  event, but I won't have the bytecode at this stage. So is there no way to get its bytecode before it is linked and initialized in the JVM?


Hidden classes are not loaded so I would not expect any class load 
events. However the exact nature of the JVMTI class load event is 
unclear as it talks about "class or interface creation" which is neither 
loading or defining per se. But a class prepare event sounds like it 
should be issued. However neither give you access to the bytecode of the 
class AFAICS.


David
-




Regards,
Aman Sharma

PhD Student
KTH Royal Institute of Technology
School of Electrical Engineering and Computer Science (EECS)
Department of Theoretical Computer Science (TCS)

https://algomaster99.github.io/ 



*From:* David Holmes 
*Sent:* Monday, May 20, 2024 2:59:17 AM
*To:* Aman Sharma; liangchenb...@gmail.com
*Cc:* core-libs-dev@openjdk.org; leyden-...@openjdk.org
*Subject:* Re: Deterministic naming of subclasses of 
`java/lang/reflect/Proxy`

On 17/05/2024 9:43 pm, Aman Sharma wrote:

Hi Chen,

  > java.lang.invoke.LambdaForm$MH/0x0200cc000400

I do see this as output when I pass -verbose:class. However, based on my 
experiments, I have seen that neither an agent passed via 'javaagent' 
nor an agent passed via 'agentpath' is able to intercept this hidden class.


How did you try to intercept them? Hidden classes are not "loaded" in
the normal sense so won't trigger class load events.

Also, I was a bit confused since I saw somewhere that the names of 
hidden classes are null. But thanks for clarifying here.


The JEP clearly defines the name format for hidden classes - though the
final component is VM specific (and typically a hashcode).

https://openjdk.org/jeps/371 

Cheers,
David
-


  > avoid dynamic class loading

I don't see dynamic class loading as a problem. I only mind some 
unstable generation aspects of them which make it hard to verify them 
based on an allowlist.


For example, if this hidden class is generated with the exact same name 
and the exact same bytecode during runtime as well, it would be easy to 
verify it. However, I do see the names are based on some sort of memory 
address so and I don't know what bytecode it has so I don't have 
suggestions to make them stable as of now. For Proxy classes, I feel it 
can be addressed unless you disagree or some involved in Project Leyden 
does. :) Thank you for forwarding my mail there.


Regards,
Aman Sharma

PhD Student
KTH Royal Institute of Technology
https://algomaster99.github.io/  

>



*From:* liangchenb...@gmail.com 
*Sent:* Friday, May 17, 2024 1:23:58 pm
*To:* Aman Sharma 
*Cc:* core-libs-dev@openjdk.org ; 
leyden-...@openjdk.org 
*Subject:* Re: Deterministic naming of subclasses of 
`java/lang/reflect/Proxy`


Hi Aman,
For `-verbose:class`, it's a JVM argument instead of a program argument; 
so when you run a java program like `java Main`, you should call it as 
`java -verbose:class Main`.

When done correctly, you should see hidden class outputs like:
[0.032s][info][class,load] 
java.lang.invoke.LambdaForm$MH/0x0200cc000400 source: 
__JVM_LookupDefineClass__
The loading of java.lang.invoke hidden classes requires your program to 
use MethodHandle features, like a lambda.


I think the problem you are exploring, that to avoid dynamic class 
loading and effectively turn Java Platform closed for security, is also 
being accomplished by project Leyden (as I've shared initially); Thus, I 
am forwarding this to leyden-dev instead, so you can see what approach 
Leyden uses to accomplish the same goal as yours.


Regards, Chen Liang

On Fri, May 17, 2024 at 4:40 AM Aman Sharma >> wrote:


 __

 Hi Roger,


 Do you have ideas on how to intercept them? My javaagent is not able
 to nor a JVMTI agent passed using `agentpath` option. It also does
 not seem to show 

Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]

[Chen Liang]

On Mon, 20 May 2024 09:50:17 GMT, Jens Lidestrom  wrote:

>> Per Minborg has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Add benchmarks for memoized IntFunction and Function
>>  - Add benchmark for memoized supplier
>
> src/java.base/share/classes/jdk/internal/lang/StableArray.java line 66:
> 
>> 64:  * @throws IllegalArgumentException if the provided {@code length} 
>> is {@code < 0}
>> 65:  */
>> 66: static  StableArray of(int length) {
> 
> I interpret the method name `of` as a method that creates an object that 
> contains the argument as some kind of member, in the way that `List.of` and 
> friends work.
> 
> My intuitive interpretation of `StableArray.of(10)` is that it returns an 
> array with the single element 10.
> 
> I think a method like this should be named `empty`, or `emptyOfLength` or 
> something like that.

Stable arrays aren't supposed to be initialized with values, so I think your 
point is moot.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1606708916

Re: Deterministic naming of subclasses of `java/lang/reflect/Proxy`

[Aman Sharma]

Hi David,


> How did you try to intercept them? Hidden classes are not "loaded" in
the normal sense so won't trigger class load events.


I could not intercept them. I only see them when I pass `-verbose:class` in the 
Java CLI.


I also couldn't intercept them using JVMTI Class File Load 
Hook
 event. However JEP 371 suggests that it should be possible to intercept them 
using JVMTI Class 
Load 
event, but I won't have the bytecode at this stage. So is there no way to get 
its bytecode before it is linked and initialized in the JVM?


Regards,
Aman Sharma

PhD Student
KTH Royal Institute of Technology
School of Electrical Engineering and Computer Science (EECS)
Department of Theoretical Computer Science (TCS)

https://algomaster99.github.io/

From: David Holmes 
Sent: Monday, May 20, 2024 2:59:17 AM
To: Aman Sharma; liangchenb...@gmail.com
Cc: core-libs-dev@openjdk.org; leyden-...@openjdk.org
Subject: Re: Deterministic naming of subclasses of `java/lang/reflect/Proxy`

On 17/05/2024 9:43 pm, Aman Sharma wrote:
> Hi Chen,
>
>  > java.lang.invoke.LambdaForm$MH/0x0200cc000400
>
> I do see this as output when I pass -verbose:class. However, based on my
> experiments, I have seen that neither an agent passed via 'javaagent'
> nor an agent passed via 'agentpath' is able to intercept this hidden class.

How did you try to intercept them? Hidden classes are not "loaded" in
the normal sense so won't trigger class load events.

> Also, I was a bit confused since I saw somewhere that the names of
> hidden classes are null. But thanks for clarifying here.

The JEP clearly defines the name format for hidden classes - though the
final component is VM specific (and typically a hashcode).

https://openjdk.org/jeps/371

Cheers,
David
-

>  > avoid dynamic class loading
>
> I don't see dynamic class loading as a problem. I only mind some
> unstable generation aspects of them which make it hard to verify them
> based on an allowlist.
>
> For example, if this hidden class is generated with the exact same name
> and the exact same bytecode during runtime as well, it would be easy to
> verify it. However, I do see the names are based on some sort of memory
> address so and I don't know what bytecode it has so I don't have
> suggestions to make them stable as of now. For Proxy classes, I feel it
> can be addressed unless you disagree or some involved in Project Leyden
> does. :) Thank you for forwarding my mail there.
>
> Regards,
> Aman Sharma
>
> PhD Student
> KTH Royal Institute of Technology
> https://algomaster99.github.io/ 
>
> 
> *From:* liangchenb...@gmail.com 
> *Sent:* Friday, May 17, 2024 1:23:58 pm
> *To:* Aman Sharma 
> *Cc:* core-libs-dev@openjdk.org ;
> leyden-...@openjdk.org 
> *Subject:* Re: Deterministic naming of subclasses of
> `java/lang/reflect/Proxy`
>
> Hi Aman,
> For `-verbose:class`, it's a JVM argument instead of a program argument;
> so when you run a java program like `java Main`, you should call it as
> `java -verbose:class Main`.
> When done correctly, you should see hidden class outputs like:
> [0.032s][info][class,load]
> java.lang.invoke.LambdaForm$MH/0x0200cc000400 source:
> __JVM_LookupDefineClass__
> The loading of java.lang.invoke hidden classes requires your program to
> use MethodHandle features, like a lambda.
>
> I think the problem you are exploring, that to avoid dynamic class
> loading and effectively turn Java Platform closed for security, is also
> being accomplished by project Leyden (as I've shared initially); Thus, I
> am forwarding this to leyden-dev instead, so you can see what approach
> Leyden uses to accomplish the same goal as yours.
>
> Regards, Chen Liang
>
> On Fri, May 17, 2024 at 4:40 AM Aman Sharma  > wrote:
>
> __
>
> Hi Roger,
>
>
> Do you have ideas on how to intercept them? My javaagent is not able
> to nor a JVMTI agent passed using `agentpath` option. It also does
> not seem to show up in logs when I pass `-verbose:class`.
>
>
> Also, what do you think of renaming the proxy classes as suggested
> below?
>
>
> Regards,
> Aman Sharma
>
> PhD Student
> KTH Royal Institute of Technology
> School of Electrical Engineering and Computer Science (EECS)
> Department of Theoretical Computer Science (TCS)
> 
> 
> https://algomaster99.github.io/
> 
> 

Re: RFR: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr [v2]

[Axel Boldt-Christmas]

On Mon, 20 May 2024 10:11:26 GMT, David Holmes  wrote:

>> Axel Boldt-Christmas has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Update copyright year
>
> test/lib/jdk/test/lib/process/OutputAnalyzer.java line 691:
> 
>> 689:  * @throws RuntimeException If the pattern was not found
>> 690:  */
>> 691: public OutputAnalyzer 
>> stderrShouldMatchIgnoreDeprecatedWarnings(String pattern) {
> 
> Given we have `...IgnoreVMWarnings` this special case should really be called 
> `...IgnoreDeprecatedVMWarnings`.

The name was chosen based on: 
https://github.com/openjdk/jdk/blob/77c8516085225a04bd5a954197fc5ef7e5c5ee61/test/lib/jdk/test/lib/process/OutputAnalyzer.java#L184

Should I still change it?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19297#discussion_r1606671332

Re: RFR: 8327964: Simplify BigInteger.implMultiplyToLen intrinsic [v3]

[Bhavana Kilambi]

On Wed, 17 Apr 2024 12:37:01 GMT, Yudi Zheng  wrote:

>> `multiply_to_len` seems to be used by `generate_squareToLen` as well for 
>> aarch64 and riscv but `zlen` is still passed in a register.
>> 
>> https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp#L4710
>> https://github.com/openjdk/jdk/blob/870a6127cf54264c691f7322d775b202705c3bfa/src/hotspot/cpu/riscv/stubGenerator_riscv.cpp#L2881
>> 
>> I think it might work anyway but it might be better to adapt them if only 
>> for completeness.
>
> @dafedafe @dean-long please take a look and let me know if there are further 
> issues, thanks!

Hi @mur47x111, do you happen to have any performance results with this patch?

-

PR Comment: https://git.openjdk.org/jdk/pull/18226#issuecomment-2120179701

Re: RFR: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr [v2]

[David Holmes]

On Mon, 20 May 2024 07:27:19 GMT, Axel Boldt-Christmas  
wrote:

>> Improve `java/util/zip/EntryCount64k.java` stderr parsing by ignoring 
>> deprecated warnings. Testing non-generational ZGC requires the use of a 
>> deprecated option.
>
> Axel Boldt-Christmas has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright year

I'm undecided whether it is really worth adding a new utility function to just 
ignore deprecated warnings, or whether we should just use the existing function 
to ignore all warnings.

If we special case for deprecated warnings then we may want to also add a 
special version of `asLinesWithoutVMWarnings`

test/lib/jdk/test/lib/process/OutputAnalyzer.java line 691:

> 689:  * @throws RuntimeException If the pattern was not found
> 690:  */
> 691: public OutputAnalyzer 
> stderrShouldMatchIgnoreDeprecatedWarnings(String pattern) {

Given we have `...IgnoreVMWarnings` this special case should really be called 
`...IgnoreDeprecatedVMWarnings`.

-

PR Review: https://git.openjdk.org/jdk/pull/19297#pullrequestreview-2065863128
PR Review Comment: https://git.openjdk.org/jdk/pull/19297#discussion_r1606551444

Re: RFR: 8330465: Stable Values and Collections (Internal) [v20]

[Jens Lidestrom]

On Fri, 17 May 2024 09:31:33 GMT, Per Minborg  wrote:

>> # Stable Values & Collections (Internal)
>> 
>> > src="https://github.com/openjdk/jdk/assets/7457876/db4b22a1-af87-4914-adac-b05a87e7cb42;
>>  width=20% height=20%>
>> 
>> ## Summary
>> This PR proposes to introduce an internal _Stable Values & Collections_ API, 
>> which provides immutable value holders where elements are initialized _at 
>> most once_. Stable Values & Collections offer the performance and safety 
>> benefits of final fields while offering greater flexibility as to the timing 
>> of initialization.
>> 
>> ## Goals
>>  * Provide an easy and intuitive API to describe value holders that can 
>> change at most once.
>>  * Decouple declaration from initialization without significant footprint or 
>> performance penalties.
>>  * Reduce the amount of static initializer and/or field initialization code.
>>  * Uphold integrity and consistency, even in a multi-threaded environment.
>>  
>> For more details, see the draft JEP: https://openjdk.org/jeps/8312611
>> 
>> ## Performance 
>> Performance compared to instance variables using two `AtomicReference` and 
>> two protected by double-checked locking under concurrent access by all 
>> threads:
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableBenchmark.atomicthrpt   10259.478 ?   
>> 36.809  ops/us
>> StableBenchmark.dcl   thrpt   10225.710 ?   
>> 26.638  ops/us
>> StableBenchmark.stablethrpt   10   4382.478 ? 
>> 1151.472  ops/us <- StableValue significantly faster
>> 
>> 
>> Performance compared to static variables protected by `AtomicReference`, 
>> class-holder idiom holder, and double-checked locking (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableStaticBenchmark.atomic  thrpt   10   6487.835 ?  
>> 385.639  ops/us
>> StableStaticBenchmark.dcl thrpt   10   6605.239 ?  
>> 210.610  ops/us
>> StableStaticBenchmark.stable  thrpt   10  14338.239 ? 
>> 1426.874  ops/us
>> StableStaticBenchmark.staticCHI   thrpt   10  13780.341 ? 
>> 1839.651  ops/us
>> 
>> 
>> Performance for stable lists (thread safe) in both instance and static 
>> contexts whereby we access a single value compared to `ArrayList` instances 
>> (which are not thread-safe) (all threads):
>> 
>> 
>> Benchmark  Mode  Cnt  Score  
>> Error   Units
>> StableListElementBenchmark.instanceArrayList  thrpt   10   5812.992 ? 
>> 1169.730  ops...
>
> Per Minborg has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add benchmarks for memoized IntFunction and Function
>  - Add benchmark for memoized supplier

src/java.base/share/classes/jdk/internal/lang/StableArray.java line 66:

> 64:  * @throws IllegalArgumentException if the provided {@code length} is 
> {@code < 0}
> 65:  */
> 66: static  StableArray of(int length) {

I interpret the method name `of` as a method that creates an object that 
contains the argument as some kind of member, in the way that `List.of` and 
friends work.

My intuitive interpretation of `StableArray.of(10)` is that it returns an array 
with the single element 10.

I think a method like this should be named `empty`, or `emptyOfLength` or 
something like that.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18794#discussion_r1606529054

Re: RFR: 8331865: Consolidate size and alignment checks in LayoutPath [v2]

[Maurizio Cimadamore]

On Fri, 17 May 2024 23:42:17 GMT, Paul Sandoz  wrote:

> Separately, we might be missing a few long argument accepting guard methods 
> for simpler cases as I suspect they are still focused more on int index types.

Not sure I understand what guard methods you are referring to here?

-

PR Comment: https://git.openjdk.org/jdk/pull/19251#issuecomment-2120083825

Integrated: 8331851: Add specific regression leap year tests for Calendar.roll()

[serhiysachkov]

On Wed, 15 May 2024 09:39:16 GMT, serhiysachkov  wrote:

> Add specific regression tests for Calendar.roll() method to explicitly 
> various leap year test scenarios. This is inspired by the ambiguity which 
> occurred in leap year unaware test creation as in case with Calendar.add() in 
> swing component test case as detailed in 
> (https://bugs.openjdk.org/browse/JDK-8327088).

This pull request has now been integrated.

Changeset: d6b7f9b1
Author:Serhiy Sachkov 
Committer: Mahendra Chhipa 
URL:   
https://git.openjdk.org/jdk/commit/d6b7f9b170b6ce4f7275cc7595b71b9a3e93c133
Stats: 190 lines in 1 file changed: 190 ins; 0 del; 0 mod

8331851: Add specific regression leap year tests for Calendar.roll()

Reviewed-by: naoto

-

PR: https://git.openjdk.org/jdk/pull/19247

RFR: 8332486: ClassFile API ArrayIndexOutOfBoundsException with label metadata

[Adam Sotona]

Parsing of a specifically corrupted class file cause unexpected 
`ArrayIndexOutOfBoundsException` during label inflation.
This patch checks the valid range and throws expected 
`IllegalArgumentException` instead.
Relevant test is added.

Please review.

Thanks,
Adam

-

Commit messages:
 - 8332486: ClassFile API ArrayIndexOutOfBoundsException with label metadata

Changes: https://git.openjdk.org/jdk/pull/19304/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=19304=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332486
  Stats: 19 lines in 2 files changed: 18 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/19304.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19304/head:pull/19304

PR: https://git.openjdk.org/jdk/pull/19304

Re: RFR: 8331196: vector api: Remove unnecessary index check in Byte/ShortVector.fromArray/fromArray0Template

[Ludovic Henry]

On Fri, 26 Apr 2024 14:06:02 GMT, Hamlin Li  wrote:

> Hi,
> Can you help to review this simple patch?
> Some index check in Byte/ShortVector.fromArray/fromArray0Template seems not 
> necessary, could be removed.
> Thanks

@PaulSandoz who would be a good person to give a review on this one? It's not 
architecture specific. Thanks!

-

PR Comment: https://git.openjdk.org/jdk/pull/18977#issuecomment-2119954616

RFR: 8332505: JEP 457: ClassRemapper forgets to remap bootstrap method references

[Adam Sotona]

Class-File API `ClassRemapper` component suppose to remap all classes 
referenced in a class file.
Actual implementation missed remapping of bootstrap methods referenced from 
`invokedynamic` instructions.
This patch fixes the remapping and adds relevant test.

Please review.

Thanks,
Adam

-

Commit messages:
 - 8332505: JEP 457: ClassRemapper forgets to remap bootstrap method references

Changes: https://git.openjdk.org/jdk/pull/19299/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=19299=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332505
  Stats: 9 lines in 2 files changed: 4 ins; 0 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/19299.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19299/head:pull/19299

PR: https://git.openjdk.org/jdk/pull/19299

Re: RFR: 8332495: java/util/logging/LoggingDeadlock2.java fails with AssertionError: Some tests failed [v2]

[Jaikiran Pai]

On Mon, 20 May 2024 07:25:12 GMT, Axel Boldt-Christmas  
wrote:

>> Improve ` java/util/logging/LoggingDeadlock2.java` stderr parsing by 
>> ignoring deprecated warnings. Testing non-generational ZGC requires the use 
>> of a deprecated option.
>
> Axel Boldt-Christmas has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright year

Marked as reviewed by jpai (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19298#pullrequestreview-2065553921

Re: RFR: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr [v2]

[Jaikiran Pai]

On Mon, 20 May 2024 07:27:19 GMT, Axel Boldt-Christmas  
wrote:

>> Improve `java/util/zip/EntryCount64k.java` stderr parsing by ignoring 
>> deprecated warnings. Testing non-generational ZGC requires the use of a 
>> deprecated option.
>
> Axel Boldt-Christmas has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright year

Marked as reviewed by jpai (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19297#pullrequestreview-2065548780

Re: RFR: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr [v2]

[Axel Boldt-Christmas]

> Improve `java/util/zip/EntryCount64k.java` stderr parsing by ignoring 
> deprecated warnings. Testing non-generational ZGC requires the use of a 
> deprecated option.

Axel Boldt-Christmas has updated the pull request incrementally with one 
additional commit since the last revision:

  Update copyright year

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19297/files
  - new: https://git.openjdk.org/jdk/pull/19297/files/d25cde7c..77c85160

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19297=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=19297=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/19297.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19297/head:pull/19297

PR: https://git.openjdk.org/jdk/pull/19297

Re: RFR: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr [v2]

[Stefan Karlsson]

On Mon, 20 May 2024 07:24:16 GMT, Axel Boldt-Christmas  
wrote:

>> Improve `java/util/zip/EntryCount64k.java` stderr parsing by ignoring 
>> deprecated warnings. Testing non-generational ZGC requires the use of a 
>> deprecated option.
>
> Axel Boldt-Christmas has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright year

Marked as reviewed by stefank (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19297#pullrequestreview-2065538663

Re: RFR: 8332495: java/util/logging/LoggingDeadlock2.java fails with AssertionError: Some tests failed [v2]

[Axel Boldt-Christmas]

> Improve ` java/util/logging/LoggingDeadlock2.java` stderr parsing by ignoring 
> deprecated warnings. Testing non-generational ZGC requires the use of a 
> deprecated option.

Axel Boldt-Christmas has updated the pull request incrementally with one 
additional commit since the last revision:

  Update copyright year

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/19298/files
  - new: https://git.openjdk.org/jdk/pull/19298/files/003028de..5205a53c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=19298=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=19298=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/19298.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19298/head:pull/19298

PR: https://git.openjdk.org/jdk/pull/19298

Re: RFR: 8332495: java/util/logging/LoggingDeadlock2.java fails with AssertionError: Some tests failed [v2]

[Stefan Karlsson]

On Mon, 20 May 2024 07:22:49 GMT, Axel Boldt-Christmas  
wrote:

>> Improve ` java/util/logging/LoggingDeadlock2.java` stderr parsing by 
>> ignoring deprecated warnings. Testing non-generational ZGC requires the use 
>> of a deprecated option.
>
> Axel Boldt-Christmas has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright year

Marked as reviewed by stefank (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/19298#pullrequestreview-2065533348

Re: RFR: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr

[Jaikiran Pai]

On Mon, 20 May 2024 06:41:45 GMT, Axel Boldt-Christmas  
wrote:

> Improve `java/util/zip/EntryCount64k.java` stderr parsing by ignoring 
> deprecated warnings. Testing non-generational ZGC requires the use of a 
> deprecated option.

Hello Axel, the change looks OK to me. I was going to suggest 
`stderrShouldMatch(pattern, boolean ignoreDeprecatedWarning)` as the new method 
instead of `stderrShouldMatchIgnoreDeprecatedWarnings`, but then I see that 
there are also methods with names like this in `OutputAnalyzer`. So this change 
is consistent with those.

`OutputAnalyzer.java` will need a copyright year update before integrating.

-

Marked as reviewed by jpai (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19297#pullrequestreview-2065518056

Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]

[Alan Bateman]

On Sun, 19 May 2024 05:01:32 GMT, Joe Wang  wrote:

>> Add two sample configuration files:
>> 
>>   jaxp-strict.properties: used to set strict configuration, stricter than 
>> jaxp.properties in previous versions such as JDK 22
>> 
>>>   jaxp-compat.properties: used to regain compatibility from any more 
>>> restricted configuration than previous versions such as JDK 22
>> 
>> Updated on 5/16/2024
>> 
>> Design change:
>> The design is changed to include in the JDK two configuration files that are 
>> the default jaxp.properties and jaxp-strict.properties, instead of three, 
>> dropping jaxp-compat.properties.
>> 
>> Updated on 5/18/2024
>> 
>> Withdraw changes to jaxp.properties. The original idea was to match 
>> jaxp-strict.properties with regard to the properties. However, that change 
>> impact the configuration process, resulting in tests that verify the process 
>> to fail.
>
> Joe Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   withdraw changes to jaxp.properties. The configuration process has not 
> changed, changing the default configuration would result in many failures 
> that test the process.

Looks good, just one comment on on the jaxp-strict.properties file text.

src/java.xml/share/conf/jaxp-strict.properties line 17:

> 15: # java -Djava.xml.config.file=$JAVA_HOME/conf/jaxp-strict.properties
> 16: #
> 17: # The pathname to the configuration file must be valid. If it is not 
> absolute,

I think it would be better to drop this paragraph or else just replace it with 
a sentence to say that the java.xml module description specifies the system 
property.

-

Marked as reviewed by alanb (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18831#pullrequestreview-2065520089
PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606350445

Re: RFR: 8332495: java/util/logging/LoggingDeadlock2.java fails with AssertionError: Some tests failed

[Jaikiran Pai]

On Mon, 20 May 2024 06:42:19 GMT, Axel Boldt-Christmas  
wrote:

> Improve ` java/util/logging/LoggingDeadlock2.java` stderr parsing by ignoring 
> deprecated warnings. Testing non-generational ZGC requires the use of a 
> deprecated option.

The change looks OK to me. The copyright year on the file will need an update.

-

Marked as reviewed by jpai (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19298#pullrequestreview-2065506460

RFR: 8332495: java/util/logging/LoggingDeadlock2.java fails with AssertionError: Some tests failed

[Axel Boldt-Christmas]

Improve ` java/util/logging/LoggingDeadlock2.java` stderr parsing by ignoring 
deprecated warnings. Testing non-generational ZGC requires the use of a 
deprecated option.

-

Commit messages:
 - 8332495: java/util/logging/LoggingDeadlock2.java fails with AssertionError: 
Some tests failed

Changes: https://git.openjdk.org/jdk/pull/19298/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=19298=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332495
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/19298.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19298/head:pull/19298

PR: https://git.openjdk.org/jdk/pull/19298

RFR: 8332494: java/util/zip/EntryCount64k.java failing with java.lang.RuntimeException: '\\A\\Z' missing from stderr

[Axel Boldt-Christmas]

Improve `java/util/zip/EntryCount64k.java` stderr parsing by ignoring 
deprecated warnings. Testing non-generational ZGC requires the use of a 
deprecated option.

-

Commit messages:
 - 8332494: java/util/zip/EntryCount64k.java failing with 
java.lang.RuntimeException: '\\A\\Z' missing from stderr

Changes: https://git.openjdk.org/jdk/pull/19297/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=19297=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332494
  Stats: 20 lines in 2 files changed: 18 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/19297.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19297/head:pull/19297

PR: https://git.openjdk.org/jdk/pull/19297