Re: RFR: 8353185: Introduce the concept of upgradeable files in context of JEP 493

[Severin Gehwolf]

On Thu, 3 Apr 2025 06:31:20 GMT, Alan Bateman  wrote:

> What about changes to conf files, especially Java.security (for hardening TLS 
> settings) - or at least pointing to a include?

You can still do that after creating the custom runtime as you do today when 
linking using JMODs. The point of this fix is to be able to have a way to still 
run `jlink` based on the run-time image when you're using cacerts file from an 
external location (outside the JDKs control) or have updated tzdata (also 
outside the JDK build). I wouldn't expect for this to be needed in many (most?) 
cases.

-

PR Comment: https://git.openjdk.org/jdk/pull/24388#issuecomment-2774916530

Re: RFR: 8352935: Launcher should not add $JDK/../lib to LD_LIBRARY_PATH [v2]

[Joachim Kern]

> In the JDK launcher, there is a codepath which would set/modify the 
> LD_LIBRARY_PATH. This happens unconditionally on AIX and Linux/musl and can 
> also happen on other Linux platforms if an LD_LIBRARY_PATH is pre-set which 
> contains a libjvm.so.
> 
> The LD_LIBRARY_PATH is set to $JVMPATH:$JDK/lib:$JDK/../lib. The last part of 
> this, $JDK/../lib, seems to be a remnant of times when there was a jre 
> subfolder in a JDK deployment. So it should likely be removed.

Joachim Kern has updated the pull request incrementally with one additional 
commit since the last revision:

  copyright years

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/24351/files
  - new: https://git.openjdk.org/jdk/pull/24351/files/9d41d444..6f32ead9

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=24351&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24351&range=00-01

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

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

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v27]

[Maurizio Cimadamore]

On Thu, 3 Apr 2025 11:23:36 GMT, Per Minborg  wrote:

>> Yes - but they cache different things -- the first caches results for values 
>> 0-9 -- the second cached results for 1, 2, 4, 8, 16, 32.
>
> That was intentional as the list is always `0..size` whereas a map can use 
> arbitrary inputs. Is there a better way to illustrate this than what we have 
> now?

I guess my "beef" is that the examples for int function and function are 
similar -- they use same class name. Reader might expect (as I did) that they 
do the same thing, just using a different backing storage. But they don't -- 
not only is the implementation tweaked to use function instead of int function 
-- also what is cached changes subtly. I'm afraid this will be difficult to 
follow for readers. If you want to highlight that the "function" example has 
more freedom, I think we'd be better off with using another example. For 
instance:

* use square(int)->int to show off int function/list
* use square_root(int)->int to show off function/map

-

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2026872636

Re: RFR: 8352565: Add native method implementation of Reference.get() [v2]

[Kim Barrett]

On Tue, 1 Apr 2025 22:01:55 GMT, Brent Christian  wrote:

>> Kim Barrett has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   parameterized return type of native get0
>
> test/hotspot/jtreg/gc/TestNativeReferenceGet.java line 162:
> 
>> 160: System.out.println("Testing nonconcurrent GC");
>> 161: clearReferents();
>> 162: strengthenReferents();
> 
> Might the GC clear refs between `clearReferents()` and 
> `strengthenReferents()`?

Yeah, an ill-timed GC between those operations would result in test failure.
I've added a GC immediately before those operations to make that pretty 
unlikely.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24315#discussion_r2025385681

Re: RFR: 8353118: Deprecate the use of `java.locale.useOldISOCodes` system property

[Justin Lu]

On Fri, 28 Mar 2025 20:17:30 GMT, Naoto Sato  wrote:

> Proposing to remove the `java.locale.useOldISOCodes` system property. This 
> property is for backward compatibility introduced back in JDK17 and I believe 
> it is now fine to remove it. In this PR targeting JDK25, it emits a 
> deprecate-for-removal warning on startup if the system property is set to 
> true (no behavioral change except the warning). The plan is eventually to 
> remove it after JDK25. A corresponding CSR has been drafted.

Marked as reviewed by jlu (Committer).

-

PR Review: https://git.openjdk.org/jdk/pull/24302#pullrequestreview-2729873125

RFR: 8353267: jmod create finds the wrong set of packages when class file are in non-package location

[Alan Bateman]

`jmod create` maps the contents of the module to a set of packages. This 
mapping derives illegal package names when class resources are located in 
non-package locations, e.g. in the META-INF tree. `jlink` also has an issue in 
this area and doesn't correctly determine if a resource is in a named package 
when the resource is class file in a non-package location.

-

Commit messages:
 - Remove trailing space
 - Improve comment
 - Initial commit

Changes: https://git.openjdk.org/jdk/pull/24362/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24362&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8353267
  Stats: 214 lines in 4 files changed: 180 ins; 6 del; 28 mod
  Patch: https://git.openjdk.org/jdk/pull/24362.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24362/head:pull/24362

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

Re: RFR: 8352709: Remove bad timing annotations from WhileOpTest.java

[Jaikiran Pai]

On Mon, 24 Mar 2025 12:17:22 GMT, Leo Korinth  wrote:

> WhileOpTest.java is a TestNG test (`TestNG.dirs` is set in Find file: 
> `test/jdk/java/util/stream/test/TEST.properties`)
> 
> As such, the `@run main/timeout=240` annotation is ignored. If that was not 
> the case, it would complain about not specifying a main class.

The change looks good to me.

test/jdk/java/util/stream/test/org/openjdk/tests/java/util/stream/WhileOpTest.java
 line 50:

> 48:  * @test
> 49:  * @bug 8071597 8193856
> 50:  * @run main/timeout=240

The original change which introduced this `main/timeout` appears to be 
https://bugs.openjdk.org/browse/JDK-8134459. Going through the RFR of that 
change 
https://mail.openjdk.org/pipermail/core-libs-dev/2018-January/050878.html it 
appears that in addition to this timeout being introduced there was an 
additional change done to help improve the duration of this test. Plus there's 
also a comment in that JBS issue that the timeout hadn't been observed for a 
while (except locally).

I think all that explains why this test hasn't been timing out in the presence 
of this no-op directive.

-

Marked as reviewed by jpai (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24193#pullrequestreview-2731184354
PR Review Comment: https://git.openjdk.org/jdk/pull/24193#discussion_r2022189487

Re: RFR: 8349206: j.u.l.Handler classes create deadlock risk via synchronized publish() method [v13]

[David Beaumont]

> 8349206: j.u.l.Handler classes create deadlock risk via synchronized 
> publish() method.
> 
> 1. Remove synchronization of calls to publish() in Handlers in 
> java.util.logging package.
> 2. Add explanatory comments to various affected methods.
> 3. Add a test to ensure deadlocks no longer occur.
> 
> Note that this change does not address issue in MemoryHandler (see 
> JDK-8349208).

David Beaumont has updated the pull request incrementally with one additional 
commit since the last revision:

  Tweak wording.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/23491/files
  - new: https://git.openjdk.org/jdk/pull/23491/files/15cad6ed..8db4c907

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=23491&range=12
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23491&range=11-12

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

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

Integrated: 8353684: [BACKOUT] j.u.l.Handler classes create deadlock risk via synchronized publish() method

[Stuart Marks]

On Thu, 3 Apr 2025 21:03:08 GMT, Stuart Marks  wrote:

> Back out commit for 
> [JDK-8349206](https://bugs.openjdk.org/browse/JDK-8349206) because of build 
> failure.
> 
> This reverts commit ebcb9a8b128cc6411610566c8368db63d25a5127.

This pull request has now been integrated.

Changeset: 57df89c4
Author:Stuart Marks 
URL:   
https://git.openjdk.org/jdk/commit/57df89c46449a19bb626fee2ea01c868e6dfb712
Stats: 453 lines in 8 files changed: 8 ins; 430 del; 15 mod

8353684: [BACKOUT] j.u.l.Handler classes create deadlock risk via synchronized 
publish() method

Reviewed-by: dholmes

-

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

Re: RFR: 8304674: AttachCurrentThread 's argument is JavaVMAttachArgs

[Jaikiran Pai]

On Mon, 31 Mar 2025 01:28:47 GMT, tison  wrote:

>> Try to add clear type annotations.
>> 
>> Please let me know how this patch can be properly reviewed >_<
>
> retest?

Hello @tisonkun, is there an issue created for this? 
https://bugs.openjdk.org/browse/JDK-8304674 against which this PR is linked is 
for something else. The PR description too isn't clear what this change is 
about.

-

PR Comment: https://git.openjdk.org/jdk/pull/21658#issuecomment-2764925445

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v40]

[Maurizio Cimadamore]

On Thu, 3 Apr 2025 14:00:00 GMT, Per Minborg  wrote:

>> Implement JEP 502.
>> 
>> The PR passes tier1-tier3 tests.
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Make the sqrt example different

src/java.base/share/classes/java/lang/StableValue.java line 410:

> 408:  * semantics associated with {@code final} fields is too restrictive.
> 409:  * 
> 410:  * In JDK 24, {@code final} instance fields in records and hidden 
> classes (such as classes

I suggest to drop this -- we normally don't reference to JDK versions in the 
javadoc.

Perhaps this para should say: 


The _content_ of a set stable value is treated as a constant by the JVM, 
provided that the reference to the stable value is also constant (e.g. in cases 
where the stable value itself is stored in a `static final` field). 

This means that, at least in some cases, access to the content of a stable 
value enjoys the same constant-folding optimizations that are available when 
accessing `static final` fields.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2027192344

Re: RFR: 8344708: Compiler Implementation of Module Import Declarations [v4]

[Jan Lahoda]

> This is a patch to finalize the module imports feature. Please see:
> https://bugs.openjdk.org/browse/JDK-8344700

Jan Lahoda has updated the pull request incrementally with one additional 
commit since the last revision:

  Reflecting review feedback - avoiding hardcoded constants.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/23801/files
  - new: https://git.openjdk.org/jdk/pull/23801/files/067a0444..d66703c1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=23801&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23801&range=02-03

  Stats: 19 lines in 3 files changed: 4 ins; 0 del; 15 mod
  Patch: https://git.openjdk.org/jdk/pull/23801.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/23801/head:pull/23801

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

Re: RFR: 8353185: Introduce the concept of upgradeable files in context of JEP 493

[Bernd]

On Wed, 2 Apr 2025 18:39:57 GMT, Severin Gehwolf  wrote:

> For JEP 493-enabled builds there are no JMODs. Certain files come from the 
> installed JDK image when a user creates a custom run-time from it. This is 
> problematic for example for files that often come from a different package 
> (e.g. `cacerts` file for Linux distro builds of OpenJDK packaged as RPMs), or 
> more generally when they get updated out-of-band of the JDK build itself like 
> the tzupdater tool.
> 
> When that happens the hash sum recorded at JDK build time of those files no 
> longer match, causing `jlink` to fail. I propose to allow for those files to 
> get "upgraded" should this happen. The way this works is as follows:
> 
> 1. The list of upgradeable files is hard-coded to `lib/tzdb.dat` and 
> `lib/security/cacerts`. Only those two files from the `java.base` module will 
> be allowed to be upgraded with a link from the current run-time image.
> 2. The upgrade needs to be opt-in. Similar to `--ignore-signing-information` 
> for signed modular JARs. A user needs to add 
> `--upgrade-files=/` for it to succeed.
> 
> `--upgrade-files` is a hidden `jlink` option since it only does anything for 
> JEP 493 enabled builds. Users get a hint to apply the option if they so 
> desire:
> 
> Example:
> 
> 
> $ ./bin/jlink --add-modules java.base --output java.base-jdk
> Error: [..]/lib/security/cacerts has been modified
> Hint: lib/security/cacerts is an upgradeable file. Add 
> --upgrade-files=java.base/lib/security/cacerts to allow the upgrade.
> 
> 
> using the hint we get:
> 
> 
> $ ./bin/jlink --add-modules java.base --output java.base-jdk 
> --upgrade-files=java.base/lib/security/cacerts
> $ ./java.base-jdk/bin/java --list-modules
> java.base@25-internal
> $ sha512sum ./java.base-jdk/lib/security/cacerts 
> cf2b4c17161e79001c8e07def3de36c0d523f00a2a6b6e33893a2a3669d930957c11ac765dd29d5ff80e63ad100ef0258291891377f7133b997111ba97b15146
>   ./java.base-jdk/lib/security/cacerts
> $ sha512sum ./lib/security/cacerts 
> cf2b4c17161e79001c8e07def3de36c0d523f00a2a6b6e33893a2a3669d930957c11ac765dd29d5ff80e63ad100ef0258291891377f7133b997111ba97b15146
>   ./lib/security/cacerts
> 
> 
> **Testing**
> 
> - [ ] GHA
> - [x] `jdk/tools/jlink` jtreg tests
> - [x] Some manual tests with updated `tzdb.dat` and `cacerts` files.
> 
> Thoughts?

Well, shipping customized runtimes sounds like a valid usecase, but yes maybe 
it’s more important for jpackage.

-

PR Comment: https://git.openjdk.org/jdk/pull/24388#issuecomment-2774651205

Re: RFR: 8352064: AIX: now also able to build static-jdk image with a statically linked launcher [v3]

[Joachim Kern]

On Fri, 21 Mar 2025 10:33:10 GMT, Magnus Ihse Bursie  wrote:

>> Joachim Kern has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   added needed include
>
> make/StaticLibs.gmk line 117:
> 
>> 115:   OUTPUT_FILE := $(lib).exp, \
>> 116:   COMMAND := ( $(AR) $(ARFLAGS) -w $(lib) | $(GREP) -v '^\.' | 
>> $(AWK) '{print $$1}' | $(SORT) -u > $(lib).exp ), \
>> 117: )) \
> 
> To better align with how we store the targets of Setup calls, can you please 
> change this to:
> 
> Suggestion:
> 
>   STATIC_LIBS := -Wl,-bexpfull $(STATIC_LIB_FILES) $(addprefix 
> -Wl$(COMMA)-bE:, $(STATIC_LIB_EXPORT_FILES))
>   $(foreach lib, $(STATIC_LIB_FILES), \
> $(eval $(call SetupExecute, generate_export_list_$(notdir $(lib)), \
>   INFO := Generating export list for $(notdir $(lib)), \
>   DEPS :=  $(lib), \
>   OUTPUT_FILE := $(lib).exp, \
>   COMMAND := ( $(AR) $(ARFLAGS) -w $(lib) | $(GREP) -v '^.' | $(AWK) 
> '{print $$1}' | $(SORT) -u > $(lib).exp ), \
> )) \
> $(eval   STATIC_LIB_EXPORT_FILES += $(lib).exp) \

Done

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24062#discussion_r2007406147

Re: RFR: 8352971: Increase maximum number of hold counts for ReentrantReadWriteLock

[Alan Bateman]

On Thu, 27 Mar 2025 11:30:11 GMT, Alan Bateman  wrote:

>> I'm breaking this change out as a separate improvement, since it will not be 
>> generally possible to adjust these limits on the j.u.c primitives since they 
>> might already use a backing `long` to pack in information which needs to be 
>> updated atomically (would require 128-bit atomics to widen them, and it 
>> still infeasible to change return types of pre-existing APIs).
>
> We'll need to double check serialization compatibility as the "sync" field is 
> in the serial form.

> @AlanBateman I was able to serialize a RRWL created before this patch and 
> deserialize it after this patch.

Okay, so we serialize a FairSync or NonfairSync when they are a AQS. When we 
deserialize, they will be a AQLS. It works because the internal classes haven't 
been renamed. If either internal class were to be renamed then we've get a 
CNFE. So I think what you have is okay and there's no compatibility or interop 
issue created at this time. However, having "sync" in the serial form is 
technical debt that will surface at some point. Ideally's RRWL's serial form 
would have something else to persistent the "fairness" bit as that is also it 
needs.

-

PR Comment: https://git.openjdk.org/jdk/pull/24261#issuecomment-2771959548

Re: RFR: 8352016: Improve java/lang/RuntimeTests/RuntimeExitLogTest.java [v2]

[KIRIYAMA Takuya]

On Wed, 19 Mar 2025 20:09:51 GMT, Roger Riggs  wrote:

>> KIRIYAMA Takuya has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8352016: Improve java/lang/RuntimeTests/RuntimeExitLogTest.java
>
> test/jdk/java/lang/RuntimeTests/ExitLogging-FINER.properties line 8:
> 
>> 6: 
>> 7: java.util.logging.ConsoleHandler.level = ALL
>> 8: java.lang.Runtime.level = FINER
> 
> It is a waste of resources to test every combination of logger levels.
> This is not a test of the filtering mechanism built into the logger; it is a 
> test that the message is logged when the logger level is set to DEBUG or 
> finer.
> Only two test cases are needed for that.

I see. This test only takes a very short time, but shouldn't the cases be added?

> test/jdk/java/lang/RuntimeTests/RuntimeExitLogTest.java line 78:
> 
>> 76: "java\\.lang\\.Throwable: Runtime\\.exit\\(" + status + 
>> "\\)\\n" +
>> 77: "\\s+at 
>> java\\.base/java\\.lang\\.Shutdown\\.logRuntimeExit\\(Shutdown\\.java:\\d+\\)\\n"
>>  +
>> 78: "\\s+at(?: .+)";
> 
> Testing the exact logging message is too brittle/too exact.
> It should only be testing for the specific variable information that is 
> unique to the case under test.
> It becomes burdensome to update tests when some small change occurs in a 
> message.

The test should verify that the stack traces required for troubleshooting are 
logged correctly.
It avoid verifying line numbers likely to change frequently.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2007141009
PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2007140416

Re: RFR: 8352289: [macos] Review skipped tests in tools/jpackage/macosx/SigningPackage*

[Alexander Matveev]

On Mon, 24 Mar 2025 14:35:50 GMT, Alexey Semenyuk  wrote:

> Follow-up for https://github.com/openjdk/jdk/pull/24087 PR. 
> Revamp signing tests:
>  - Run the tests only if `jpackage.test.MacSignTests` property is set to 
> "run". This property also can be set to "setup" and "teardown" to set up and 
> tear down jpackage signing test environment. See 
> https://github.com/openjdk/jdk/pull/24087 PR.
>  - Moved tests from SigningOptionsTest.java to ErrorTest.java. Altered 
> `JPackageCommand.validateOutput()` to safely handle the case of an empty list 
> of expected strings.
>  - Removed SigningCheck.java. Verification of jpackage signing test 
> environment is performed in `SigningBase.verifySignTestEnvReady()` method 
> using verification API from MacSign class. 
> `SigningBase.verifySignTestEnvReady()` method is called in every signing 
> jtreg test. It is a replacement for `SigningCheck.checkCertificates()` call.
>  - Change signatures of test methods to leverage capabilities of jpackage 
> test lib added in https://github.com/openjdk/jdk/pull/21996 PR.
> 
> Before these changes, if the signing test environment was not set, signing 
> tests exited with "skipped" status. With this change, these tests will not be 
> executed unless `jpackage.test.MacSignTests` property is set to "run". The 
> tests fail if the property is set and the signing test environment is not set 
> correctly. This is a deliberate choice to encourage running these tests only 
> if the signing test environment is configured correctly.

Looks good.

-

Marked as reviewed by almatvee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24201#pullrequestreview-2712013832

Re: RFR: 8352176: Automate setting up environment for mac signing tests [v3]

[Alexey Semenyuk]

> Automate setting up an environment for mac signing tests: create keychain(s), 
> self-signing certificates, and register them in the system.
> 
> To set up the environment, run `make test-only 
> TEST=test/jdk/tools/jpackage/macosx/base/SigningBase.java 
> JTREG=JAVA_OPTIONS=-Djpackage.test.SignEnv=setup` command.
> 
> The above command will create "jpackagerTest.keychain" keychain, one private 
> RSA key, and four self-signed certificates using this key:
> | Certificate common name (CN) | Usage |
> |--|-|
> | Developer ID Application: jpackage.openjdk.java.net|Code sign|
> | Developer ID Installer: jpackage.openjdk.java.net|.pkg sign|
> | Developer ID Application: jpackage.openjdk.java.net (ö)|Code sign|
> | Developer ID Installer: jpackage.openjdk.java.net (ö)|.pkg sign|
> 
> Certificates will be added to the list of trusted certificates using a 
> sequence of `security add-trusted-cert...` commands (one command per 
> certificate). This step will require user interaction to enter the user 
> account password as many times as the number of created certificates (four). 
> A user will be presented with the "Trust certificate" dialog describing which 
> certificate is about to be added to the list of trusted certificates before 
> the dialog prompting the user password pops up:
>  src="https://github.com/user-attachments/assets/a67d0966-2dea-4bc6-93a6-f52dad599898";
>  />
> 
> When the user presses the "OK" button on the "Trust certificate" dialog, the 
> dialog prompting the user password will pop up:
>  src="https://github.com/user-attachments/assets/1d1f022d-54ac-4a7e-8d0a-9bfe65c76b49";
>  />
> 
> Suppose the user presses the "Cancel" button on the "Trust certificate" 
> dialog. In that case, the dialog prompting the user password will NOT pop up, 
> and the whole sequence of adding certificates to the list of trusted 
> certificates will abort.
> 
> If the user presses the "Cancel" button on the dialog prompting the user 
> password, it will be dismissed, and the user will start over with the same 
> "Trust certificate" dialog.
> 
> Every "Trust certificate" dialog has a one-minute timeout. If the dialog is 
> automatically dismissed because of the timeout expiration, adding 
> certificates to the list of trusted certificates will abort.
> 
> To tear down the environment, run `make test-only 
> TEST=test/jdk/tools/jpackage/macosx/base/SigningBase.java 
> JTREG=JAVA_OPTIONS=-Djpackage.test.SignEnv=teardown` command. This command 
> will unlink and delete...

Alexey Semenyuk has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  Make MacSign.setUp() work with LibreSSL. Can specify path to openssl command 
with "jpackage.test.openssl" system property. Add MacSign.isDeployed().

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/24087/files
  - new: https://git.openjdk.org/jdk/pull/24087/files/b59c9776..abf8783a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=24087&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24087&range=01-02

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

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

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v25]

[Maurizio Cimadamore]

On Tue, 1 Apr 2025 13:27:34 GMT, Per Minborg  wrote:

>> Implement JEP 502.
>> 
>> The PR passes tier1-tier3 tests.
>
> Per Minborg has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add lazy toSting for StableMap::values
>  - Make toString for reversed and sublist lazy

src/java.base/share/classes/java/lang/StableValue.java line 50:

> 48: 
> 49: /**
> 50:  * A stable value is a deferred holder of shallowly immutable content.

The JEP introduces this terminology:

> A stable value is an object, of type 
> [StableValue](https://cr.openjdk.org/~pminborg/stable-values2/api/java.base/java/lang/StableValue.html),
>  that holds a single data value, its content

I'm not saying we should copy this definition -- but I note that the word 
`deferred holder` (and also `deferred` in general) only appears once (here) in 
the whole doc. On the other hand, I find many occurrences of `lazy`/`lazily 
computed`, so maybe you want to stick with that here:


A stable value is a holder of shallowly immutable, lazily computed content.

Or

A stable value is a holder of shallowly immutable content that can be lazily 
computed.

src/java.base/share/classes/java/lang/StableValue.java line 52:

> 50:  * A stable value is a deferred holder of shallowly immutable content.
> 51:  * 
> 52:  * A {@code StableValue} can be created using the factory method

s/can be/is (more direct) -- here and elsewhere

-

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024346623
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024347188

Re: RFR: 8352935: Launcher should not add $JDK/../lib to LD_LIBRARY_PATH [v2]

[Jaikiran Pai]

On Thu, 3 Apr 2025 12:57:40 GMT, Jaikiran Pai  wrote:

> Please give me a day or so to run this change against our CI.

tier1, tier2 and tier3 testing of this change went fine in our CI.

-

PR Comment: https://git.openjdk.org/jdk/pull/24351#issuecomment-2777338708

Re: RFR: 8353331: Test ForkJoinPool20Test::testFixedDelaySequence is failing

[Doug Lea]

On Tue, 1 Apr 2025 10:56:25 GMT, Alan Bateman  wrote:

> ForkJoinPool20Test::testFixedDelaySequence is failing intermittently. The 
> assert that checks that the periodically tasks only executes 8 times is 
> removed, it may run more than this before it cancelled.

Yes, the bound is no longer guaranteed after adapting this test from STPE.

-

Marked as reviewed by dl (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24353#pullrequestreview-2732388145

RE: JDK-8352891 Performance improvements to ByteArrayOutputStream

[Engebretson, John]

  I certainly see the value in the unsynchronized variant and am happy to do 
that – but that isn’t sufficient to address the original problem of scaling the 
array.  MemoryOutputStream results in 3x improvement and eliminates the hard 
2GB cap, and can replace custom logic in Tomcat, Spring, etc.  What’s our next 
move there?
  Thanks!  😊
 John

From: Alan Bateman 
Sent: Thursday, April 3, 2025 3:18 AM
To: Engebretson, John ; Markus KARG 
; core-libs-dev@openjdk.org
Subject: RE: [EXTERNAL] JDK-8352891 Performance improvements to 
ByteArrayOutputStream


CAUTION: This email originated from outside of the organization. Do not click 
links or open attachments unless you can confirm the sender and know the 
content is safe.


On 02/04/2025 19:04, Engebretson, John wrote:

  Apologies, human error – here’s the message I intended:

  Thank you!  I’ve updated the PR accordingly and summarized the benchmarks in 
the description.  Here’s the short version:

  1.  For small payloads, unsynchronized and optimized versions are 2-4x faster 
than base
  2.  For large payloads, optimized version is 3x faster than base or 
unsynchronized

  I discovered a capacity-related incompatibility between ByteArrayOutputStream 
and MemoryOutputStream: the size() method returns int, but MemoryOutputStream 
can exceed that value.  I added range checking to size() and a new sizeAsLong() 
method… but it really makes me wonder MemoryOutputStream belongs as a subclass 
of ByteArrayOutputStream.  It now has two significant incompatibilities: 
ignoring the protected fields, and size restrictions.

The protected fields are only accessible to subclasses so it's not an issue. 
BAOS::toByteArray (in addition to size) means the entire content must fit into 
a byte[]. A sink capable of accumulating but bytes that this is a different API.

I think we should at least pursue ByteArrayOutputStream.unsynchronized(int cap) 
and work through the javadoc changes to allow that. It does not need to use 
buf, we have flexibility on how the bytes are buffered.

-Alan

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v26]

[Per Minborg]

> Implement JEP 502.
> 
> The PR passes tier1-tier3 tests.

Per Minborg has updated the pull request incrementally with three additional 
commits since the last revision:

 - Use static final SV in the snippets
 - Update StableValue main description
 - Add stable collections to MOAT tests

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/23972/files
  - new: https://git.openjdk.org/jdk/pull/23972/files/be84045a..0e4ebba9

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=23972&range=25
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23972&range=24-25

  Stats: 35 lines in 7 files changed: 19 ins; 0 del; 16 mod
  Patch: https://git.openjdk.org/jdk/pull/23972.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/23972/head:pull/23972

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

Re: RFR: 8352565: Add native method implementation of Reference.get()

[Chen Liang]

On Sat, 29 Mar 2025 21:47:18 GMT, Kim Barrett  wrote:

> Please review this change which adds a native method providing the
> implementation of Reference::get.  Referece::get is an intrinsic candidate, so
> this native method implementation is only used when the intrinsic is not.
> 
> Currently there is intrinsic support by the interpreter, C1, C2, and graal,
> which are always used.  With this change we can later remove all the
> per-platform interpreter intrinsic implementations, and might also remove the
> C1 intrinsic implementation.
> 
> Testing:
> (1) mach5 tier1-6 normal (so using all the existing intrinsics).
> (2) mach5 tier1-6 with interpreter and C1 Reference::get intrinsics disabled.

src/java.base/share/classes/java/lang/ref/Reference.java line 365:

> 363:  * C2 to sometimes prefer the native implementation over the 
> intrinsic.
> 364:  */
> 365: private native Object get0();

I think you can declare this as `private native T get0();` without changes to 
native method signatures, so you can avoid the unchecked cast above. (See 
Class::getPrimitiveClass declaration)

Also, can C2 choose to use native over intrinsic? That is concerning from a 
performance POV, as I think there are a few such performance sensitive methods 
in core libraries.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24315#discussion_r2022010819

Re: RFR: 8342382: Implementation of JEP G1: Improve Application Throughput with a More Efficient Write-Barrier [v29]

[Thomas Schatzl]

> Hi all,
> 
>   please review this change that implements (currently Draft) JEP: G1: 
> Improve Application Throughput with a More Efficient Write-Barrier.
> 
> The reason for posting this early is that this is a large change, and the JEP 
> process is already taking very long with no end in sight but we would like to 
> have this ready by JDK 25.
> 
> ### Current situation
> 
> With this change, G1 will reduce the post write barrier to much more resemble 
> Parallel GC's as described in the JEP. The reason is that G1 lacks in 
> throughput compared to Parallel/Serial GC due to larger barrier.
> 
> The main reason for the current barrier is how g1 implements concurrent 
> refinement:
> * g1 tracks dirtied cards using sets (dirty card queue set - dcqs) of buffers 
> (dirty card queues - dcq) containing the location of dirtied cards. 
> Refinement threads pick up their contents to re-refine. The barrier needs to 
> enqueue card locations.
> * For correctness dirty card updates requires fine-grained synchronization 
> between mutator and refinement threads,
> * Finally there is generic code to avoid dirtying cards altogether (filters), 
> to avoid executing the synchronization and the enqueuing as much as possible.
> 
> These tasks require the current barrier to look as follows for an assignment 
> `x.a = y` in pseudo code:
> 
> 
> // Filtering
> if (region(@x.a) == region(y)) goto done; // same region check
> if (y == null) goto done; // null value check
> if (card(@x.a) == young_card) goto done;  // write to young gen check
> StoreLoad;// synchronize
> if (card(@x.a) == dirty_card) goto done;
> 
> *card(@x.a) = dirty
> 
> // Card tracking
> enqueue(card-address(@x.a)) into thread-local-dcq;
> if (thread-local-dcq is not full) goto done;
> 
> call runtime to move thread-local-dcq into dcqs
> 
> done:
> 
> 
> Overall this post-write barrier alone is in the range of 40-50 total 
> instructions, compared to three or four(!) for parallel and serial gc.
> 
> The large size of the inlined barrier not only has a large code footprint, 
> but also prevents some compiler optimizations like loop unrolling or inlining.
> 
> There are several papers showing that this barrier alone can decrease 
> throughput by 10-20% 
> ([Yang12](https://dl.acm.org/doi/10.1145/2426642.2259004)), which is 
> corroborated by some benchmarks (see links).
> 
> The main idea for this change is to not use fine-grained synchronization 
> between refinement and mutator threads, but coarse grained based on 
> atomically switching card tables. Mutators only work on the "primary" card 
> table, refinement threads on a se...

Thomas Schatzl has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 37 commits:

 - Merge branch 'master' into 8342382-card-table-instead-of-dcq
 - Merge branch 'master' into 8342382-card-table-instead-of-dcq
 - Merge branch 'master' into submit/8342382-card-table-instead-of-dcq
 - * make young gen length revising independent of refinement thread
 * use a service task
 * both refinement control thread and young gen length revising use the 
same infrastructure to get the number of available bytes and determine the time 
to the next update
 - * fix IR code generation tests that change due to barrier cost changes
 - * factor out card table and refinement table merging into a single
 method
 - Merge branch 'master' into 8342382-card-table-instead-of-dcq3
 - * obsolete G1UpdateBufferSize
   
   G1UpdateBufferSize has previously been used to size the refinement
   buffers and impose a minimum limit on the number of cards per thread
   that need to be pending before refinement starts.
   
   The former function is now obsolete with the removal of the dirty
   card queues, the latter functionality has been taken over by the new
   diagnostic option `G1PerThreadPendingCardThreshold`.
   
   I prefer to make this a diagnostic option is better than a product option
   because it is something that is only necessary for some test cases to
   produce some otherwise unwanted behavior (continuous refinement).
   
   CSR is pending.
 - * more documentation on why we need to rendezvous the gc threads
 - Merge branch 'master' into 8342381-card-table-instead-of-dcq
 - ... and 27 more: https://git.openjdk.org/jdk/compare/aff5aa72...51fb6e63

-

Changes: https://git.openjdk.org/jdk/pull/23739/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23739&range=28
  Stats: 7089 lines in 110 files changed: 2610 ins; 3555 del; 924 mod
  Patch: https://git.openjdk.org/jdk/pull/23739.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/23739/head:pull/23739

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

Re: RFR: 8352064: AIX: now also able to build static-jdk image with a statically linked launcher

[Joachim Kern]

On Mon, 17 Mar 2025 15:01:10 GMT, Joachim Kern  wrote:

>> Looks like this variable is misspelled (missing "S").
>> 
>> DEPS := $(STATIC_LIB_FILE), \
>> 
>> Also note that by default log level INFO is not printed. You would need to 
>> run the build with `make LOG=info` to see the message.
>
> Ups, thank you for giving me the 'S'. But nevertheless if I make with
> `make all LOG=info` an `$(info generate_export_list: 
> $(generate_export_list))` still shows an empty list

I did not get the SetupExecute function, even if I strip it down to a simple 
example

$(eval $(call SetupExecute, generate_export_list, \
INFO := Generating export list for static libs, \
DEPS := /path/libjli.a, \
OUTPUT_FILE := /path/libjli.a.exp, \
COMMAND := $(AR) $(ARFLAGS) -w $$(patsubst %.exp, %, $$@) | $(GREP) -v '^.' 
| $(AWK) '{print 1}' | $(SORT) -u >$$@, \
))

$(java): $(STATIC_LIB_FILES) /path/libjli.a.exp

I still get
`gmake[3]: *** No rule to make target '/path/libjli.a.exp', needed by 
'/path2/java'.  Stop.
`
Why does make not recognize, that the rule is in my
 `call SetupExecute, generate_export_list,`
macro?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24062#discussion_r2000736109

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v28]

[Per Minborg]

> Implement JEP 502.
> 
> The PR passes tier1-tier3 tests.

Per Minborg has updated the pull request incrementally with three additional 
commits since the last revision:

 - Add a dependency layout image snippet
 - Wip on condy
 - Use @enablePreview in MOAT

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/23972/files
  - new: https://git.openjdk.org/jdk/pull/23972/files/ab842789..de3d5616

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=23972&range=27
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23972&range=26-27

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

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

Re: RFR: 8351593: [JMH] test PhoneCode.Bulk reports NPE exception

[Claes Redestad]

On Wed, 12 Mar 2025 15:53:33 GMT, Vladimir Ivanov  wrote:

> the test output was cleanup for common execution.

I would argue against that. 

While tests and test resources already comprise ~360M it could be argued that 
adding this 3.5M dictionary file wouldn't hurt all that much -- but OTOH this 
benchmark was ported over in a bulk fashion and obviously hasn't done what it's 
supposed to be doing ever since. I lean towards removing defunct and otherwise 
questionable microbenchmarks from the inlined JMH suite.

-

PR Comment: https://git.openjdk.org/jdk/pull/24011#issuecomment-2741441341

Re: RFR: 8304674: File java.c compile error with -fsanitize=address -O0 [v2]

[SendaoYan]

> Hi all,
> File src/java.base/share/native/libjli/java.c compile error: control reaches 
> end of non-void function [-Werror=return-type] with gcc options 
> -fsanitize=address -O0. The function int JavaMain(void* _args) in this file 
> will execute return ret in LEAVE() macro, but gcc with -O0 is not smart 
> enough to recognized that the function already has return statement before at 
> the end of function. It's a gcc bug which has been recorded by 
> [80959](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80959), and below code 
> snippet can demonstrate the gcc bug. I think we should disable return-type 
> gcc warning for java.c file before the gcc bug has been fixed. Risk is low.
> 
> 
>> cat java.c
> char a() {
>   return 0;
>   int b;
>   if (a(&b))
> return 0;
> }
> 
>> gcc -O0 -Wall -Wextra -Werror -O0 -c java.c -fsanitize=address
> java.c: In function ‘a’:
> java.c:6:1: error: control reaches end of non-void function 
> [-Werror=return-type]
> 6 | }
>   | ^
> cc1: all warnings being treated as errors

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

  Use #pragma GCC diagnostic ignored "-Wreturn-type" instead of use 
diasable_warning in makefile

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/24318/files
  - new: https://git.openjdk.org/jdk/pull/24318/files/78785e27..83beb975

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=24318&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24318&range=00-01

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

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

Re: Reg: Proposal: Generalized Number.parseNumber(String) Method for Java

[Joseph D. Darcy]

Another reaction: in practice there is not much numeric about 
java.lang.Number. Operationally the type means "convertible to a 
primitive type" and there is nothing else extending Number lets you do 
with a class. Additionally in retrospect, Number would have been better 
as an interface rather than an abstract class.


When looking to augment numeric capabilities of the platform, directly 
involving java.lang.Number is not the first or second place I'd look.


Cheers,

-Joe

On 3/28/2025 11:38 AM, Kevin Bourrillion wrote:
I appreciate you raising this; I think it is relevant to some internal 
discussions we’re having about the future of Number. Here’s some 
personal reactions from a team member (not a team consensus opinion):


Unfortunately, we’ve found many times that `Number` is a pretty 
deficient return type that users can’t do much of anything useful with 
(except apply arbitrarily lossy conversions to, or test with 
`instanceof`).


And I’m skeptical that most users ever want to end up in a situation 
where they have a `Number[]` or `Collection` that could be 
heterogeneous. This state feels like a temporary one you want to get 
out of.


It’s not clear that `Number` can really be rehabilitated much, either. 
It is sort of a “deficient type” by its nature.


When you do need this sort of flexible parsing, I think the status quo 
is: you can always parse to BigDecimal first, then ask questions about 
what more “minimal” type the value you got might fit into. Obviously 
this does not have optimal performance, but I am not sure there’s a 
good spot-fix here that doesn’t just raise more issues than it tried 
to resolve.


We will keep thinking about this, though. Valhalla means we will 
inevitably have more "numeric types" than ever, and this is prompting 
us to think about what these types should have in common. (It’s a big 
topic and I apologize for not even attempting to outline it all for 
you here yet.)




On Mar 28, 2025, at 10:29 AM, Sathish Kumar Thiyagarajan 
 wrote:


Dear Core-Libs Dev Team,

*Note:* I have now subscribed to the mailing list and am resending 
this message as advised.


I would like to propose an improvement to the JDK: a *generalized 
|Number.parseNumber(String)| method*.



  Motivation

Java provides multiple number types (|byte|, |short|, |int|, |long|, 
etc.), and developers typically choose them based on memory 
considerations. Currently, Java offers |String| to |Number| 
conversions using concrete classes:


 *

|Long.parseLong(String)|

 *

|Integer.parseInt(String)|

 *

|Short.parseShort(String)|, etc.

While these are useful, Java lacks a *generalized method* that 
returns the most memory-efficient |Number| representation based on 
the input, like:


|Number.parseNumber(String numberAsText); |


  Use Case: JSON Serialization

This would be particularly useful in cases like *JSON serialization 
in REST APIs (Using Jackson )*, 
where number types are often altered during 
serialization/deserialization. Consider the following test case:


|@Test void testNumberMemoryUsage() throws JsonProcessingException { 
ObjectMapper mapper = new ObjectMapper(); Map 
numbersObject = Map.of("aShort", (short) 1234, "aFloat", (float) 
1.33); final String jsonText = 
mapper.writeValueAsString(numbersObject); Map 
parsedJsonObject = mapper.readValue(jsonText, new TypeReference<>() 
{}); // Expected: Short.class | Actual: Integer.class 
assertEquals(Short.class, parsedJsonObject.get("aShort").getClass()); 
// Expected: Float.class | Actual: Double.class 
assertEquals(Float.class, parsedJsonObject.get("aFloat").getClass()); } |



  Reference Implementation

Here’s a rough implementation to illustrate the idea:

|private static Number parseNumber(final String numberStr) { try { if 
(numberStr.contains(".")) { double doubleValue = 
Double.parseDouble(numberStr); return (doubleValue >= 
-Float.MAX_VALUE && doubleValue <= Float.MAX_VALUE) ? (float) 
doubleValue : doubleValue; } else { long longValue = 
Long.parseLong(numberStr); if (longValue >= Byte.MIN_VALUE && 
longValue <= Byte.MAX_VALUE) { return (byte) longValue; } else if 
(longValue >= Short.MIN_VALUE && longValue <= Short.MAX_VALUE) { 
return (short) longValue; } else if (longValue >= Integer.MIN_VALUE 
&& longValue <= Integer.MAX_VALUE) { return (int) longValue; } else { 
return longValue; } } } catch (NumberFormatException e) { return 
parseBigNumber(numberStr); } } private static Number 
parseBigNumber(final String numberStr) { try { return new 
BigInteger(numberStr); // Try BigInteger first } catch 
(NumberFormatException e) { // Only create BigDecimal if BigInteger 
fails BigDecimal bd = new BigDecimal(numberStr); try { // Convert to 
BigInteger if there's no fraction return bd.toBigIntegerExact(); } 
catch (ArithmeticException ex) { return bd; // If it's a decimal, 
return BigDecimal } } } |


Would love to hear your thoughts on this proposal. Appreciate your 
fe

Re: RFR: 8352689: Allow for hash sum overrides when linking from the run-time image [v2]

[Severin Gehwolf]

On Wed, 26 Mar 2025 10:12:43 GMT, Alan Bateman  wrote:

> > The cacerts issue mentioned in the JBS issue relates to an RPM installation 
> > of the JDK where the cacerts file is a symlink to the distro provided one. 
> > So I think that's "use system" issue.
> > TZ updates would potentially break this too. If an external tool updates 
> > `tzdb.dat` then the hash sum computed at JDK build-time will no longer 
> > match. I believe this could also be solved with a sha-override (e.g. by 
> > coming from a file `@${java.home}/sha-override.txt`) which would get the 
> > update along with `tzdb.dat`.
> 
> I think one direction to explore is configuring which files or directory are 
> "upgradable". Upgradable modules aren't excluded from the hash computation to 
> allow upgrade via the upgrade module path. Something similar here would allow 
> jlink to report that tzdb.dat has been upgraded. Maybe cacerts is the same 
> but need to look closer as the hash computation shouldn't be following sym 
> links outside of the runtime image.

I'll keep looking into this specific case. However, it sounds a bit orthogonal 
to the patch at hand which I do believe we still need for the original reasons 
mentioned (RPM changing binaries after the JDK build is long done and the 
windows issue of the JDK build itself placing different *.pdb files into the 
image than was present at jlink time). So perhaps we should explore this in 
parallel?

-

PR Comment: https://git.openjdk.org/jdk/pull/24190#issuecomment-2754680306

Re: RFR: 8352693: Use a simpler console reader instead of JLine for System.console()

[Magnus Ihse Bursie]

On Wed, 26 Mar 2025 07:54:48 GMT, Jan Lahoda  wrote:

> The `java.io.Console` has several backends: a simple on in `java.base`, a 
> more convenient one in `jdk.internal.le` (with line-reading based on JLine) 
> and one for JShell.
> 
> The backend based on JLine is proving to be a somewhat problematic - JLine is 
> very powerful, possibly too powerful and complex for the simple task of 
> editing a line with no completion, no history, no variables, no commands, 
> etc. As a consequence, there are inevitable sharp edges in this backend.
> 
> The idea in this PR is to replace the use of JLine in the `jdk.internal.le` 
> backend with a simple escape code interpreter, that only handles a handful of 
> keys/codes (left/right arrow, home, end, delete, backspace, enter), and 
> ignores the rest. The goal is to have something simple with less surprising 
> behavior.

src/jdk.internal.le/windows/native/lible/WindowsTerminal.cpp line 32:

> 30: #include 
> 31: #include 
> 32: //#include 

Should you really check in new files with commented-out code?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24242#discussion_r2020962013

Re: RFR: 8352015: LIBVERIFY_OPTIMIZATION remove special optimization settings

[Magnus Ihse Bursie]

On Fri, 14 Mar 2025 11:15:26 GMT, Matthias Baesken  wrote:

> On Linux there are some special settings for LIBVERIFY_OPTIMIZATION that are 
> most likely not needed any more and could be removed.
> The removal (on Linux) brings the lib optimization level de facto from LOW to 
> HIGH.

Marked as reviewed by ihse (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/24054#pullrequestreview-2694167632

Re: RFR: 8326447: jpackage creates Windows installers that cannot be signed [v3]

[Guillaume Toison]

On Sat, 29 Mar 2025 23:49:48 GMT, Cormac Redmond  wrote:

>> Alexey Semenyuk has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Keep bundles produces by the test in the test work directory
>
> I am seeing this also, after switching from Azul to Temurin (JDK23).
> 
> It's not just the installer, the application .exe itself cannot be signed 
> either.
> 
> Something is definitely not correct with Temurin's release.

@credmond see: https://github.com/adoptium/adoptium-support/issues/1244
The issue seems to be with the Temurin build

-

PR Comment: https://git.openjdk.org/jdk/pull/23732#issuecomment-2764441490

Re: Reg: Proposal: Generalized Number.parseNumber(String) Method for Java

[Sathish Kumar Thiyagarajan]

Thanks, Volkan.

I agree that JSON parsers may have their own number codecs and may not
necessarily rely on Integer.parseInt() or Long.parseLong(). I appreciate
the clarification.

Let me simplify my concern:

I've tried converting a *String to a Number* and loading it into memory,
and my expectation is that it should *occupy the least memory possible*.
Wouldn't that be the ideal approach? If so, does it make sense to have a
utility that helps achieve this efficiently?


On Tue, 1 Apr 2025 at 14:12, Volkan Yazıcı  wrote:

> Sathish, please see my response below.
>
> On Tue, Apr 1, 2025 at 7:58 AM Sathish Kumar Thiyagarajan <
> sathishkumar.thiyagara...@gmail.com> wrote:
>
>> However, *Jackson (and most JSON parsers) default to Integer for whole
>> numbers* because they rely on Integer.parseInt(String), which is a safe
>> fallback for most cases.
>>
> AFAIK, almost all JSON parsers use their own number codecs; i.e., encoders
> and decoders. In the context of Jackson, `new ObjectMapper().readValue("1",
> Number.class)` ends up in
> `com.fasterxml.jackson.core.io.NumberInput::parseInt`, not
> `Integer::parseInt`. There are multiple reasons for such libraries to roll
> out their own number codecs; performance, matching the internal string
> representation of library's choice, etc. It is very unlikely adding a
> `Number::parseNumber` will bring any benefit to such use cases.
>
> [Quoting from another post
> ] It
> is better to frame feedback in the form of *"I’ve tried this, and run
> into the following problem ..."* At the very least it helps us see what
> problems people actually run into as opposed to what problems people
> speculate that they or others may run into.
>
>>

Withdrawn: 8352107: Allow jtreg test cases to query test VM properties

[Ioi Lam]

On Sun, 16 Mar 2025 02:54:36 GMT, Ioi Lam  wrote:

> This PR tries to cut down the use of `WhiteBox` in the HotSpot test cases. It 
> modifies `VMProps` to save the set of VM properties into a file called 
> "vm.properties" under Jtreg's work directory. The new API 
> `jdk.test.lib.VMPropsGetter` loads the properties from this file to pass onto 
> individual test cases.
> 
> See `getJtregWorkDir()` for the code that figures out the work directory. It 
> assumes that `VMProps` and all test cases are always executed under
> 
> - `workDir/scratch/` ; or
> - `workDir/scratch/[0-9]+/` 
> 
> This is probably not the right thing to do. I would be better for Jtreg to 
> pass the location of the work directory to the test cases, with a 
> command-line options like `-Djtreg.work.dir=`.
> 
> To show the benefit of this PR, I modified a few test cases to remove their 
> use of WhiteBox.

This pull request has been closed without being integrated.

-

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

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v6]

[Per Minborg]

On Mon, 17 Mar 2025 01:58:55 GMT, Luca Kellermann  wrote:

> > I see that, probably due to prior `java.util` contracts, a stable list or 
> > map cannot present a `toString` with unset component values.  A stable list 
> > or map uses a “canned” `toString` method that calls `get`, which must force 
> > all component values to be evaluated before the `toString` can be printed.
> 
> I also noticed this issue of `toString` eagerly setting all elements of 
> stable collections and agree that it probably shouldn't do this. Note that 
> all views of these collections (obtained via `List.subList`, `List.reversed`, 
> `Map.entrySet`, `Map.values`, etc.) would also need their own `toString` 
> implementation.
> 
> > Just as `WeakHashMap` bends the `Map` API (regarding `equals`), I think 
> > `StableValue` composites should bend the `List` and `Map` APIs, regarding 
> > `toString`.  Sometimes the contracts have to be bent for the whole design 
> > to fit together.
> 
> Neither `List`, `Set`, nor `Map` mention any requirements for `toString` in 
> their interface specification. Only `AbstractCollection` and `AbstractMap` 
> have a default implementation of `toString`. So I don't think any contract 
> would be bent here.

I think requiring the `toString()` method of `StableList::subList` and 
`StableList::reversed` is a bit too much, at least in the first incarnation. 
However, I have noted this as a candidate in future implementations.

-

PR Comment: https://git.openjdk.org/jdk/pull/23972#issuecomment-2765490546

Re: RFR: 8352565: Add native method implementation of Reference.get() [v3]

[Kim Barrett]

> Please review this change which adds a native method providing the
> implementation of Reference::get.  Referece::get is an intrinsic candidate, so
> this native method implementation is only used when the intrinsic is not.
> 
> Currently there is intrinsic support by the interpreter, C1, C2, and graal,
> which are always used.  With this change we can later remove all the
> per-platform interpreter intrinsic implementations, and might also remove the
> C1 intrinsic implementation.
> 
> Testing:
> (1) mach5 tier1-6 normal (so using all the existing intrinsics).
> (2) mach5 tier1-6 with interpreter and C1 Reference::get intrinsics disabled.

Kim Barrett has updated the pull request incrementally with one additional 
commit since the last revision:

  add package decl to test

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/24315/files
  - new: https://git.openjdk.org/jdk/pull/24315/files/37dc9b74..36bb26a1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=24315&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24315&range=01-02

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

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

Re: RFR: 8066583: DeflaterInput/OutputStream and InflaterInput/OutputStream should explain responsibility for freeing resources [v6]

[Jaikiran Pai]

On Tue, 1 Apr 2025 00:29:47 GMT, Chen Liang  wrote:

>> src/java.base/share/classes/java/util/zip/InflaterOutputStream.java line 89:
>> 
>>> 87:  * @apiNote
>>> 88:  * The default decompressor will be closed when this output stream
>>> 89:  * is {@linkplain #close() closed}.
>> 
>> This can be normative, meaning you can drop the apiNote.
>
> Does the original specification imply a default decompressor is created, or 
> is such a default decompressor simply used?

Hello Chen, if you mean whether multiple instances of these streams can share 
the same default decompressor, then no, they can't (and they don't). Each 
creates its own instance of default decompressor/compressor.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/23655#discussion_r2022293348

Re: RFR: 8349206: j.u.l.Handler classes create deadlock risk via synchronized publish() method [v13]

[duke]

On Wed, 2 Apr 2025 21:59:32 GMT, David Beaumont  wrote:

>> 8349206: j.u.l.Handler classes create deadlock risk via synchronized 
>> publish() method.
>> 
>> 1. Remove synchronization of calls to publish() in Handlers in 
>> java.util.logging package.
>> 2. Add explanatory comments to various affected methods.
>> 3. Add a test to ensure deadlocks no longer occur.
>> 
>> Note that this change does not address issue in MemoryHandler (see 
>> JDK-8349208).
>
> David Beaumont has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Tweak wording.

@david-beaumont 
Your change (at version 8db4c907633fed5aa103b3c4f6aaad5096f0790a) is now ready 
to be sponsored by a Committer.

-

PR Comment: https://git.openjdk.org/jdk/pull/23491#issuecomment-2776774585

Re: RFR: 8352176: Automate setting up environment for mac signing tests [v6]

[Alexey Semenyuk]

> Automate setting up an environment for mac signing tests: create keychain(s), 
> self-signing certificates, and register them in the system.
> 
> To set up the environment, run `make test-only 
> TEST=test/jdk/tools/jpackage/macosx/base/SigningBase.java 
> JTREG=JAVA_OPTIONS=-Djpackage.test.SignEnv=setup` command.
> 
> The above command will create "jpackagerTest.keychain" keychain, one private 
> RSA key, and four self-signed certificates using this key:
> | Certificate common name (CN) | Usage |
> |--|-|
> | Developer ID Application: jpackage.openjdk.java.net|Code sign|
> | Developer ID Installer: jpackage.openjdk.java.net|.pkg sign|
> | Developer ID Application: jpackage.openjdk.java.net (ö)|Code sign|
> | Developer ID Installer: jpackage.openjdk.java.net (ö)|.pkg sign|
> 
> Certificates will be added to the list of trusted certificates using a 
> sequence of `security add-trusted-cert...` commands (one command per 
> certificate). This step will require user interaction to enter the user 
> account password as many times as the number of created certificates (four). 
> A user will be presented with the "Trust certificate" dialog describing which 
> certificate is about to be added to the list of trusted certificates before 
> the dialog prompting the user password pops up:
>  src="https://github.com/user-attachments/assets/a67d0966-2dea-4bc6-93a6-f52dad599898";
>  />
> 
> When the user presses the "OK" button on the "Trust certificate" dialog, the 
> dialog prompting the user password will pop up:
>  src="https://github.com/user-attachments/assets/1d1f022d-54ac-4a7e-8d0a-9bfe65c76b49";
>  />
> 
> Suppose the user presses the "Cancel" button on the "Trust certificate" 
> dialog. In that case, the dialog prompting the user password will NOT pop up, 
> and the whole sequence of adding certificates to the list of trusted 
> certificates will abort.
> 
> If the user presses the "Cancel" button on the dialog prompting the user 
> password, it will be dismissed, and the user will start over with the same 
> "Trust certificate" dialog.
> 
> Every "Trust certificate" dialog has a one-minute timeout. If the dialog is 
> automatically dismissed because of the timeout expiration, adding 
> certificates to the list of trusted certificates will abort.
> 
> To tear down the environment, run `make test-only 
> TEST=test/jdk/tools/jpackage/macosx/base/SigningBase.java 
> JTREG=JAVA_OPTIONS=-Djpackage.test.SignEnv=teardown` command. This command 
> will unlink and delete...

Alexey Semenyuk has updated the pull request incrementally with one additional 
commit since the last revision:

  Rename jpackage.test.SignEnv in jpackage.test.MacSignTests. Add 
SigningBase.verifySignTestEnvReady()

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/24087/files
  - new: https://git.openjdk.org/jdk/pull/24087/files/04c465ec..c7c5c975

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=24087&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24087&range=04-05

  Stats: 41 lines in 4 files changed: 26 ins; 1 del; 14 mod
  Patch: https://git.openjdk.org/jdk/pull/24087.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24087/head:pull/24087

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

RFR: 8353585: Provide ChoiceFormat#parse(String, ParsePosition) tests

[Justin Lu]

Please review this PR which provides unit tests for `ChoiceFormat#parse(String, 
ParsePosition)` to check default, multi match, and no match behavior. There 
were no existing relevant tests.

-

Commit messages:
 - more cases + cleanup
 - init

Changes: https://git.openjdk.org/jdk/pull/24395/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24395&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8353585
  Stats: 74 lines in 1 file changed: 74 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/24395.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24395/head:pull/24395

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

Re: RFR: 8304674: File java.c compile error with -fsanitize=address -O0

[SendaoYan]

On Mon, 31 Mar 2025 12:57:38 GMT, Magnus Ihse Bursie  wrote:

> While I normally advocate using DISABLE_WARNING in makefiles instead of 
> pragmas, in this particular case I wonder if not a pragma in the `LEAVE` 
> macro would be better?

PR has been changed to use `#pragma GCC diagnostic ignored "-Wreturn-type"` 
instead of used DISABLE_WARNING in makefile. Change has been verified locally.

-

PR Comment: https://git.openjdk.org/jdk/pull/24318#issuecomment-2768435238

Re: RFR: 8352689: Allow for hash sum overrides when linking from the run-time image [v2]

[Severin Gehwolf]

On Wed, 26 Mar 2025 17:43:50 GMT, Christoph Langer  wrote:

> Would it maybe make sense/be possible to offer some re-hash functionality for 
> using in 2nd step builds?

What would that be? Right now linking from the run-time image doesn't allow for 
`jdk.jlink` to be included, which prevents a 2nd step build.

-

PR Comment: https://git.openjdk.org/jdk/pull/24190#issuecomment-2755304861

Re: RFR: 8344708: Compiler Implementation of Module Import Declarations [v2]

[Jan Lahoda]

> This is a patch to finalize the module imports feature. Please see:
> https://bugs.openjdk.org/browse/JDK-8344700

Jan Lahoda has updated the pull request incrementally with four additional 
commits since the last revision:

 - Cleanup, fixing tests.
 - Adjusting JShell defaults.
 - Fixing tests.
 - Cleanup - updated specification will permit requires transitive java.base; 
for all classfile versions; java.se no longer participates in preview.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/23801/files
  - new: https://git.openjdk.org/jdk/pull/23801/files/46c8454a..cffcf851

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=23801&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23801&range=00-01

  Stats: 306 lines in 18 files changed: 219 ins; 50 del; 37 mod
  Patch: https://git.openjdk.org/jdk/pull/23801.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/23801/head:pull/23801

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

Re: JDK-8352891 Performance improvements to ByteArrayOutputStream

[Alan Bateman]

On 31/03/2025 16:51, Engebretson, John wrote:


  Alan – is this what you have in mind:

ByteArrayOutputStream.getInstance() // returns existing class

ByteArrayOutputStream.getUnsynchronizedInstance() // returns subclass 
of BAOS that overrides the synchronization


ByteArrayOutputStream.getInstance() // 
returns the new class





BAOS has been synchronized since JDK 1.0. While undocumented, it's 
possible that existing code depends on this 30 year behavior so I think 
we are stuck with it.


The removal of biased locking has spurred on a few complaints that the 
class is needlessly synchronized. A static factory to return an 
unsynchronized BOAS would help but only if it isn't used with code that 
assumes all operations are synchronized. So I think we will have to look 
at the API docs for this.


It's not clear that we need to have several implementation with 
different performance tradeoffs. So I think part of the exploration will 
be to see what usages perform better or worse, and whether having a 
parameter to specify the initial size or some hint of the max size would 
help the discussion.


-Alan

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v6]

[Per Minborg]

On Thu, 13 Mar 2025 15:44:37 GMT, Maurizio Cimadamore  
wrote:

>> Per Minborg has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 246 commits:
>> 
>>  - Merge branch 'master' into implement-jep502
>>  - Clean up exception messages and fix comments
>>  - Rename field
>>  - Rename method and fix comment
>>  - Rework reenterant logic
>>  - Use acquire semantics for reading rather than volatile semantics
>>  - Add missing null check
>>  - Simplify handling of sentinel, wrap, and unwrap
>>  - Fix JavaDoc issues
>>  - Fix members in StableEnumFunction
>>  - ... and 236 more: https://git.openjdk.org/jdk/compare/4e51a8c9...d6e1573f
>
> src/java.base/share/classes/java/lang/StableValue.java line 339:
> 
>> 337:  * which would introduce security vulnerabilities.
>> 338:  * 
>> 339:  * As objects can be set via stable values but never removed, this can 
>> be a source
> 
> It feels like this could probably be expanded upon -- also covering stable 
> functions (and morphed into a new section)

I do not understand the comment. Each factory has a note on `Serializable` and 
now there is no general comment about security issues as per comments made 
earlier. Can you elaborate, please?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2004612892

Integrated: 8352276: Skip jtreg tests using native executable with libjvm.so/libjli.so dependencies on static JDK

[Jiangli Zhou]

On Tue, 18 Mar 2025 18:57:04 GMT, Jiangli Zhou  wrote:

> Please review this PR that adds `@requires !jdk.static` to tests, thanks.
> 
> - runtime/StackGap/TestStackGap.java
> - runtime/StackGuardPages/TestStackGuardPages.java
> - runtime/TLS/TestTLS.java
> - runtime/jni/daemonDestroy/TestDaemonDestroy.java
> - runtime/jni/getCreatedJavaVMs/TestGetCreatedJavaVMs.java
> - jdk/java/lang/reflect/exeCallerAccessTest/CallerAccessTest.java
> - jdk/jni/nullCaller/NullCallerTest.java
> - jdk/tools/launcher/JniInvocationTest.java
> 
> These tests use native executables that have dependencies on `libjvm.so`, 
> `libjli.so`, etc. Static JDK does not provide the separate JDK/VM shared 
> libraries, hence cannot support the specific testing mode.

This pull request has now been integrated.

Changeset: 91836e18
Author:Jiangli Zhou 
URL:   
https://git.openjdk.org/jdk/commit/91836e181a789ef16e8d70bfde4c040e6f5031db
Stats: 8 lines in 8 files changed: 8 ins; 0 del; 0 mod

8352276: Skip jtreg tests using native executable with libjvm.so/libjli.so 
dependencies on static JDK

Reviewed-by: dholmes

-

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

Re: RFR: 8352935: Launcher should not add $JDK/../lib to LD_LIBRARY_PATH

[Magnus Ihse Bursie]

On Tue, 1 Apr 2025 09:13:45 GMT, Joachim Kern  wrote:

> In the JDK launcher, there is a codepath which would set/modify the 
> LD_LIBRARY_PATH. This happens unconditionally on AIX and Linux/musl and can 
> also happen on other Linux platforms if an LD_LIBRARY_PATH is pre-set which 
> contains a libjvm.so.
> 
> The LD_LIBRARY_PATH is set to $JVMPATH:$JDK/lib:$JDK/../lib. The last part of 
> this, $JDK/../lib, seems to be a remnant of times when there was a jre 
> subfolder in a JDK deployment. So it should likely be removed.

This seems reasonable to me. But there is so much old yucky stuff going on with 
the JDK bootstrapping issues, that I still get a bit nervous every time someone 
changes it.

Do you know *why* we do this, i.e. modify LD_LIBRARY_PATH?

-

PR Comment: https://git.openjdk.org/jdk/pull/24351#issuecomment-2772870911

Re: RFR: 8351996: Behavioral updates for ClassValue::remove [v2]

[Jaikiran Pai]

On Wed, 19 Mar 2025 21:35:53 GMT, Chen Liang  wrote:

>> The recent patch #23866 makes calling `ClassValue::remove()` from 
>> `ClassValue::computeValue()` end up in infinite loops while fixing the stale 
>> value risk from the method.
>> 
>> The proposed fix is to preserve the stale value risk fix, and update the 
>> remove-from-compute behavior from the original designated no-op behavior to 
>> throwing an exception, as the original behavior conflicts with the stale 
>> value fix.
>> 
>> The implementation track the owner thread in promises (accessed in locked 
>> section); as a result, we can fail-fast on recursive removals from 
>> `computeValue`. I did not choose to use `ThreadTracker` as it is designed 
>> for single tracker and multiple threads, while this case here sees often 
>> just one thread, and the threads outlive the promise objects.
>> 
>> Also updated the API specs for `remove` to more concisely describe the 
>> memory effects. Please review the associated CSR as well.
>
> Chen Liang 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 four additional commits since 
> the last revision:
> 
>  - Use identity of thread, some optimizations for single thread case
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> fix/classvalue-compute-remove
>  - Track threads on the promise for cheap reentrancy checks
>  - 8351996: Alternative way to ensure no stale values for ClassValue::remove

Hello Chen, not a review of the code, but the tier1 failures in the GitHub 
actions jobs look related:


java.lang.StackOverflowError: Recursive initialization of class value
at 
java.base/java.lang.ClassValue$Entry.registerExtraThread(ClassValue.java:321)
at 
java.base/java.lang.ClassValue$ClassValueMap.startEntry(ClassValue.java:481)
at java.base/java.lang.ClassValue.getFromHashMap(ClassValue.java:196)
at java.base/java.lang.ClassValue.getFromBackup(ClassValue.java:183)
at java.base/java.lang.ClassValue.get(ClassValue.java:119)

-

PR Comment: https://git.openjdk.org/jdk/pull/24043#issuecomment-2743411188

Re: RFR: 8351933: Inaccurate masking of TC subfield decrement in ForkJoinPool

[Chen Liang]

On Thu, 13 Mar 2025 13:34:51 GMT, Dmitry Chuyko  wrote:

> Please review a tiny fix in the ForkJoinPool. Since JDK 9 (JDK-8134852 [1]) 
> in one case when TC subfield in ctl field is decremented, the applied masking 
> (UMASK, upper bits) may not preserve neighbor RC subfield sometimes. In JDKs 
> prior to 19 FJP may stop executing tasks, which requires a long running 
> application restart [2]. Since 19 it is even harder to reproduce because of 
> the separate parallelism field.
> 
> The fix is to replace 'UMASK & (c - TC_UNIT)'  with '(c & RC_MASK) | ((c - 
> TC_UNIT)  & TC_MASK)' which preserves the RC part of the compareAndSetCtl() 
> candidate argument. On 17u and 11u that repairs known tests and applications. 
> This PR is for the mainline, and I intend to backport it to 21u, 17u and 11u.
> 
> [1] https://bugs.openjdk.org/browse/JDK-8134852
> [2] https://bugs.openjdk.org/browse/JDK-8330017

The issue looks valid to me and the fix looks good. Would be nice if another 
reviewer can double check.

-

Marked as reviewed by liach (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24034#pullrequestreview-2691756966

Re: RFR: 8352971: Increase maximum number of hold counts for ReentrantReadWriteLock

[Alan Bateman]

On Wed, 26 Mar 2025 16:19:16 GMT, Viktor Klang  wrote:

> I'm breaking this change out as a separate improvement, since it will not be 
> generally possible to adjust these limits on the j.u.c primitives since they 
> might already use a backing `long` to pack in information which needs to be 
> updated atomically (would require 128-bit atomics to widen them, and it still 
> infeasible to change return types of pre-existing APIs).

test/jdk/java/util/concurrent/tck/ReentrantReadWriteLock20Test.java line 75:

> 73: try {
> 74: latch.await();
> 75: } catch (InterruptedException ie) {}

Something isn't right if any of these virtual threads are interrupted so I 
think failure needs to be set if there is any exception in the runnable.

test/jdk/java/util/concurrent/tck/ReentrantReadWriteLock20Test.java line 94:

> 92: next.join();
> 93: } catch (InterruptedException ie) {
> 94: }

I don't think we should swallow InterruptedException here. The only reason that 
the main thread will be interrupted here is if jtreg is trying to timeout the 
test, so I think it should throw when that happens.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24261#discussion_r2024466317
PR Review Comment: https://git.openjdk.org/jdk/pull/24261#discussion_r2024467711

RFR: 8350462: MethodTypeForm.LF_INTERPRET can cache the MemberName instead

[Zihao Lin]

Modify the cache in MethodTypeForm that currently stores the LF_INTERPRET 
Lambda form. Instead of caching the entire LambdaForm, change it to store a 
SoftReference. This will avoid unnecessary memory usage.

-

Commit messages:
 - Merge branch 'openjdk:master' into JDK-8350462
 - 8350462: MethodTypeForm.LF_INTERPRET can cache the MemberName instead

Changes: https://git.openjdk.org/jdk/pull/24468/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24468&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8350462
  Stats: 31 lines in 2 files changed: 17 ins; 9 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/24468.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24468/head:pull/24468

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

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v6]

[Per Minborg]

On Sun, 16 Mar 2025 00:34:45 GMT, John R Rose  wrote:

> Comments on visual noise and side effects in `toString`.
> 
> `renderWrapped` is clever for a single stable value, but it makes for a very 
> noisy display string, with confusing doubly-nested `[]`, for composite stable 
> values. I'm talking about `StableFunction` mainly, I guess.
> 
> I suggest omitting the inner `[]` for such composites. A simple boolean on 
> `renderWrapped` will do that trick. In that case, `renderWrapped` has the job 
> of either presenting a fixed (recognizable) sentinel string, or else 
> forwards, without further editorial comment, to the `toString` of the 
> contained value.

The `toString()` for `StableValue` is inspired by `Optional` which works in the 
same way by adding `[ ]` around the contents. Any more thought in the reviewer 
community on how we should handle this?

-

PR Comment: https://git.openjdk.org/jdk/pull/23972#issuecomment-2743849237

Re: RFR: 8352064: AIX: now also able to build static-jdk image with a statically linked launcher [v3]

[Joachim Kern]

On Thu, 20 Mar 2025 14:30:58 GMT, Joachim Kern  wrote:

>> After "JDK-8339480: Build static-jdk image with a statically linked 
>> launcher" AIX was not able to build the new target. Therefore with 
>> "JDK-8345590 AIX 'make all' fails after JDK-8339480" the new target was 
>> disabled again.
>> 
>> Now with this change we can enable the statically linked launcher target 
>> again.
>> There are 3 things to do.
>> 1.   Modify `dladdr()`. Because this API does not exist on AIX it is 
>> implemented based on the `loadquery()` API. Unfortunately, this API does 
>> only return the name of the executable, but not its path. Beforehand this 
>> was no problem, because we asked for a loaded library, for which the API 
>> provides the path. But now we are asking for the executable itself.
>> 2.   `dladdr()` is differently implemented three times in the openjdk code. 
>> In the static case I supressed now the usage of the additional modules 
>> containing version two and three. I know this shouldn't be the final 
>> version. Magnus mentioned that they have discussed from time to time to have 
>> a "basic JDK utility lib" that can be shared between hotspot and the JDK 
>> libraries. I think this is a good idea for the future, but far beyond the 
>> scope of this PR. The second best thing Magnus proposed is to only have the 
>> `dladdr()` functionality in Hotspot and then export it. Let's see how the 
>> community decides.
>> 3.   Because we lack a linker flag like `whole-archive`, I had to force the 
>> export of all symbols by creating export files containing all symbols of the 
>> static libs. I introduced the new rule for the export file generation as 
>> "raw" make recipes. Magnus claimed to use the `SetupExecute`. Unfortunately 
>> I was not able to make it function. So I still have my "raw" solution in 
>> place, but my last try with `SetupExecute` as comment beneath. Help is 
>> welcome.
>
> Joachim Kern has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   added needed include

I found the problem:
`include Execute.gmk`
was missing. Now in the same directory beside the three files e.g.

libjli.a
BUILD_LIBJLI_run_ar.cmdline
BUILD_LIBJLI_run_ar

I also get 

libjli.a.exp
_generate_export_list_libjli.a_exec.cmdline
_generate_export_list_libjli.a_exec

So I think point 3 of the PR description is solved, although I find the name 
`_generate_export_list_libjli.a_exec` a little bit to long.

The next point to work on is No. 2 of the PR description (3 `dladdr()` 
implementations spread over the code). Who can help me out with a review and a 
tip in which direction I should go?

-

PR Comment: https://git.openjdk.org/jdk/pull/24062#issuecomment-2740683211

Re: RFR: 8298783: java/lang/ref/FinalizerHistogramTest.java failed with "RuntimeException: MyObject is not found in test output" [v4]

[Brent Christian]

On Fri, 28 Mar 2025 09:34:58 GMT, Jaikiran Pai  wrote:

>> Brent Christian has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   convert to WeakRefs, use a RefQ, print ForceGC results
>
> test/jdk/java/lang/ref/FinalizerHistogramTest.java line 131:
> 
>> 129: volatile boolean ref1Cleared = false;
>> 130: volatile boolean ref2Cleared = false;
>> 131: Thread thread;
> 
> Nit: This need not be a field in the class and instead can be local to the 
> constructor where the Thread is started. But it's OK to leave it like this if 
> you prefer to.

I feel better about maintaining a strong reference (`refQForTwo -> thread`) to 
the `Thread`.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24143#discussion_r2021665867

RFR: 8346948: Update CLDR to Version 47.0

[Naoto Sato]

Upgrading the CLDR to version 47.0. A corresponding CSR has also been drafted.

-

Commit messages:
 - Updated the CLDR chart
 - .md files update
 - TestUnicodeExtension version
 - LikelySubtagLocalesTest
 - release-47
 - CLDR-17484: English day periods are wrong
 - release-47-beta2
 - release-47-beta1
 - CLDR-18268: Add timeData for GS (#4319)
 - release-47-alpha2
 - ... and 4 more: https://git.openjdk.org/jdk/compare/4a02de82...3606a05e

Changes: https://git.openjdk.org/jdk/pull/24120/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24120&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8346948
  Stats: 2731 lines in 1109 files changed: 931 ins; 261 del; 1539 mod
  Patch: https://git.openjdk.org/jdk/pull/24120.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24120/head:pull/24120

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

Re: RFR: 8352064: AIX: now also able to build static-jdk image with a statically linked launcher [v3]

[Magnus Ihse Bursie]

On Thu, 20 Mar 2025 14:30:58 GMT, Joachim Kern  wrote:

>> After "JDK-8339480: Build static-jdk image with a statically linked 
>> launcher" AIX was not able to build the new target. Therefore with 
>> "JDK-8345590 AIX 'make all' fails after JDK-8339480" the new target was 
>> disabled again.
>> 
>> Now with this change we can enable the statically linked launcher target 
>> again.
>> There are 3 things to do.
>> 1.   Modify `dladdr()`. Because this API does not exist on AIX it is 
>> implemented based on the `loadquery()` API. Unfortunately, this API does 
>> only return the name of the executable, but not its path. Beforehand this 
>> was no problem, because we asked for a loaded library, for which the API 
>> provides the path. But now we are asking for the executable itself.
>> 2.   `dladdr()` is differently implemented three times in the openjdk code. 
>> In the static case I supressed now the usage of the additional modules 
>> containing version two and three. I know this shouldn't be the final 
>> version. Magnus mentioned that they have discussed from time to time to have 
>> a "basic JDK utility lib" that can be shared between hotspot and the JDK 
>> libraries. I think this is a good idea for the future, but far beyond the 
>> scope of this PR. The second best thing Magnus proposed is to only have the 
>> `dladdr()` functionality in Hotspot and then export it. Let's see how the 
>> community decides.
>> 3.   Because we lack a linker flag like `whole-archive`, I had to force the 
>> export of all symbols by creating export files containing all symbols of the 
>> static libs. I introduced the new rule for the export file generation as 
>> "raw" make recipes. Magnus claimed to use the `SetupExecute`. Unfortunately 
>> I was not able to make it function. So I still have my "raw" solution in 
>> place, but my last try with `SetupExecute` as comment beneath. Help is 
>> welcome.
>
> Joachim Kern has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   added needed include

Regaring point 2: the current solution works, and is acceptable, I think. The 
two excluded files only contain the dladdr() implementation, and this is 
implemented in hotspot, so no functionality is lost. Compacting this into just 
a single implementation is a separate issue, and need not be resolved now.

Regarding the name of `_generate_export_list_libjli.a_exec`: this is generated 
from the name of the SetupExecute rule, `generate_export_list_$(lib)`. I don't 
mind the name, it is clear. I don't see any particular point in keeping the 
name short, but if it bothers you, shorten the name of the rule. Try keeping it 
clear what it does, though.

-

PR Comment: https://git.openjdk.org/jdk/pull/24062#issuecomment-2742949097

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v25]

[Maurizio Cimadamore]

On Wed, 2 Apr 2025 11:43:40 GMT, Maurizio Cimadamore  
wrote:

>> I didn't mean to say "this is not true, so use a `static final` instead". 
>> What I meant was more "let's not dive into VM-specific details now" (see my 
>> other comment on the topic), and just focus on semantics instead. Then let's 
>> put all the constant-folding related stuff somewhere else. I liked using 
>> non-static final fields!
>
> Basically: drop all references to "performance" and "constant folding" 
> everywhere. And then introduce a section dedicated to that topic which talks 
> a bit about how the content of a SV is trusted not to change by the JVM, 
> which enables constant-folding optimizations.

The reason I'm suggesting this is that what's constant-folded is kind of a 
moving target, so we don't want to say/promise too much. For now, we can say 
that if the SV, or the stable function/collection is stored in a static/final 
field then its content will be constant-folded. But we know that there's other 
stuff in the pipeline (e.g. [this JEP](https://openjdk.org/jeps/8349536)) which 
will affect this picture, and enable for more constant-folding when accessing 
SV and stable functions/collections.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2024659484

Re: RFR: 8327858: Improve spliterator and forEach for single-element immutable collections [v4]

[Chen Liang]

On Fri, 11 Oct 2024 15:49:33 GMT, Chen Liang  wrote:

>> Please review this patch that:
>> 1. Implemented `forEach` to optimize for 1 or 2 element collections.
>> 2. Implemented `spliterator` to optimize for a single element.
>> 
>> The default implementations for multiple-element immutable collections are 
>> fine as-is, specializing implementation doesn't provide much benefit.
>
> Chen Liang has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 18 commits:
> 
>  - s'marks requests
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> feature/imm-coll-stream
>  - Add test to ensure reproducible iteration order
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> feature/imm-coll-stream
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> feature/imm-coll-stream
>  - Use the improved form in forEach
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> feature/imm-coll-stream
>  - Null checks should probably be in the beginning...
>  - mark implicit null checks
>  - Merge branch 'master' of https://github.com/openjdk/jdk into 
> feature/imm-coll-stream
>  - ... and 8 more: https://git.openjdk.org/jdk/compare/7276a1be...4f1f4f1b

Keep alive... I wonder if this patch that tries to improve functional 
programming access to immutable small collections has any problem that it lacks 
a review.

-

PR Comment: https://git.openjdk.org/jdk/pull/15834#issuecomment-2767713089

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v27]

[Maurizio Cimadamore]

On Thu, 3 Apr 2025 12:14:52 GMT, Maurizio Cimadamore  
wrote:

>> That was intentional as the list is always `0..size` whereas a map can use 
>> arbitrary inputs. Is there a better way to illustrate this than what we have 
>> now?
>
> I guess my "beef" is that the examples for int function and function are 
> similar -- they use same class name. Reader might expect (as I did) that they 
> do the same thing, just using a different backing storage. But they don't -- 
> not only is the implementation tweaked to use function instead of int 
> function -- also what is cached changes subtly. I'm afraid this will be 
> difficult to follow for readers. If you want to highlight that the "function" 
> example has more freedom, I think we'd be better off with using another 
> example. For instance:
> 
> * use square(int)->int to show off int function/list
> * use square_root(int)->int to show off function/map

(e.g. stable int function is good for _total functions_ -- stable function is 
good for _partial functions_)

-

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2026878559

Re: RFR: 8352044: Add --with-import-jvms to configure

[Alan Bateman]

On Wed, 19 Mar 2025 12:45:23 GMT, Erik Joelsson  wrote:

> Just to make sure I'm understanding you correctly. Are you proposing that 
> instead of building a single JDK distribution with multiple JVMs, they would 
> build a separate JDK for each alternative JVM configuration and publish 
> java.base.jmod from each of them for end users to pick from when generating 
> their run-images?

I think there is a compelling case for having a release + fastdebug build of a 
server libjvm in the same run-time image. I'm dubious about server + minimal in 
the same run-time image, it's just too weird unless users of minimal VM are 
also running with `--limit-modules` to use a reduced set of modules before 
running `jlink` to create the target run-time image for the small environment.

For embedded environments  then it could be compelling to have the the packaged 
modules (JMOD files, and not just java.base) published for several 
OS/architectures. There's a connection to cross building here. Once you have 
the packaged modules then someone could run `jlink` (on a main-stream platform) 
to produce the run-time containing the small set of modules that are 
appropriate for the target environment.

-

PR Comment: https://git.openjdk.org/jdk/pull/24063#issuecomment-2739578163

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v33]

[Per Minborg]

On Wed, 2 Apr 2025 14:04:52 GMT, Alan Bateman  wrote:

>> Per Minborg has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Add info that Map#values and Map#entrySet are stable
>
> src/java.base/share/classes/java/lang/StableValue.java line 649:
> 
>> 647:  * be thrown.
>> 648:  *
>> 649:  * @param inputs   the set of (non-null) allowed input values
> 
> You might get asked what the behavior is when the inputs set contains the 
> null element.

Nice catch. The docs only state that if any _parameter_ is `null`, an NPE is 
thrown. I've fixed this now.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2026455401

Re: RFR: 8352016: Improve java/lang/RuntimeTests/RuntimeExitLogTest.java

[Mikhail Yankelevich]

On Fri, 14 Mar 2025 09:17:06 GMT, KIRIYAMA Takuya  wrote:

> The current test program for the logging feature added in JDK-8301627 does 
> not fully check some important cases.
> 
> Issue Details:
> The test does not properly check cases where logging might not happen due to 
> different logging levels. (e.g. ALL, TRACE, WARNING, etc.)
> The check for the logged stack trace is not enough, as it does not confirm 
> enough details in the output.
> 
> Fix Details:
> Added more test cases to check behavior under different logging levels.
> Improved the stack trace check by verifying more details in the logged output.
> These changes make the test more complete and ensure that the logging feature 
> works as expected.
> Also, any existing test cases prior to this pull request are retained.
> 
> The test was verified in the following OS environments, and it passed 
> successfully in both environments.
> - Windows Server 2022 Standard 21H2
> - Red Hat Enterprise Linux release 9.2 (Plow)
> 
> Could you please review this fix?

Looks good, just a minor comment. Thank you!

test/jdk/java/lang/RuntimeTests/RuntimeExitLogTest.java line 52:

> 50: private static final String TEST_JDK = System.getProperty("test.jdk");
> 51: private static final String TEST_SRC = System.getProperty("test.src");
> 52: private static final String NL = System.getProperty("line.separator");

Wouldn't it be easier to just use `System.lineSeparator();` here, what do you 
think? 

Also I'd personally rename it to `NEW_LINE` or `SEPARATOR`, as it took me a few 
read throughs to understand what does NL stand for, but thats minor.

test/jdk/java/lang/RuntimeTests/RuntimeExitLogTest.java line 53:

> 51: private static final String TEST_SRC = System.getProperty("test.src");
> 52: private static final String NL = System.getProperty("line.separator");
> 53: private static Object HOLD_LOGGER;

Nitpick: This is not used, could you please remove if there will be another 
revision.

test/jdk/java/lang/RuntimeTests/RuntimeExitLogTest.java line 167:

> 165: List lines = reader.lines().toList();
> 166: boolean match = (expectMessage.isEmpty())
> 167: ? lines.size() == 0

Nitpick: `? lines.isEmpty()` would be easier to read imo, but it's fine as it 
is.

test/jdk/java/lang/RuntimeTests/RuntimeExitLogTest.java line 174:

> 172: System.err.println(expectMessage.replaceAll("\\n", 
> NL));
> 173: System.err.println(" Actual output begin");
> 174: lines.forEach(l -> System.err.println(l));

Nitpick: `lines.forEach(System.err::println);` would be easier to read in my 
opinion, but it's fine as it is.

-

PR Review: https://git.openjdk.org/jdk/pull/24050#pullrequestreview-2694348466
PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2000948118
PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2000943776
PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2000950379
PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2000949810

Re: RFR: 8352642: Set zipinfo-time=false when constructing zipfs FileSystem in com.sun.tools.javac.file.JavacFileManager$ArchiveContainer for better performance

[Jaikiran Pai]

On Sun, 23 Mar 2025 12:38:04 GMT, Jason Zaugg  wrote:

> 8352642: Set zipinfo-time=false when constructing zipfs FileSystem in 
> com.sun.tools.javac.file.JavacFileManager$ArchiveContainer for better 
> performance

I'm running some tests with this change. I'll approve it once that completes.

-

PR Comment: https://git.openjdk.org/jdk/pull/24176#issuecomment-2748502690

Re: RFR: 8352088: Call of com.sun.jdi.ThreadReference.threadGroups() can lock up target VM [v7]

[Chris Plummer]

On Tue, 1 Apr 2025 20:00:22 GMT, Chris Plummer  wrote:

>> Calling ThreadGroupReference.groups() from an event handler can cause a 
>> deadlock. Details in first comment. Tested with :jdk_lang on all supported 
>> platforms and tier1, tier2, tier3, and tier5 svc testing.
>
> Chris Plummer has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   minor comment update

Thanks reviews Sergei, Jai, and Alan

-

PR Comment: https://git.openjdk.org/jdk/pull/24236#issuecomment-2773202187

Re: RFR: 8352016: Improve java/lang/RuntimeTests/RuntimeExitLogTest.java [v2]

[Jaikiran Pai]

On Fri, 21 Mar 2025 09:05:27 GMT, KIRIYAMA Takuya  wrote:

>> The current test program for the logging feature added in JDK-8301627 does 
>> not fully check some important cases.
>> 
>> Issue Details:
>> The test does not properly check cases where logging might not happen due to 
>> different logging levels. (e.g. ALL, TRACE, WARNING, etc.)
>> The check for the logged stack trace is not enough, as it does not confirm 
>> enough details in the output.
>> 
>> Fix Details:
>> Added more test cases to check behavior under different logging levels.
>> Improved the stack trace check by verifying more details in the logged 
>> output.
>> These changes make the test more complete and ensure that the logging 
>> feature works as expected.
>> Also, any existing test cases prior to this pull request are retained.
>> 
>> The test was verified in the following OS environments, and it passed 
>> successfully in both environments.
>> - Windows Server 2022 Standard 21H2
>> - Red Hat Enterprise Linux release 9.2 (Plow)
>> 
>> Could you please review this fix?
>
> KIRIYAMA Takuya has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8352016: Improve java/lang/RuntimeTests/RuntimeExitLogTest.java

test/jdk/java/lang/RuntimeTests/RuntimeExitLogTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.

Hello @tkiriyama, this should be `2023, 2025, `.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2014552843

Re: RFR: 8352906: stdout/err.encoding on Windows set by incorrect Win32 call [v3]

[Alan Bateman]

On Thu, 27 Mar 2025 16:03:08 GMT, Naoto Sato  wrote:

>> Those system property values on Windows were derived from Windows' 
>> `GetConsoleCP()` call, but they should have been taken from 
>> `GetConsoleOutputCP()`. Replacing the incorrect call with the correct one 
>> won't change any behavior, as both calls return the same value by default 
>> (`GetOEMCP()`). However, it is still the right thing to do.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Handled the error case

Marked as reviewed by alanb (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/24263#pullrequestreview-2722540034

RFR: 8353795: Add Writer.of(StringBuilder)

[Markus KARG]

This Pull Requests proposes an implementation for 
[JDK-8353795](https://bugs.openjdk.org/browse/JDK-8353795): Adding the new 
method `public static Writer Writer.of(StringBuilder)`, providing a 
non-synchronized Writer implementation optimized for writing into 
`StringBuilder`.

A basic test is provided to proof that the new `Writer` behaves as expected. 
For comparison, the same test is also run against `StringWriter`.

-

Commit messages:
 - Removed unused import
 - Limited to Writer.of(StringBuilder)
 - Must not change implementation of StringWriter as getBuffer would not 
reflect current content of internally used Writer
 - Fixed typo
 - Do not wrap Writer, but return it as-is
 - Forward flush() and close() for Flushable and Closeable
 - Draft: Test for Writer.of(Appendable)
 - Writer.of(Appendable)

Changes: https://git.openjdk.org/jdk/pull/24469/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24469&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8353795
  Stats: 379 lines in 4 files changed: 376 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/24469.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24469/head:pull/24469

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

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v33]

[Per Minborg]

> Implement JEP 502.
> 
> The PR passes tier1-tier3 tests.

Per Minborg has updated the pull request incrementally with one additional 
commit since the last revision:

  Add info that Map#values and Map#entrySet are stable

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/23972/files
  - new: https://git.openjdk.org/jdk/pull/23972/files/b8d52436..5dbcd4d0

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=23972&range=32
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23972&range=31-32

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

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

Re: RFR: 8352773: JVMTI should disable events during java upcalls

[Chris Plummer]

On Tue, 25 Mar 2025 20:36:28 GMT, Chris Plummer  wrote:

> Calling ThreadGroupReference.groups() from an event handler can cause a 
> deadlock. Details in first comment. Tested with :jdk_lang on all supported 
> platforms and tier1, tier2, tier3, and tier5 svc testing.

The reason is because this JDI API eventually ends up with JVMTI doing a java 
upcall to ThreadGroup.subgroupsAsArray(), which can trigger class loading the 
first time it is called. Thjis results in a ClassPrepareEvent that the debugger 
will get, but not process because its event handler thread is blocked on the 
ThreadGroupReference.groups(), so we have a deadlock.

The workaround is to make ThreadGroupReference.groups() not trigger any class 
loading. The fix is subtle. Before the fix, the java stack trace at the time of 
the ClasPrepareEvent looks like:

java.util.Arrays.copyOf(java.lang.Object[], int, java.lang.Class) bci:18 
line:3513
java.util.ArrayList.toArray(java.lang.Object[]) bci:21 line:401
java.lang.ThreadGroup.subgroupsAsArray() bci:10 line:751

This is ThreadGroup.subgroupsAsArray:

private ThreadGroup[] subgroupsAsArray() {
List groups = synchronizedSubgroups();
return groups.toArray(new ThreadGroup[0]);
}

This is ArrayList.toArray():

public  T[] toArray(T[] a) {
if (a.length < size)
// Make a new array of a's runtime type, but my contents:
return (T[]) Arrays.copyOf(elementData, size, a.getClass());  <--- Line 
401
System.arraycopy(elementData, 0, a, 0, size);
if (a.length > size)
a[size] = null;
return a;
}

And this is Arrays.copyOf():

public static  T[] copyOf(U[] original, int newLength, Class newType) {
@SuppressWarnings("unchecked")
T[] copy = ((Object)newType == (Object)Object[].class)
? (T[]) new Object[newLength]
: (T[]) Array.newInstance(newType.getComponentType(), newLength);
System.arraycopy(original, 0, copy, 0,
 Math.min(original.length, newLength));
return copy;
}

Line 3513 is actually the return statement, so this seems incorrect of by a 
bit. Adding some tracing of ClassPrepareEvents shows that we are currently 
handling the following:

cbClassPrepare: java.lang.reflect.Array

So it looks like we took the Array.newInstance() path, which triggered class 
loading of java.lang.reflect.Array. After the fix we never end up in 
Arrays.copyOf(). Instead ArrayList.toArray() calls System.arraycopy() directly, 
avoiding any class loading..

-

PR Comment: https://git.openjdk.org/jdk/pull/24236#issuecomment-2752702624

Re: RFR: 8352419: Test tools/jpackage/share/ErrorTest.java#id0 and #id1 fail [v2]

[Arno Zeller]

On Tue, 1 Apr 2025 19:00:56 GMT, Alexey Semenyuk  wrote:

>> Make tools/jpackage/share/ErrorTest.java test safely handle the case when 
>> native bundling is unavailable on the test host.
>> 
>> Additionally:
>>  - If native bundling is unavailable on the test host, PackageTest will 
>> throw `jtreg.SkippedException` and mark the test as skipped instead of 
>> silently doing nothing. This should mark a few jpackage tests "skipped" in 
>> tests runs on Alpine Linux that doesn't support either .deb or .rpm 
>> packaging.
>>  - Get rid of the dependency on `jtreg.SkippedException` from `/jdk/test` 
>> lib as this exception can be thrown from almost every jpackage test because 
>> of the changes in PackageTest. Instead embed `jtreg.SkippedException` 
>> classfile in TKit.java source and load it from there. This is a less 
>> intrusive alternative to adding 
>> 
>>  * @library /test/lib
>>  * @build jtreg.SkippedException
>> 
>> in every jtreg test declaration
>
> Alexey Semenyuk 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:
> 
>  - Merge branch 'master' into JDK-8352419
>  - Fix failure in PackageTestTest when native packaging is not available
>  - bash ./bin/blessed-modifier-order.sh test/jdk/tools/jpackage/
>  - 8352419: Test tools/jpackage/share/ErrorTest.java#id0 and #id1 fail
>  - Add PackageType.throwSkippedExceptionIfNativePackagingUnavailable(). 
> Stable order of platform package types. Make PackageTest fail if native 
> packagers are unavailable.
>  - Get rid of dependency on "/test/lib" tets lib

Test on Windows without WIX was successful - thanks for improving this!

-

PR Comment: https://git.openjdk.org/jdk/pull/24294#issuecomment-2771483413

Re: RFR: 8343110: Add getChars(int, int, char[], int) to CharSequence and CharBuffer [v3]

[Markus KARG]

On Wed, 26 Mar 2025 11:34:34 GMT, Markus KARG  wrote:

>> src/java.base/share/classes/java/lang/CharSequence.java line 307:
>> 
>>> 305: /**
>>> 306:  * Characters are copied from this sequence into the
>>> 307:  * destination character array {@code dst}. The first character to
>> 
>> I think the "Characters are copied ..." sentence goes back to StringBuffer 
>> in JDK 1.0. CS doesn't need to copy this exactly and I think better to say 
>> that it copies chars from this sequence into the given destination array.
>
> Do you like this more: `Copies chars from this sequence into the given 
> destination array.` ?

Thank you, Alan. Fixed in 
https://github.com/openjdk/jdk/pull/21730/commits/a7f56f4ecba4be1a28e551e8a247998db7d7cb79.

>> src/java.base/share/classes/java/lang/CharSequence.java line 335:
>> 
>>> 333:  *
>>> 334:  * @implSpec
>>> 335:  * The default implementation iterates over {@link #charAt(int)}.
>> 
>> This sentence doesn't make sense, did something get deleted?
>
> Do you like this more: `The default implementation invokes {@link 
> #charAt(int)} in a loop.`?

Thank you, Alan. Fixed in 
https://github.com/openjdk/jdk/pull/21730/commits/a5d26c5bc143e37520e5f42bcb0299d4e12784b0.

>> src/java.base/share/classes/java/nio/X-Buffer.java.template line 1900:
>> 
>>> 1898: 
>>> 1899: /**
>>> 1900:  * {@inheritDoc}
>> 
>> The method description here will need to start with  "Absolute bulk get 
>> method". This is important because CB defines both "absolute bulk get" and 
>> "relative bulk get methods", it has to be very clear in the API docs.
>> 
>> If the proposal goes again then I think the method description won't be 
>> inherited into CB, instead it will say that it transfers chars from this 
>> buffer into the given destination array.
>
> IIUC then you want me to replace `{@inheritedDoc}` by `This absolute bulk get 
> method transfers chars from this buffer into the given destination array.`?

Thank you, Alan. Fixed in 
https://github.com/openjdk/jdk/pull/21730/commits/29e1521fa25ba88dbbe4af077888044666f790f6.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/21730#discussion_r2019766905
PR Review Comment: https://git.openjdk.org/jdk/pull/21730#discussion_r2019766966
PR Review Comment: https://git.openjdk.org/jdk/pull/21730#discussion_r2019766823

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v33]

[Maurizio Cimadamore]

On Thu, 3 Apr 2025 08:22:47 GMT, Per Minborg  wrote:

> "A stable value is a holder of content that can be lazily computed at most 
> once"

I'd replace `computed` with `set` (as that's the term we use in the API).

-

PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2026860656

Re: RFR: 8298783: java/lang/ref/FinalizerHistogramTest.java failed with "RuntimeException: MyObject is not found in test output" [v3]

[Brent Christian]

> I propose some cleanups to `FinalizerHistogramTest.java` to hopefully clear 
> up the intermittent failures:
> 
> * run with `othervm`: this test blocks the (global) finalizer thread, and 
> also requires the (global) finalizer thread to enter the test's `finalize()` 
> method
> * The test uses `volatile` ints, but sets them based on their current value, 
> which is not reliable; convert to `AtomicInteger`
> * use `PhantomReference`s to ensure that at least two `MyObject`s have become 
> unreachable. If one is stuck in `finalize()`, at least one is still waiting 
> to be finalized and should show up in the histogram.

Brent Christian has updated the pull request incrementally with one additional 
commit since the last revision:

  'return' not needed in lambda

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/24143/files
  - new: https://git.openjdk.org/jdk/pull/24143/files/efbe07be..78f334a6

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=24143&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24143&range=01-02

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

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

Integrated: 8319447: Improve performance of delayed task handling

[Doug Lea]

On Wed, 19 Feb 2025 16:07:56 GMT, Doug Lea  wrote:

> (Copied from https://bugs.openjdk.org/browse/JDK-8319447)
> 
> The problems addressed by this CR/PR are that ScheduledThreadPoolExecutor is 
> both ill-suited for many (if not most) of its applications, and is a 
> performance bottleneck (as seen especially in Loom and CompletableFuture 
> usages). After considering many options over the years, the approach taken 
> here is to connect (lazily, only if used) a form of ScheduledExecutorService 
> (DelayScheduler) to any ForkJoinPool (including the commonPool), which can 
> then use more efficient and scalable techniques to request and trigger 
> delayed actions, periodic actions, and cancellations, as well as coordinate 
> shutdown and termination mechanics (see the internal documentation in 
> DelayScheduler.java for algotihmic details). This speeds up some Loom 
> operations by almost an order of magnitude (and similarly for 
> CompletableFuture). Further incremental improvements may be possible, but 
> delay scheduling overhead is now unlikely to be a common performance concern.
> 
> We also introduce method submitWithTimeout to schedule a timeout that cancels 
> or otherwise completes a submitted task that takes too long. Support for this 
> very common usage was missing from the ScheduledExecutorService API, and 
> workarounds that users have tried are wasteful, often leaky, and error-prone. 
> This cannot be added to the ScheduledExecutorService interface because it 
> relies on ForkJoinTask methods (such as completeExceptionally) to be 
> available in user-supplied timeout actions. The need to allow a pluggable 
> handler reflects experience with the similar CompletableFuture.orTimeout, 
> which users have found not to be flexible enough, so might be subject of 
> future improvements.
> 
> A DelayScheduler is optionally (on first use of a scheduling method) 
> constructed and started as part of a ForkJoinPool, not any other kind of 
> ExecutorService. It doesn't make sense to do so with the other j.u.c pool 
> implementation ThreadPoolExecutor. ScheduledThreadPoolExecutor already 
> extends it in incompatible ways (which is why we can't just improve or 
> replace STPE internals). However, as discussed in internal documentation, the 
> implementation isolates calls and callbacks in a way that could be extracted 
> out into (package-private) interfaces if another j.u.c pool type is 
> introduced.
> 
> Only one of the policy controls in ScheduledThreadPoolExecutor applies to 
> ForkJoinPools with DelaySchedulers: new method cancelDelayedTasksOnShutdown 
> controls whether quiescent shutdown should wait for dela...

This pull request has now been integrated.

Changeset: 8b0602db
Author:Doug Lea 
URL:   
https://git.openjdk.org/jdk/commit/8b0602dbed2f7ced190ec81753defab8a4bc316d
Stats: 1969 lines in 12 files changed: 1596 ins; 204 del; 169 mod

8319447: Improve performance of delayed task handling

Reviewed-by: vklang, alanb

-

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

Re: RFR: 8347712: IllegalStateException on multithreaded ZipFile access with non-UTF8 charset [v5]

[Jaikiran Pai]

> Can I please get a review of this change which proposes to fix an issue 
> `java.util.zip.ZipFile` which would cause failures when multiple instances of 
> `ZipFile` using non-UTF8 `Charset` were operating against the same underlying 
> ZIP file? This addresses https://bugs.openjdk.org/browse/JDK-8347712.
> 
> ZIP file specification allows for ZIP entries to mark a `UTF-8` flag to 
> indicate that the entry name and comment are encoded using UTF8. A 
> `java.util.zip.ZipFile` can be constructed by passing it a `Charset`. This 
> `Charset` (which defaults to UTF-8) gets used for decoding entry names and 
> comments for non-UTF8 entries.
> 
> The internal implementation of `ZipFile` uses a `ZipCoder` (backed by 
> `java.nio.charset.CharsetEncoder/CharsetDecoder` instance) for the given 
> `Charset`. Except for UTF8 `ZipCoder`, other `ZipCoder`s are not thread safe.
> 
> The internal implementation of `ZipFile` maintains a cache of 
> `ZipFile$Source`. A `Source` corresponds to the underlying ZIP file and 
> during construction, uses a `ZipCoder` for parsing the ZIP entries and once 
> constructed holds on to the parsed ZIP structure. Multiple instances of a 
> `ZipFile` which all correspond to the same ZIP file on the filesystem, share 
> a single instance of `Source` (after the `Source` has been constructed and 
> cached). Although `ZipFile` instances aren't expected to be thread-safe, the 
> fact that multiple different instances of `ZipFile` could be sharing the same 
> instance of `Source` in concurrent threads, mandates that the `Source` must 
> be thread-safe.
> 
> In Java 15, we did a performance optimization through 
> https://bugs.openjdk.org/browse/JDK-8243469. As part of that change, we 
> started holding on to the `ZipCoder` instance (corresponding to the `Charset` 
> provided during `ZipFile` construction) in the `Source`. This stored 
> `ZipCoder` was then used for `ZipFile` operations when working with the ZIP 
> entries. As noted previously, any non-UTF8 `ZipCoder` is not thread-safe and 
> as a result, any usages of `ZipCoder` in the `Source` makes `Source` not 
> thread-safe too. That effectively violates the requirement that `Source` must 
> be thread-safe to allow for its usage in multiple different `ZipFile` 
> instances concurrently. This then causes `ZipFile` usages to fail in 
> unexpected ways like the one shown in the linked 
> https://bugs.openjdk.org/browse/JDK-8347712.
> 
> The commit in this PR addresses the issue by not maintaining `ZipCoder` as a 
> instance field of `Source`. Instead the `ZipCoder` is now maintained in the 
> `ZipFile`,...

Jaikiran Pai 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 nine additional commits since 
the last revision:

 - Eirik's suggestion - update test method comment
 - rename entryNameCharset to charset in the test
 - Eirik's suggestion - update test to even call ZipFile.getEntry()
 - Eirik's inputs - replace useUTF8Coder() with zipCoderFor()
 - merge latest from master branch
 - add code comment
 - rename field
 - tiny typo fix in newly introduced documentation
 - 8347712: IllegalStateException on multithreaded ZipFile access with non-UTF8 
charset

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/23986/files
  - new: https://git.openjdk.org/jdk/pull/23986/files/935b04ed..83ac59fc

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=23986&range=04
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23986&range=03-04

  Stats: 33458 lines in 568 files changed: 15587 ins; 14378 del; 3493 mod
  Patch: https://git.openjdk.org/jdk/pull/23986.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/23986/head:pull/23986

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

Re: RFR: 8352489: Relax jspawnhelper version checks to informative

[Alan Bateman]

On Thu, 20 Mar 2025 09:52:02 GMT, Aleksey Shipilev  wrote:

> See the bug for rationale. 
> 
> This goal for this improvement is to be easily backportable, so we can catch 
> up with update releases. As such, it does a few borderline-trivial changes, 
> and _does not_ change the jspawnhelper protocol. So the overwrite of 
> `jspawnhelper` with this new version would be "compatible" in a very weak 
> sense of the word.
> 
> Additional testing:
>  - [x] Linux x86_64 server fastdebug, `java/lang/ProcessBuilder`

Something is very broken if the JDK is being overridden while in use. So I 
don't think I agree that relaxing the check is the right thing to do, it's not 
fixing the real issue.

-

PR Comment: https://git.openjdk.org/jdk/pull/24127#issuecomment-2739830227

Re: JDK-8352891 Performance improvements to ByteArrayOutputStream

[Scott Palmer]

Other than the read-only aspect, is ByteBuffer pretty close to the builder you are looking for? I would think a thin wrapper over a ByteBuffer might be a suitable alternative. Just a quick thought, I haven’t looked at any proposed code.ScottOn Mar 28, 2025, at 6:09 PM, Archie Cobbs  wrote:This is just a drive-by, opinion-saturated comment, so feel free to ignore...I think the missing puzzle piece here is slightly different/more general than "OutputStream into memory". IMHO what's really missing from the standard toolbox provided by the JDK is the byte equivalent of StringBuilder+String. In other words, just as String is a read-only wrapper for a character array (or array range), and StringBuilder is used to build a String, Java lacks a read-only container for byte array (or range), and an associated builder. The "read-only" part is very important - as with String, the internal byte[] array must be incorruptible and for the same reasons.If we had those two things, they would presumably be optimized for performance already, so you would then have the building block needed to create a more efficient version of ByteArrayOutputStream - by simply wrapping an OutputStream around the builder (better yet, maybe the builder already extends OutputStream). I'm guessing adding such a thing has probably been discussed before and rejected. If that's true, I'm not optimistic a less general solution to the same overall problem would be accepted, but if there is interest on the list for reconsidering such a thing I'd support it. Like many folks, I've had to resort to writing my own implementation of just such a thing in the past (and am happy to share it if interested).-ArchieOn Fri, Mar 28, 2025 at 7:06 AM Engebretson, John  wrote:







Hi all!  This message is to discuss the proposal for a public class that is faster/cheaper than ByteArrayOutputStream.  Details are on the ticket [1] so I will only summarize here:
- ByteArrayOutputStream is slower than the provided alternative, and wastes memory bandwidth and allocation.
- The new alternative cannot replace ByteArrayOutputStream because BAOS exposes two implementation-specific fields.
- The problem is broadly present, and different solutions exist in Spring, Tomcat, multiple applications inside my company, and undoubtedly elsewhere.
- There are places within the JDK that will benefit from the improved performance.
 
My goal is to broadly distribute this new datastructure, and the JDK is an obvious place to do it.  My proposal is to publish a new public class, java.io.MemoryOutputStream extends OutputStream.  I acknowledge the difficulties in doing
 so.
 
I welcome any thoughts about the problem, solution, or deployment.  I created a draft PR [2] for discussion or questions about the proposed implementation.
 
1. https://bugs.openjdk.org/browse/JDK-8352891
2. https://github.com/openjdk/jdk/pull/24232



-- Archie L. Cobbs

Re: RFR: 8342869: Errors related to unused code on Windows after 8339120 in awt

[Julian Waters]

On Wed, 23 Oct 2024 05:07:37 GMT, Julian Waters  wrote:

> After 8339120, gcc began catching many different instances of unused code in 
> the Windows specific codebase. Some of these seem to be bugs. I've taken the 
> effort to mark out all the relevant globals and locals that trigger the 
> unused warnings and addressed all of them by commenting out the code as 
> appropriate. I am confident that in many cases this simplistic approach of 
> commenting out code does not fix the underlying issue, and the warning 
> actually found a bug that should be fixed. In these instances, I will be 
> aiming to fix these bugs with help from reviewers, so I recommend anyone 
> reviewing who knows more about the code than I do to see whether there is 
> indeed a bug that needs fixing in a different way than what I did
> 
> build.log on release configuration:
> 
> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/java.desktop/windows/native/libawt/windows/awt_Component.cpp:3560:39:
>  warning: '_VKS_ALT_MASK' defined but not used [-Wunused-const-variable=]
>  3560 | static const UINT _VKS_ALT_MASK = 0x04;
>   |   ^
> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/java.desktop/windows/native/libawt/windows/awt_Font.cpp:
>  In member function 'void CSegTable::MakeTable()':
> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/java.desktop/windows/native/libawt/windows/awt_Font.cpp:1361:14:
>  warning: typedef 'PSUBTABLE' locally defined but not used 
> [-Wunused-local-typedefs]
>  1361 | } SUBTABLE, *PSUBTABLE;
>   |  ^
> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/java.desktop/windows/native/libawt/windows/awt_Font.cpp:
>  In member function 'virtual void CEUDCSegTable::Create(LPCWSTR)':
> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/java.desktop/windows/native/libawt/windows/awt_Font.cpp:1588:10:
>  warning: typedef 'PHEAD' locally defined but not used 
> [-Wunused-local-typedefs]
>  1588 | } HEAD, *PHEAD;
>   |  ^
> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/java.desktop/windows/native/libawt/windows/awt_Font.cpp:1595:11:
>  warning: typedef 'PENTRY' locally defined but not used 
> [-Wunused-local-typedefs]
>  1595 | } ENTRY, *PENTRY;
>   |   ^~
> C:/users/vertig0/downloads/eclipse-committers-2023-12-r-win32-x86_64/workspace/jdk/src/java.desktop/windows/...

Keep open please

-

PR Comment: https://git.openjdk.org/jdk/pull/21655#issuecomment-2732871765

RFR: 8352171: Arrays.hashCode for sub-range of byte array API addition

[Zihao Lin]

Add the java.util.Arrays.hashCode(byte[], int start, int end).

Hi team, I am new here, please give me some guidance. Thank you.

-

Commit messages:
 - 8352171: Arrays.hashCode for sub-range of byte array API addition

Changes: https://git.openjdk.org/jdk/pull/24128/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24128&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8352171
  Stats: 30 lines in 1 file changed: 30 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/24128.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24128/head:pull/24128

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

Re: RFR: 8352289: [macos] Review skipped tests in tools/jpackage/macosx/SigningPackage*

[Alexey Semenyuk]

On Mon, 24 Mar 2025 14:35:50 GMT, Alexey Semenyuk  wrote:

> Follow-up for https://github.com/openjdk/jdk/pull/24087 PR. 
> Revamp signing tests:
>  - Run the tests only if `jpackage.test.MacSignTests` property is set to 
> "run". This property also can be set to "setup" and "teardown" to set up and 
> tear down jpackage signing test environment. See 
> https://github.com/openjdk/jdk/pull/24087 PR.
>  - Moved tests from SigningOptionsTest.java to ErrorTest.java. Altered 
> `JPackageCommand.validateOutput()` to safely handle the case of an empty list 
> of expected strings.
>  - Removed SigningCheck.java. Verification of jpackage signing test 
> environment is performed in `SigningBase.verifySignTestEnvReady()` method 
> using verification API from MacSign class. 
> `SigningBase.verifySignTestEnvReady()` method is called in every signing 
> jtreg test. It is a replacement for `SigningCheck.checkCertificates()` call.
>  - Change signatures of test methods to leverage capabilities of jpackage 
> test lib added in https://github.com/openjdk/jdk/pull/21996 PR.
> 
> Before these changes, if the signing test environment was not set, signing 
> tests exited with "skipped" status. With this change, these tests will not be 
> executed unless `jpackage.test.MacSignTests` property is set to "run". The 
> tests fail if the property is set and the signing test environment is not set 
> correctly. This is a deliberate choice to encourage running these tests only 
> if the signing test environment is configured correctly.

@sashamatveev PTAL

-

PR Comment: https://git.openjdk.org/jdk/pull/24201#issuecomment-2749755409

Re: RFR: 8298783: java/lang/ref/FinalizerHistogramTest.java failed with "RuntimeException: MyObject is not found in test output" [v4]

[Andrey Turbanov]

On Fri, 28 Mar 2025 01:08:42 GMT, Brent Christian  wrote:

>> I propose some cleanups to `FinalizerHistogramTest.java` to hopefully clear 
>> up the intermittent failures:
>> 
>> * run with `othervm`: this test blocks the (global) finalizer thread, and 
>> also requires the (global) finalizer thread to enter the test's `finalize()` 
>> method
>> * The test uses `volatile` ints, but sets them based on their current value, 
>> which is not reliable; convert to `AtomicInteger`
>> * use `PhantomReference`s to ensure that at least two `MyObject`s have 
>> become unreachable. If one is stuck in `finalize()`, at least one is still 
>> waiting to be finalized and should show up in the histogram.
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   convert to WeakRefs, use a RefQ, print ForceGC results

test/jdk/java/lang/ref/FinalizerHistogramTest.java line 70:

> 68: lock.lock();
> 69: refQForTwo = new RefQForTwo(new MyObject(), new MyObject());
> 70: for(int i = 2; i < OBJECTS_COUNT; ++i) {

Suggestion:

for (int i = 2; i < OBJECTS_COUNT; ++i) {

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24143#discussion_r2029954070

Re: RFR: 8352064: AIX: now also able to build static-jdk image with a statically linked launcher

[Joachim Kern]

On Fri, 14 Mar 2025 15:41:56 GMT, Joachim Kern  wrote:

> After "JDK-8339480: Build static-jdk image with a statically linked launcher" 
> AIX was not able to build the new target. Therefore with "JDK-8345590 AIX 
> 'make all' fails after JDK-8339480" the new target was disabled again.
> 
> Now with this change we can enable the statically linked launcher target 
> again.
> There are 3 things to do.
> 1.Modify `dladdr()`. Because this API does not exist on AIX it is 
> implemented based on the `loadquery()` API. Unfortunately, this API does only 
> return the name of the executable, but not its path. Beforehand this was no 
> problem, because we asked for a loaded library, for which the API provides 
> the path. But now we are asking for the executable itself.
> 2.`dladdr()` is differently implemented three times in the openjdk code. 
> In the static case I supressed now the usage of the additional modules 
> containing version two and three. I know this shouldn't be the final version. 
> Magnus mentioned that they have discussed from time to time to have a "basic 
> JDK utility lib" that can be shared between hotspot and the JDK libraries. I 
> think this is a good idea for the future, but far beyond the scope of this 
> PR. The second best thing Magnus proposed is to only have the `dladdr()` 
> functionality in Hotspot and then export it. Let's see how the community 
> decides.
> 3.Because we lack a linker flag like `whole-archive`, I had to force the 
> export of all symbols by creating export files containing all symbols of the 
> static libs. I introduced the new rule for the export file generation as 
> "raw" make recipes. Magnus claimed to use the `SetupExecute`. Unfortunately I 
> was not able to make it function. So I still have my "raw" solution in place, 
> but my last try with `SetupExecute` as comment beneath. Help is welcome.

I applied all of magnus proposals, but I still get
`gmake[3]: *** No rule to make target 
'/static_libs/jdk/build/aix-ppc64-server-fastdebug/support/native/java.base/libverify/static/libverify.a.exp',
 needed by 
'/static_libs/jdk/build/aix-ppc64-server-fastdebug/support/static-native/launcher/java'.
  Stop.`

-

PR Comment: https://git.openjdk.org/jdk/pull/24062#issuecomment-2733873645

Re: RFR: 8352693: Use a simpler console reader instead of JLine for System.console()

[Jan Lahoda]

On Thu, 27 Mar 2025 18:28:17 GMT, Naoto Sato  wrote:

>> The `java.io.Console` has several backends: a simple on in `java.base`, a 
>> more convenient one in `jdk.internal.le` (with line-reading based on JLine) 
>> and one for JShell.
>> 
>> The backend based on JLine is proving to be a somewhat problematic - JLine 
>> is very powerful, possibly too powerful and complex for the simple task of 
>> editing a line with no completion, no history, no variables, no commands, 
>> etc. As a consequence, there are inevitable sharp edges in this backend.
>> 
>> The idea in this PR is to replace the use of JLine in the `jdk.internal.le` 
>> backend with a simple escape code interpreter, that only handles a handful 
>> of keys/codes (left/right arrow, home, end, delete, backspace, enter), and 
>> ignores the rest. The goal is to have something simple with less surprising 
>> behavior.
>
> src/jdk.internal.le/share/classes/jdk/internal/console/SimpleConsoleReader.java
>  line 69:
> 
>> 67: case 4: break READ; //EOF/Ctrl-D
>> 68: case 127:
>> 69: //backspace:
> 
> Is it `delete`?

I don't think so. Delete is an escape sequence (`\033[3~`).

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24242#discussion_r2021413402

Re: RFR: 8352716: (tz) Update Timezone Data to 2025b

[Naoto Sato]

On Tue, 25 Mar 2025 16:52:28 GMT, Naoto Sato  wrote:

> Incorporating the latest IANA Time Zone Database (2025b). Manually confirmed 
> the newly introduced time zone stays at the same offset (-03) on/after 
> 2025-04-06:
> 
> jshell> 
> ZoneId.of("America/Coyhaique").getRules().getValidOffsets(LocalDateTime.of(2025,
>  4, 6, 0, 0))
> $198 ==> [-03:00]

Thanks for the reviews!

-

PR Comment: https://git.openjdk.org/jdk/pull/24234#issuecomment-2754965142

Re: RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v21]

[Per Minborg]

On Mon, 31 Mar 2025 15:15:31 GMT, Maurizio Cimadamore  
wrote:

> > Here are the latest benchmarks run on an M1 (macOS):
> > ```
> > Benchmark   Mode  Cnt  Score   
> > Error  Units
> > StableFunctionBenchmark.functionavgt   10  4.228 ? 
> > 0.172  ns/op
> > StableFunctionBenchmark.map avgt   10  4.323 ? 
> > 0.289  ns/op
> > StableFunctionBenchmark.staticIntFunction   avgt   10  1.724 ? 
> > 0.121  ns/op
> > StableFunctionBenchmark.staticSMap  avgt   10  1.710 ? 
> > 0.045  ns/op
> > StableFunctionSingleBenchmark.function  avgt   10  4.329 ? 
> > 0.184  ns/op
> > StableFunctionSingleBenchmark.map   avgt   10  4.291 ? 
> > 0.142  ns/op
> > StableFunctionSingleBenchmark.staticIntFunction avgt   10  0.704 ? 
> > 0.022  ns/op
> > StableFunctionSingleBenchmark.staticSMapavgt   10  0.708 ? 
> > 0.027  ns/op
> > StableIntFunctionBenchmark.intFunction  avgt   10  1.558 ? 
> > 0.063  ns/op
> > StableIntFunctionBenchmark.list avgt   10  1.579 ? 
> > 0.141  ns/op
> > StableIntFunctionBenchmark.staticIntFunctionavgt   10  1.044 ? 
> > 0.031  ns/op
> > StableIntFunctionBenchmark.staticList   avgt   10  2.280 ? 
> > 2.013  ns/op
> > StableIntFunctionSingleBenchmark.intFunctionavgt   10  2.333 ? 
> > 0.033  ns/op
> > StableIntFunctionSingleBenchmark.list   avgt   10  2.335 ? 
> > 0.046  ns/op
> > StableIntFunctionSingleBenchmark.staticIntFunction  avgt   10  0.670 ? 
> > 0.022  ns/op
> > StableIntFunctionSingleBenchmark.staticList avgt   10  0.679 ? 
> > 0.021  ns/op
> > StableSupplierBenchmark.stable  avgt   10  1.377 ? 
> > 0.042  ns/op
> > StableSupplierBenchmark.staticStableavgt   10  0.362 ? 
> > 0.077  ns/op
> > StableSupplierBenchmark.staticSupplier  avgt   10  0.338 ? 
> > 0.016  ns/op
> > StableSupplierBenchmark.supplieravgt   10  1.609 ? 
> > 0.042  ns/op
> > StableValueBenchmark.atomic avgt   10  1.357 ? 
> > 0.046  ns/op
> > StableValueBenchmark.dclavgt   10  1.369 ? 
> > 0.058  ns/op
> > StableValueBenchmark.refSupplieravgt   10  0.442 ? 
> > 0.007  ns/op
> > StableValueBenchmark.stable avgt   10  1.522 ? 
> > 0.267  ns/op
> > StableValueBenchmark.stableNull avgt   10  1.237 ? 
> > 0.117  ns/op
> > StableValueBenchmark.staticAtomic   avgt   10  1.220 ? 
> > 0.058  ns/op
> > StableValueBenchmark.staticDcl  avgt   10  0.357 ? 
> > 0.022  ns/op
> > StableValueBenchmark.staticHolder   avgt   10  1.452 ? 
> > 0.205  ns/op
> > StableValueBenchmark.staticRecordHolder avgt   10  0.367 ? 
> > 0.028  ns/op
> > StableValueBenchmark.staticStable   avgt   10  0.365 ? 
> > 0.026  ns/op
> > Finished running test 'micro:java.lang.stable'
> > ```
> 
> This seems an outlier:
> 
> ```
> StableIntFunctionBenchmark.staticList   avgt   10  2.280 ? 2.013  
> ns/op
> ```
> 
> (I also note the high error)
> 
> I believe it could be useful to have one more benchmark showing a 
> `StableValue` holding a `MethodHandle` and do a `get()` + `invokeExact`. I 
> believe that should report more dramatic distinctions when compared to 
> atomic/dcl?

Thanks for "hawk-eying" this discrepancy. There seemed to be some flux when I 
ran the benchmarks (I've used a laptop). Running the benchmarks in a more 
controlled environment revealed there was no difference. Also, rerunning the 
particular benchmark now shows:


StableIntFunctionBenchmark.intFunction avgt   10  2.317 ? 1.252  ns/op
StableIntFunctionBenchmark.listavgt   10  2.303 ? 1.302  ns/op
StableIntFunctionBenchmark.staticIntFunction   avgt   10  1.044 ? 0.036  ns/op
StableIntFunctionBenchmark.staticList  avgt   10  1.052 ? 0.061  ns/op


I will add a `MethodHandle` benchmark. Good suggestion!

-

PR Comment: https://git.openjdk.org/jdk/pull/23972#issuecomment-2766618937

Re: RFR: 8350462: MethodTypeForm.LF_INTERPRET can cache the MemberName instead

[Chen Liang]

On Sat, 5 Apr 2025 14:26:13 GMT, Zihao Lin  wrote:

> Modify the cache in MethodTypeForm that currently stores the LF_INTERPRET 
> Lambda form. Instead of caching the entire LambdaForm, change it to store a 
> SoftReference. This will avoid unnecessary memory usage.

src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java line 71:

> 69: // Indexes into lambdaForms:
> 70: static final int
> 71: LF_INVVIRTUAL  =  0,  // DMH invokeVirtual

You can remove the LF_INTERPRET constant from this list.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/24468#discussion_r2029997791

Integrated: 8352184: Jtreg tests using CommandLineOptionTest.getVMTypeOption() and optionsvalidation.JVMOptionsUtils fail on static JDK

[Jiangli Zhou]

On Sat, 22 Mar 2025 03:46:38 GMT, Jiangli Zhou  wrote:

> Please review following changes, thanks.
> 
> - Add `static` to the vm_info for static JDK. The `-version` output now 
> contains `static` on static JDK, e.g.:
> 
> 
> $ static-jdk/bin/java -version
> openjdk version "25-internal" 2025-09-16
> OpenJDK Runtime Environment (fastdebug build 
> 25-internal-adhoc.jianglizhou.jdk)
> OpenJDK 64-Bit Server VM (fastdebug build 25-internal-adhoc.jianglizhou.jdk, 
> mixed mode, static, sharing)
> 
> $ jdk/bin/java -version
> openjdk version "25-internal" 2025-09-16
> OpenJDK Runtime Environment (fastdebug build 
> 25-internal-adhoc.jianglizhou.jdk)
> OpenJDK 64-Bit Server VM (fastdebug build 25-internal-adhoc.jianglizhou.jdk, 
> mixed mode, sharing)
> 
> 
> Following changes resolve jtreg test failures on static JDK due to 
> '-server|-client|-minimal|-zero' flag added by 
> `CommandLineOptionTest.getVMTypeOption()` or 
> `optionsvalidation.JVMOptionsUtils`. '-server|-client|-minimal|-zero' flags 
> are unrecognized on static JDK (please see 
> https://bugs.openjdk.org/browse/JDK-8350982).
> 
> - Add `jdk.test.lib.Platform.isStatic()`, which checks for `static` in 
> `java.vm.info` system property to determine if current test is running on 
> static JDK. 
> - Change `CommandLineOptionTest` to only call `getVMTypeOption()` on regular 
> JDK (`!Platform.isStatic()`).
> - Change `optionsvalidation.JVMOptionsUtils` to only set VMType to 
> '-server|-client|-minimal' on regular JDK ( `!Platform.isStatic()`.

This pull request has now been integrated.

Changeset: 79824c34
Author:Jiangli Zhou 
URL:   
https://git.openjdk.org/jdk/commit/79824c344ee36bcf9f3434ccb3b44d2d24defc5c
Stats: 53 lines in 5 files changed: 29 ins; 8 del; 16 mod

8352184: Jtreg tests using CommandLineOptionTest.getVMTypeOption() and 
optionsvalidation.JVMOptionsUtils fail on static JDK

Reviewed-by: dholmes, shade

-

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

Re: RFR: 8352693: Use a simpler console reader instead of JLine for System.console() [v2]

[Jan Lahoda]

> The `java.io.Console` has several backends: a simple on in `java.base`, a 
> more convenient one in `jdk.internal.le` (with line-reading based on JLine) 
> and one for JShell.
> 
> The backend based on JLine is proving to be a somewhat problematic - JLine is 
> very powerful, possibly too powerful and complex for the simple task of 
> editing a line with no completion, no history, no variables, no commands, 
> etc. As a consequence, there are inevitable sharp edges in this backend.
> 
> The idea in this PR is to replace the use of JLine in the `jdk.internal.le` 
> backend with a simple escape code interpreter, that only handles a handful of 
> keys/codes (left/right arrow, home, end, delete, backspace, enter), and 
> ignores the rest. The goal is to have something simple with less surprising 
> behavior.

Jan Lahoda has updated the pull request incrementally with three additional 
commits since the last revision:

 - Using control characters to get backspace control character.
 - If there's no native library available, fall back to the standard provider.
 - Reflecting review feedback.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/24242/files
  - new: https://git.openjdk.org/jdk/pull/24242/files/77c41e46..d5176d68

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=24242&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24242&range=00-01

  Stats: 152 lines in 11 files changed: 114 ins; 13 del; 25 mod
  Patch: https://git.openjdk.org/jdk/pull/24242.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24242/head:pull/24242

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