Integrated: 8335290: Rename ClassFile::transform to ClassFile::transformClass
On Fri, 28 Jun 2024 22:01:33 GMT, Chen Liang wrote: > `ClassFile::transform` was initially `ClassModel::transform`, which > transforms the receiver class model to a new class byte array. This > functionality was in parallel with `ClassBuilder::transform`, which accepts a > `ClassModel` and a `ClassTransform` and forwards the results to the receiver > builder. > > After the `ClassFile` context object introduction, `ClassModel::transform` > becomes `ClassFile::transform`; now, its role is more similar to > `ClassBuilder::transformMethod`, `ClassBuilder::transformField`, or > `MethodBuilder::transformCode` (transforming subtypes), and it is confusing > with `ClassFileBuilder::transform` (which accepts the same model type as the > built type). We should rename `ClassFile::transform` to > `ClassFile::transformClass` to make this method's role more clear. > > This is separate from #19928 as this has much more user impact. This pull request has now been integrated. Changeset: 0db9bc57 Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/0db9bc57de07f8f1d0bf657621cb1b8fd7b01211 Stats: 176 lines in 58 files changed: 2 ins; 5 del; 169 mod 8335290: Rename ClassFile::transform to ClassFile::transformClass Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/19952
Re: RFR: 8335290: Rename ClassFile::transform to ClassFile::transformClass
On Fri, 28 Jun 2024 22:01:33 GMT, Chen Liang wrote: > `ClassFile::transform` was initially `ClassModel::transform`, which > transforms the receiver class model to a new class byte array. This > functionality was in parallel with `ClassBuilder::transform`, which accepts a > `ClassModel` and a `ClassTransform` and forwards the results to the receiver > builder. > > After the `ClassFile` context object introduction, `ClassModel::transform` > becomes `ClassFile::transform`; now, its role is more similar to > `ClassBuilder::transformMethod`, `ClassBuilder::transformField`, or > `MethodBuilder::transformCode` (transforming subtypes), and it is confusing > with `ClassFileBuilder::transform` (which accepts the same model type as the > built type). We should rename `ClassFile::transform` to > `ClassFile::transformClass` to make this method's role more clear. > > This is separate from #19928 as this has much more user impact. Merged fac74b118f5fda4ec297e46238d34ce5b9be1e21 (one commit before master at the time of comment) locally and tested tier 1-3 again; tests pass. Integrating now to avoid potential future conflicts. Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/19952#issuecomment-2205105705
Integrated: 8321274: Rename ZipEntry.extraAttributes to ZipEntry.externalFileAttributes
On Mon, 4 Dec 2023 15:34:34 GMT, Eirik Bjørsnøs wrote: > Please consider this PR which suggests we rename `ZipEntry.extraAttributes` > to `ZipEntry.externalFileAttributes`. > > This field was introduced in > [JDK-8218021](https://bugs.openjdk.org/browse/JDK-8218021), originally under > the name `ZipEntry.posixPerms`. > [JDK-8250968](https://bugs.openjdk.org/browse/JDK-8250968) later renamed the > field to `ZipEntry.extraAttributes` and extended its semantics to hold the > full two-byte value of the `external file attributes` field, as defined by > `APPNOTE.TXT` > > The name `extraAttributes` is misleading. It has nothing to do with the > `extra field` (an unrelated structure defined in `APPNOTE.TXT`), although the > name indicates it does. > > To prevent confusion and make life easier for future maintainers, I suggest > we rename this field to `ZipEntry.externalFileAttributes` and update related > methods, parameters and comments accordingly. > > While this change is a straightforward renaming, reviewers should consider > whether it carries its weight, especially considering it might complicate > future backports. > > As a note to reviewers, this PR includes the following intended updates: > > - Rename `ZipEntry.extraAttributes` and any references to this field to > `ZipEntry.externalFileAttributes` > - Update `JavaUtilZipFileAccess` to similarly rename methods to > `setExternalFileAttributes` and `getExternalFileAttributes` > - Rename the field `JarSigner.extraAttrsDetected` to > `JarSigner.externalFileAttrsDetected` > - Rename a local variable in `JarSigner.writeEntry` to > `externalFileAttributes` > - Rename `s.s.t.jarsigner.Main.extraAttrsDetected` to > `externalFileAttributesDetected` > - Rename resource string key names in `s.s.t.jarsigner.Resources` from > `extra.attributes.detected` to `external.file.attributes.detected` > - Rename method `SymlinkTest.verifyExtraAttrs` to > `verifyExternalFileAttributes`, also updated two references to 'extra > attributes' in this method > - Updated copyright in all affected files > > If the resource file changes should be dropped and instead handled via > `msgdop` updates, let me know and I can revert the non-default files. > > I did a search across the code base to find 'extraAttrs', 'extra.attr' after > these updates, and found nothing related to zip/jar. The `zip` and `jar` > tests run clean: > > > make test TEST="test/jdk/java/util/jar" > make test TEST="test/jdk/java/util/zip" This pull request has now been integrated. Changeset: d51141e5 Author:Eirik Bjørsnøs Committer: Jaikiran Pai URL: https://git.openjdk.org/jdk/commit/d51141e5fc84f9f933e78d0eb25af86e41798ad5 Stats: 56 lines in 12 files changed: 0 ins; 4 del; 52 mod 8321274: Rename ZipEntry.extraAttributes to ZipEntry.externalFileAttributes Reviewed-by: lancea, jpai - PR: https://git.openjdk.org/jdk/pull/16952
Re: RFR: 8325525: Create jtreg test case for JDK-8325203
On Thu, 20 Jun 2024 15:45:11 GMT, Alexey Semenyuk wrote:
>> Created jtreg test case for
>> [JDK-8325203](https://bugs.openjdk.org/browse/JDK-8325203) issue.
>>
>> The JpackageTest created tests that the child process started from the app
>> launched by jpackage launcher is not automatically terminated when the the
>> launcher is terminated.
>
> The test builds an installer that must be installed as a part of the test.
> This will not work in an environment where tests are executed under a user
> account with restricted permissions. Building app image instead of an
> installer is sufficient for this test.
>
> There are helper classes for writing jpackage tests in
> https://github.com/openjdk/jdk/tree/master/test/jdk/tools/jpackage/helpers/jdk/jpackage/test
> folder.
>
> I'd not use a file to communicate PID of the started process between the
> launcher and the test. I'd use stdout instead:
>
> public class ThirdPartyAppLauncher {
> public static void main(String[] args) throws IOException {
> Process process = new ProcessBuilder("regedit").start();
> System.out.println("RegEdit PID=" + process.pid());
> }
> }
>
>
> Compiling, jarring, and running jpackage that will create app image from the
> jar with these helpers would be as follows:
>
> JPackageCommand.helloAppImage(TKit.TEST_SRC_ROOT.resolve("apps/ThirdPartyAppLauncher.java")
> + "*Hello").executeAndAssertImageCreated();
>
>
> Then you run "ThirdPartyAppLauncher" class using jpackage launcher and
> capture its stdout:
>
> String pidStr = new
> Executor().saveOutput().dumpOutput().setExecutable(cmd.appLauncherPath().toAbsolutePath()).execute(0).getFirstLineOfOutput();
>
> // parse regedit PID
> long regeditPid = Long.parseLong(pidStr.split("=", 2)[1]);
>
> // Run your checks
> ...
>
> // Kill only a specific regedit instance
> Runtime.getRuntime().exec("taskkill /F /PID " + regeditPid);
>
>
> You may use one of the existing jpackage tests for the reference
> (https://github.com/openjdk/jdk/blob/master/test/jdk/tools/jpackage/share/AppLauncherEnvTest.java
> is a good example)
Thanks for the inputs @alexeysemenyukoracle. I have addressed the review
comments.
-
PR Comment: https://git.openjdk.org/jdk/pull/19536#issuecomment-2205063074
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Tue, 2 Jul 2024 19:28:58 GMT, Doug Lea wrote: >> @DougLea is there any timeline where we can expect the backport of this fix >> into jdk17? or any other work around? > > @suryag10 Sorry I'm not the right person to ask about backports. Thanks for the info @DougLea and @jaikiran - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-2205061210
Re: RFR: 8325525: Create jtreg test case for JDK-8325203 [v2]
> Created jtreg test case for > [JDK-8325203](https://bugs.openjdk.org/browse/JDK-8325203) issue. > > The JpackageTest created tests that the child process started from the app > launched by jpackage launcher is not automatically terminated when the the > launcher is terminated. Vanitha B P has updated the pull request incrementally with one additional commit since the last revision: 8325525 Addressed review comments - Changes: - all: https://git.openjdk.org/jdk/pull/19536/files - new: https://git.openjdk.org/jdk/pull/19536/files/86837571..4d22961a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19536&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19536&range=00-01 Stats: 309 lines in 3 files changed: 104 ins; 204 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19536.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19536/head:pull/19536 PR: https://git.openjdk.org/jdk/pull/19536
Integrated: 8335110: Fix instruction name and API spec inconsistencies in CodeBuilder
On Tue, 25 Jun 2024 17:27:27 GMT, Chen Liang wrote: > This is a collection of fixes and improvements to CodeBuilder, plus 2 renames. > > Fixes include: > 1. `CodeBuilder::receiverSlot` typo > 2. `CodeAttribute::labelToBci` update spec > 3. `CodeBuilder::exceptionCatch` implementation > 4. `CodeBuilder::if_nonnull`/`if_null` -> `ifnonnull`/`ifnull` > 5. Docs for what instructions factories emit, and to explain why some > factories have name mismatch; also a section in summary. This pull request has now been integrated. Changeset: f7af4504 Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/f7af4504a804711d93208b763b3e41eafcf61735 Stats: 121 lines in 6 files changed: 105 ins; 1 del; 15 mod 8335110: Fix instruction name and API spec inconsistencies in CodeBuilder Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/19889
Integrated: 8334734: Remove specialized readXxxEntry methods from ClassReader
On Fri, 21 Jun 2024 15:52:44 GMT, Chen Liang wrote: > `ClassReader.readXxxEntry` were added before we had generic, type-aware > `readEntry` and `readEntryOrNull` APIs (#19330). We should remove these > specialized versions in favor of the generic version to reduce API bloating. This pull request has now been integrated. Changeset: 8a664a4c Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/8a664a4c359deefd7237f3672b62d7d8c1ffb453 Stats: 169 lines in 9 files changed: 1 ins; 111 del; 57 mod 8334734: Remove specialized readXxxEntry methods from ClassReader Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/19833
Integrated: 8334726: Remove accidentally exposed individual methods from Class-File API
On Fri, 21 Jun 2024 14:38:44 GMT, Chen Liang wrote: > In preparation of Class-File API exiting review, we are housekeeping our API > surface. These 3 method removals are the most obvious and simple ones. > > This is separated from more throughout and (possibly controversial) changes > for the future, to make reviews (both code and CSR) easier. This pull request has now been integrated. Changeset: 3a2d4264 Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/3a2d426489ead9672512e0c5a6862284a54734ba Stats: 37 lines in 7 files changed: 0 ins; 30 del; 7 mod 8334726: Remove accidentally exposed individual methods from Class-File API Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/19832
[jdk23] RFR: 8335475: ClassBuilder incorrectly calculates max_locals in some cases
Please review this clean backport of #19981 onto JDK 23, fixing `StackMapGenerator` generating static methods with no declared local variable a max local of 1. - Commit messages: - Backport 1ef34c183315b70ddc27c177a2867e30132609f5 Changes: https://git.openjdk.org/jdk/pull/19993/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19993&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8335475 Stats: 32 lines in 2 files changed: 28 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/19993.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19993/head:pull/19993 PR: https://git.openjdk.org/jdk/pull/19993
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Fri, 21 Jul 2023 16:42:25 GMT, Doug Lea wrote: >> This update addresses performance issues across both LinkedTransferQueue and >> SynchronousQueue by creating a common basis for implementation across them >> (mainly in LinkedTransferQueue). Pasting from internal doc summary of >> changes: >> * * Class DualNode replaces Qnode, with fields and methods >> * that apply to any match-based dual data structure, and now >> * usable in other j.u.c classes. in particular, SynchronousQueue. >> * * Blocking control (in class DualNode) accommodates >> * VirtualThreads and (perhaps virtualized) uniprocessors. >> * * All fields of this class (LinkedTransferQueue) are >> * default-initializable (to null), allowing further extension >> * (in particular, SynchronousQueue.Transferer) >> * * Head and tail fields are lazily initialized rather than set >> * to a dummy node, while also reducing retries under heavy >> * contention and misorderings, and relaxing some accesses, >> * requiring accommodation in many places (as well as >> * adjustments in WhiteBox tests). > > Doug Lea has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 13 additional commits since > the last revision: > > - Merge branch 'openjdk:master' into JDK-8301341 > - Address more review comments > - Address review comments > - nitpicks > - Merge branch 'openjdk:master' into JDK-8301341 > - Accommodate white-box tests; use consistent constructions; minor > improvements > - Merge branch 'openjdk:master' into JDK-8301341 > - Simplify contention handling; fix test > - Fix inverted test assert; improve internal documentation; simplify code > - Merge branch 'openjdk:master' into JDK-8301341 > - ... and 3 more: https://git.openjdk.org/jdk/compare/37ac993c...f53cee67 Like Doug notes, the JDK backports are managed as a separate project. Details are available here https://openjdk.org/projects/jdk-updates/. For JDK 17 the corresponding project is https://wiki.openjdk.org/display/JDKUpdates/JDK+17u which has the necessary details about the process as well as the mailing list details where you can bring up the backporting question. - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-2204851286
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed [v3]
On Tue, 2 Jul 2024 20:49:05 GMT, Dean Long wrote: > Why does 8M trigger the OOM Killer, but 1M does not? 8M trigger the OOM killer on some environments, maybe there are some test machines that 8M trigger the OOM exception rather than OOM killer. The intention of change `8M chunks per iteration` to `1M chunks per iteration`, is make sure this testcase throw OOM exception and then [break the memory allocation loop](https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/internal/platform/docker/MetricsMemoryTester.java#L88) before jvm process OOM killed by docker container. - PR Comment: https://git.openjdk.org/jdk/pull/17514#issuecomment-2204847751
Integrated: 8335475: ClassBuilder incorrectly calculates max_locals in some cases
On Mon, 1 Jul 2024 22:54:16 GMT, Chen Liang wrote: > Trivial fix for the bug where `StackMapGenerator` is pre-allocating the > locals incorrectly, affecting static methods with 0 locals. `StackCounter` > was not affected. This pull request has now been integrated. Changeset: 1ef34c18 Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/1ef34c183315b70ddc27c177a2867e30132609f5 Stats: 32 lines in 2 files changed: 28 ins; 0 del; 4 mod 8335475: ClassBuilder incorrectly calculates max_locals in some cases Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/19981
Withdrawn: 8325438: Add exhaustive tests for Math.round intrinsics
On Wed, 7 Feb 2024 16:07:02 GMT, Hamlin Li wrote: > HI, > Can you have a look at this patch adding some tests for Math.round > instrinsics? > Thanks! > > ### FYI: > During the development of RoundVF/RoundF, we faced the issues which were only > spotted by running test exhaustively against 32/64 bits range of int/long. > It's helpful to add these exhaustive tests in jdk for future possible usage, > rather than build it everytime when needed. > Of course, we need to put it in `manual` mode, so it's not run when > `-automatic` jtreg option is specified which I guess is the mode CI used, > please correct me if I'm assume incorrectly. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/17753
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed [v3]
On Tue, 23 Jan 2024 13:04:43 GMT, SendaoYan wrote: >> 8323640: [TESTBUG]testMemoryFailCount in >> jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail >> because OOM killed > > SendaoYan has updated the pull request incrementally with one additional > commit since the last revision: > > 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed > > Signed-off-by: sendaoYan Why does 8M trigger the OOM Killer, but 1M does not? - PR Comment: https://git.openjdk.org/jdk/pull/17514#issuecomment-2204387282
Re: RFR: 8323640: [TESTBUG]testMemoryFailCount in jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because OOM killed [v3]
On Tue, 23 Jan 2024 13:04:43 GMT, SendaoYan wrote: >> 8323640: [TESTBUG]testMemoryFailCount in >> jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail >> because OOM killed > > SendaoYan has updated the pull request incrementally with one additional > commit since the last revision: > > 8323640: [TESTBUG]testMemoryFailCount in > jdk/internal/platform/docker/TestDockerMemoryMetrics.java always fail because > OOM killed > > Signed-off-by: sendaoYan @sendaoYan Your change (at version d1eb4fac03a1ebd4a519f132332cd527afe2090b) is now ready to be sponsored by a Committer. - PR Comment: https://git.openjdk.org/jdk/pull/17514#issuecomment-1906039488
Integrated: 8327854: Test java/util/stream/test/org/openjdk/tests/java/util/stream/WhileOpStatefulTest.java failed with RuntimeException
On Sat, 1 Jun 2024 11:49:39 GMT, Viktor Klang wrote: > This PR improves the test failure output for the failing test case, and the > underlying issue is likely going to be addressed by > https://github.com/openjdk/jdk/pull/19131 /cc @DougLea This pull request has now been integrated. Changeset: 27982c8f Author:Viktor Klang URL: https://git.openjdk.org/jdk/commit/27982c8f5dad0e2d080846f803055c84bac9fddd Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8327854: Test java/util/stream/test/org/openjdk/tests/java/util/stream/WhileOpStatefulTest.java failed with RuntimeException Reviewed-by: psandoz - PR: https://git.openjdk.org/jdk/pull/19508
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Tue, 2 Jul 2024 17:55:31 GMT, Suryanarayana Garlapati wrote: >> @wborn I think 17 should also be OK modulo deleting 2 lines for pre-21 >> mentioned above. I only checked with 19 though.. > > @DougLea is there any timeline where we can expect the backport of this fix > into jdk17? or any other work around? @suryag10 Sorry I'm not the right person to ask about backports. - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-2204174455
Re: RFR: 8321274: Rename ZipEntry.extraAttributes to ZipEntry.externalFileAttributes [v5]
On Tue, 2 Jul 2024 12:51:26 GMT, Jaikiran Pai wrote: > Can you issue a `/integrate delegate` command on this PR so that it then > allows me to do the actual integration along with the Oracle side of changes? Done. And big thanks for your help getting this long-lasting change across the finish line! - PR Comment: https://git.openjdk.org/jdk/pull/16952#issuecomment-2204042346
Re: RFR: 8321274: Rename ZipEntry.extraAttributes to ZipEntry.externalFileAttributes [v5]
On Sat, 29 Jun 2024 18:24:46 GMT, Eirik Bjørsnøs wrote: >> Please consider this PR which suggests we rename `ZipEntry.extraAttributes` >> to `ZipEntry.externalFileAttributes`. >> >> This field was introduced in >> [JDK-8218021](https://bugs.openjdk.org/browse/JDK-8218021), originally under >> the name `ZipEntry.posixPerms`. >> [JDK-8250968](https://bugs.openjdk.org/browse/JDK-8250968) later renamed the >> field to `ZipEntry.extraAttributes` and extended its semantics to hold the >> full two-byte value of the `external file attributes` field, as defined by >> `APPNOTE.TXT` >> >> The name `extraAttributes` is misleading. It has nothing to do with the >> `extra field` (an unrelated structure defined in `APPNOTE.TXT`), although >> the name indicates it does. >> >> To prevent confusion and make life easier for future maintainers, I suggest >> we rename this field to `ZipEntry.externalFileAttributes` and update related >> methods, parameters and comments accordingly. >> >> While this change is a straightforward renaming, reviewers should consider >> whether it carries its weight, especially considering it might complicate >> future backports. >> >> As a note to reviewers, this PR includes the following intended updates: >> >> - Rename `ZipEntry.extraAttributes` and any references to this field to >> `ZipEntry.externalFileAttributes` >> - Update `JavaUtilZipFileAccess` to similarly rename methods to >> `setExternalFileAttributes` and `getExternalFileAttributes` >> - Rename the field `JarSigner.extraAttrsDetected` to >> `JarSigner.externalFileAttrsDetected` >> - Rename a local variable in `JarSigner.writeEntry` to >> `externalFileAttributes` >> - Rename `s.s.t.jarsigner.Main.extraAttrsDetected` to >> `externalFileAttributesDetected` >> - Rename resource string key names in `s.s.t.jarsigner.Resources` from >> `extra.attributes.detected` to `external.file.attributes.detected` >> - Rename method `SymlinkTest.verifyExtraAttrs` to >> `verifyExternalFileAttributes`, also updated two references to 'extra >> attributes' in this method >> - Updated copyright in all affected files >> >> If the resource file changes should be dropped and instead handled via >> `msgdop` updates, let me know and I can revert the non-default files. >> >> I did a search across the code base to find 'extraAttrs', 'extra.attr' after >> these updates, and found nothing related to zip/jar. The `zip` and `jar` >> tests run clean: >> >> >> make test TEST="test/jdk/java/util/jar" >> make test TEST="test/jdk/java/util/zip" > > Eirik Bjørsnøs has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains eight commits: > > - Merge branch 'master' into zipentry-external-attributes > - Rename SymlinkTest.verifyExternalAttrs to verifyExternalFileAttributes > - Rename ZipFileSystem.Entry.posixPerms to externalFileAttributes > - For clarity, use "externalFileAttributes" instead of "externalAttributes" > - Merge branch 'master' into zipentry-external-attributes > - Update copyright years for 2024 > - Merge branch 'master' into zipentry-external-attributes > - Rename ZipEntry.extraAttributes to ZipEntry.externalAttributes Marked as reviewed by lancea (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/16952#pullrequestreview-2154579839
Re: RFR: 8301341: LinkedTransferQueue does not respect timeout for poll() [v10]
On Thu, 10 Aug 2023 15:03:18 GMT, Doug Lea wrote: >> Thanks for making the fixes Doug! >> Would it also be possible to backport these fixes to Java 17? >> >> It seems to be a very common issue for openHAB users now that they upgrade >> to openHAB 4 which requires Java 17. >> >> See: >> >> * >> https://community.openhab.org/t/consistent-100-cpu-use-of-safecall-queue-thread/143792 >> * >> https://community.openhab.org/t/high-cpu-usage-after-migration-to-oh4/148200 > > @wborn I think 17 should also be OK modulo deleting 2 lines for pre-21 > mentioned above. I only checked with 19 though.. @DougLea is there any timeline where we can expect the backport of this fix into jdk17? or any other work around? - PR Comment: https://git.openjdk.org/jdk/pull/14317#issuecomment-2203971692
Withdrawn: 8263261: Extend String::translateEscapes to support unicode escapes
On Thu, 18 Jan 2024 18:50:56 GMT, Jim Laskey wrote:
> Currently String::translateEscapes does not support unicode escapes, reported
> as a IllegalArgumentException("Invalid escape sequence: ...").
> String::translateEscapes should translate unicode escape sequences to provide
> full coverage,
This pull request has been closed without being integrated.
-
PR: https://git.openjdk.org/jdk/pull/17491
Re: RFR: 8332072: Convert package.html files in `java.naming` to package-info.java [v3]
On Tue, 2 Jul 2024 15:09:49 GMT, Aleksei Efimov wrote: >> Nizar Benalla has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Improve package description based on Efimov's suggestion > > src/java.naming/share/classes/javax/naming/event/package-info.java line 60: > >> 58: * An application, for example, can register its interest in changes to >> 59: * objects in a context as follows: >> 60: * > > The leftover `` can be removed: > Suggestion: Fixed both in [657ef5c](https://github.com/openjdk/jdk/pull/19529/commits/657ef5c7532bc587cdead80d35486f30bb931d5e), thank you. - PR Review Comment: https://git.openjdk.org/jdk/pull/19529#discussion_r1662823840
RFR: 8304929: MethodTypeDesc throws an unchecked exception than ReflectiveOperationException when a component class cannot be resolved
Simple fix for `MethodTypeDescImpl`'s violation of `resolveConstantDesc` specification. - Commit messages: - 8304929: MethodTypeDesc throws an unchecked exception than ReflectiveOperationException when a component class cannot be resolved Changes: https://git.openjdk.org/jdk/pull/19991/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19991&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8304929 Stats: 26 lines in 2 files changed: 15 ins; 0 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/19991.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19991/head:pull/19991 PR: https://git.openjdk.org/jdk/pull/19991
Re: RFR: 8332072: Convert package.html files in `java.naming` to package-info.java [v4]
> Can I please get a review for this small change? The motivation is that javac > does not recognize `package.html` files. > > The conversion was simple, I used a script to rename the files, append "*" on > the left and remove some HTML tags like `` and ``. I did the > conversion in place, renaming them in git but with the big amount of change > `git` thinks it's a new file. > > I also added a new `package-info.java` file to `javax.naming.ldap.spi`. I > hope that's fine. Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision: remove two unnecessary tags - Changes: - all: https://git.openjdk.org/jdk/pull/19529/files - new: https://git.openjdk.org/jdk/pull/19529/files/0104e4ac..657ef5c7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19529&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19529&range=02-03 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19529.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19529/head:pull/19529 PR: https://git.openjdk.org/jdk/pull/19529
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v9]
On Wed, 8 May 2024 03:00:50 GMT, Jan Kratochvil wrote: >> Jan Kratochvil has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 49 commits: >> >> - centos7 compat >> - 64a5feb6: >> - fixes >> - e514824f: >> - ebb459e9: >> - Merge branch 'jerboaarefactor-merge' into jerboaarefactor-merge-cgroup >> - Merge branch 'jerboaarefactor' into jerboaarefactor-merge >> - Merge remote-tracking branch 'origin/master' into jerboaarefactor >> - Merge remote-tracking branch 'origin/master' into jerboaarefactor-merge >> - Merge branch 'master-cgroup' into jerboaarefactor-merge-cgroup >> - ... and 39 more: https://git.openjdk.org/jdk/compare/9347bb7d...3da3a9e5 > > Your patch > https://github.com/jerboaa/jdk/commit/92aaa6fd7e3ff8b64de064fecfcd725a157cb5bb#diff-1c49a6b40a810aef82b7da9bfea8f03e07a43062977ba65f75df63c4b7c5b149R89 > contains a tab. @jankratochvil Please merge master and try to use the new code from https://bugs.openjdk.org/browse/JDK-8331560 to query the limits. Thanks! - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-2203599274
Re: RFR: 8332072: Convert package.html files in `java.naming` to package-info.java [v3]
On Fri, 28 Jun 2024 10:44:05 GMT, Nizar Benalla wrote: >> Can I please get a review for this small change? The motivation is that >> javac does not recognize `package.html` files. >> >> The conversion was simple, I used a script to rename the files, append "*" >> on the left and remove some HTML tags like `` and ``. I did the >> conversion in place, renaming them in git but with the big amount of change >> `git` thinks it's a new file. >> >> I also added a new `package-info.java` file to `javax.naming.ldap.spi`. I >> hope that's fine. > > Nizar Benalla has updated the pull request incrementally with one additional > commit since the last revision: > > Improve package description based on Efimov's suggestion Package descriptions generated from new `package-info.java` files look good and have the same content as the ones generated from `package.html` files. A minor issue - two leftover tags need to be removed: src/java.naming/share/classes/javax/naming/event/package-info.java line 60: > 58: * An application, for example, can register its interest in changes to > 59: * objects in a context as follows: > 60: * The leftover `` can be removed: Suggestion: src/java.naming/share/classes/javax/naming/event/package-info.java line 76: > 74: * } > 75: * } > 76: * The leftover `` can be removed: Suggestion: - Changes requested by aefimov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19529#pullrequestreview-2154146953 PR Review Comment: https://git.openjdk.org/jdk/pull/19529#discussion_r1662721260 PR Review Comment: https://git.openjdk.org/jdk/pull/19529#discussion_r1662721369
Re: RFR: 8335366: Improve String.format performance with fastpath [v13]
> We need a String format solution with good performance. String Template was
> once expected, but it has been removed. j.u.Formatter is powerful, but its
> performance is not good enough.
>
> This PR implements a subset of j.u.Formatter capabilities. The performance is
> good enough that it is a fastpath for commonly used functions. When the
> supported functions are exceeded, it will fall back to using j.u.Formatter.
>
> The performance of this implementation is good enough, the fastpath has low
> detection cost, There is no noticeable performance degradation when falling
> back to j.u.Formatter via fastpath.
>
> Below is a comparison of String.format and concat-based and StringBuilder:
>
> * benchmark java code
>
> public class StringFormat {
> @Benchmark
> public String stringIntFormat() {
> return "%s %d".formatted(s, i);
> }
>
> @Benchmark
> public String stringIntConcat() {
> return s + " " + i;
> }
>
> @Benchmark
> public String stringIntStringBuilder() {
> return new StringBuilder(s).append(" ").append(i).toString();
> }
> }
>
>
> * benchmark number on macbook m1 pro
>
> BenchmarkMode Cnt Score Error Units
> StringFormat.stringIntConcat avgt 15 6.541 ? 0.056 ns/op
> StringFormat.stringIntFormat avgt 15 17.399 ? 0.133 ns/op
> StringFormat.stringIntStringBuilder avgt 15 8.004 ? 0.063 ns/op
>
>
> From the above data, we can see that the implementation of fastpath reduces
> the performance difference between String.format and StringBuilder from 10
> times to 2~3 times.
>
> The implementation of fastpath supports the following four specifiers, which
> can appear at most twice and support a width of 1 to 9.
>
> d
> x
> X
> s
>
> If necessary, we can add a few more.
>
>
> Below is a comparison of performance numbers running on a MacBook M1, showing
> a significant performance improvement.
>
> -Benchmark Mode CntScoreError Units
> (baseline)
> -StringFormat.complexFormat avgt 15 895.954 ? 52.541 ns/op
> -StringFormat.decimalFormat avgt 15 277.420 ? 18.254 ns/op
> -StringFormat.stringFormat avgt 15 66.787 ? 2.715 ns/op
> -StringFormat.stringIntFormat avgt 15 81.046 ? 1.879 ns/op
> -StringFormat.widthStringFormat avgt 15 38.897 ? 0.114 ns/op
> -StringFormat.widthStringIntFormat avgt 15 109.841 ? 1.028 ns/op
>
> +Benchmark Mode CntScoreError Units
> (current f925339e93fdf7a281462554ce5d73139bd0f0cd)
> +StringFormat.complexF...
Shaojin Wen has updated the pull request incrementally with one additional
commit since the last revision:
optimize width padding
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19956/files
- new: https://git.openjdk.org/jdk/pull/19956/files/39289870..f1ae26a0
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=19956&range=12
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=19956&range=11-12
Stats: 10 lines in 1 file changed: 4 ins; 0 del; 6 mod
Patch: https://git.openjdk.org/jdk/pull/19956.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19956/head:pull/19956
PR: https://git.openjdk.org/jdk/pull/19956
Re: RFR: 8335366: Improve String.format performance with fastpath [v12]
On Mon, 1 Jul 2024 19:17:50 GMT, Shaojin Wen wrote:
>> We need a String format solution with good performance. String Template was
>> once expected, but it has been removed. j.u.Formatter is powerful, but its
>> performance is not good enough.
>>
>> This PR implements a subset of j.u.Formatter capabilities. The performance
>> is good enough that it is a fastpath for commonly used functions. When the
>> supported functions are exceeded, it will fall back to using j.u.Formatter.
>>
>> The performance of this implementation is good enough, the fastpath has low
>> detection cost, There is no noticeable performance degradation when falling
>> back to j.u.Formatter via fastpath.
>>
>> Below is a comparison of String.format and concat-based and StringBuilder:
>>
>> * benchmark java code
>>
>> public class StringFormat {
>> @Benchmark
>> public String stringIntFormat() {
>> return "%s %d".formatted(s, i);
>> }
>>
>> @Benchmark
>> public String stringIntConcat() {
>> return s + " " + i;
>> }
>>
>> @Benchmark
>> public String stringIntStringBuilder() {
>> return new StringBuilder(s).append(" ").append(i).toString();
>> }
>> }
>>
>>
>> * benchmark number on macbook m1 pro
>>
>> BenchmarkMode Cnt Score Error Units
>> StringFormat.stringIntConcat avgt 15 6.541 ? 0.056 ns/op
>> StringFormat.stringIntFormat avgt 15 17.399 ? 0.133 ns/op
>> StringFormat.stringIntStringBuilder avgt 15 8.004 ? 0.063 ns/op
>>
>>
>> From the above data, we can see that the implementation of fastpath reduces
>> the performance difference between String.format and StringBuilder from 10
>> times to 2~3 times.
>>
>> The implementation of fastpath supports the following four specifiers, which
>> can appear at most twice and support a width of 1 to 9.
>>
>> d
>> x
>> X
>> s
>>
>> If necessary, we can add a few more.
>>
>>
>> Below is a comparison of performance numbers running on a MacBook M1,
>> showing a significant performance improvement.
>>
>> -Benchmark Mode CntScoreError Units
>> (baseline)
>> -StringFormat.complexFormat avgt 15 895.954 ? 52.541 ns/op
>> -StringFormat.decimalFormat avgt 15 277.420 ? 18.254 ns/op
>> -StringFormat.stringFormat avgt 15 66.787 ? 2.715 ns/op
>> -StringFormat.stringIntFormat avgt 15 81.046 ? 1.879 ns/op
>> -StringFormat.widthStringFormat avgt 15 38.897 ? 0.114 ns/op
>> -StringFormat.widthStringIntFormat avgt 15 109.841 ? 1.028 ns/op
>>
>> +Benchmark...
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> improve StringFormat benchmark
I have serious concerns about going forward with this optimization.
It creates duplicate and fragile code that becomes a maintenance overhead.
It reduces the performance of the non-covered cases and does nothing for the
other cases using Formatter.
-
PR Comment: https://git.openjdk.org/jdk/pull/19956#issuecomment-2203237835
Re: RFR: 8332522: SwitchBootstraps::mappedEnumLookup constructs unused array [v4]
On Thu, 27 Jun 2024 19:30:39 GMT, Jan Lahoda wrote:
>> For general pattern matching switches, the `SwitchBootstraps` class
>> currently generates a cascade of `if`-like statements, computing the correct
>> target case index for the given input.
>>
>> There is one special case which permits a relatively easy faster handling,
>> and that is when all the case labels case enum constants (but the switch is
>> still a "pattern matching" switch, as tranditional enum switches do not go
>> through `SwitchBootstraps`). Like:
>>
>>
>> enum E {A, B, C}
>> E e = ...;
>> switch (e) {
>> case null -> {}
>> case A a -> {}
>> case C c -> {}
>> case B b -> {}
>> }
>>
>>
>> We can create an array mapping the runtime ordinal to the appropriate case
>> number, which is somewhat similar to what javac is doing for ordinary
>> switches over enums.
>>
>> The `SwitchBootstraps` class was trying to do so, when the restart index is
>> zero, but failed to do so properly, so that code is not used (and does not
>> actually work properly).
>>
>> This patch is trying to fix that - when all the case labels are enum
>> constants, an array mapping the runtime enum ordinals to the case number
>> will be created (lazily), for restart index == 0. And this map will then be
>> used to quickly produce results for the given input. E.g. for the case
>> above, the mapping will be `{0 -> 0, 1 -> 2, 2 -> 1}` (meaning `{A -> 0, B
>> -> 2, C -> 1}`).
>>
>> When the restart index is != 0 (i.e. when there's a guard in the switch, and
>> the guard returned `false`), the if cascade will be generated lazily and
>> used, as in the general case. If it would turn out there are significant
>> enum-only switches with guards/restart index != 0, we could improve there as
>> well, by generating separate mappings for every (used) restart index.
>>
>> I believe the current tests cover the code functionally sufficiently - see
>> `SwitchBootstrapsTest.testEnums`. It is only that the tests do not (and
>> regression tests cannot easily, I think) differentiate whether the
>> special-case or generic implementation is used.
>>
>> I've added a new microbenchmark attempting to demonstrate the difference.
>> There are two benchmarks, both having only enum constants as case labels.
>> One, `enumSwitchTraditional` is an "old" switch, desugared fully by javac,
>> the other, `enumSwitchWithBootstrap` is an equivalent switch that uses the
>> `SwitchBootstraps`. Before this patch, I was getting values like:
>>
>> Benchmark Mode Cnt Score Error Units
>> SwitchEnum.enumSwitchTraditionalavgt 15 11.719 ± 0.333 ns/op
>> Swi...
>
> Jan Lahoda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Clone the labels.
Marked as reviewed by liach (Committer).
src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 288:
> 286: labels[i] = convertedLabel;
> 287: if (constantsOnly)
> 288: constantsOnly =
> EnumDesc.class.isAssignableFrom(convertedLabel.getClass());
Feels a bit weird when we can use `convertedLabel instanceof EnumDesc`
-
PR Review: https://git.openjdk.org/jdk/pull/19906#pullrequestreview-2153775847
PR Review Comment: https://git.openjdk.org/jdk/pull/19906#discussion_r1662498068
Re: RFR: 8321274: Rename ZipEntry.extraAttributes to ZipEntry.externalFileAttributes [v5]
On Sat, 29 Jun 2024 18:24:46 GMT, Eirik Bjørsnøs wrote: >> Please consider this PR which suggests we rename `ZipEntry.extraAttributes` >> to `ZipEntry.externalFileAttributes`. >> >> This field was introduced in >> [JDK-8218021](https://bugs.openjdk.org/browse/JDK-8218021), originally under >> the name `ZipEntry.posixPerms`. >> [JDK-8250968](https://bugs.openjdk.org/browse/JDK-8250968) later renamed the >> field to `ZipEntry.extraAttributes` and extended its semantics to hold the >> full two-byte value of the `external file attributes` field, as defined by >> `APPNOTE.TXT` >> >> The name `extraAttributes` is misleading. It has nothing to do with the >> `extra field` (an unrelated structure defined in `APPNOTE.TXT`), although >> the name indicates it does. >> >> To prevent confusion and make life easier for future maintainers, I suggest >> we rename this field to `ZipEntry.externalFileAttributes` and update related >> methods, parameters and comments accordingly. >> >> While this change is a straightforward renaming, reviewers should consider >> whether it carries its weight, especially considering it might complicate >> future backports. >> >> As a note to reviewers, this PR includes the following intended updates: >> >> - Rename `ZipEntry.extraAttributes` and any references to this field to >> `ZipEntry.externalFileAttributes` >> - Update `JavaUtilZipFileAccess` to similarly rename methods to >> `setExternalFileAttributes` and `getExternalFileAttributes` >> - Rename the field `JarSigner.extraAttrsDetected` to >> `JarSigner.externalFileAttrsDetected` >> - Rename a local variable in `JarSigner.writeEntry` to >> `externalFileAttributes` >> - Rename `s.s.t.jarsigner.Main.extraAttrsDetected` to >> `externalFileAttributesDetected` >> - Rename resource string key names in `s.s.t.jarsigner.Resources` from >> `extra.attributes.detected` to `external.file.attributes.detected` >> - Rename method `SymlinkTest.verifyExtraAttrs` to >> `verifyExternalFileAttributes`, also updated two references to 'extra >> attributes' in this method >> - Updated copyright in all affected files >> >> If the resource file changes should be dropped and instead handled via >> `msgdop` updates, let me know and I can revert the non-default files. >> >> I did a search across the code base to find 'extraAttrs', 'extra.attr' after >> these updates, and found nothing related to zip/jar. The `zip` and `jar` >> tests run clean: >> >> >> make test TEST="test/jdk/java/util/jar" >> make test TEST="test/jdk/java/util/zip" > > Eirik Bjørsnøs has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains eight commits: > > - Merge branch 'master' into zipentry-external-attributes > - Rename SymlinkTest.verifyExternalAttrs to verifyExternalFileAttributes > - Rename ZipFileSystem.Entry.posixPerms to externalFileAttributes > - For clarity, use "externalFileAttributes" instead of "externalAttributes" > - Merge branch 'master' into zipentry-external-attributes > - Update copyright years for 2024 > - Merge branch 'master' into zipentry-external-attributes > - Rename ZipEntry.extraAttributes to ZipEntry.externalAttributes Marked as reviewed by jpai (Reviewer). Hello Eirik, the latest changes in this PR (`292d6801`) look good to me. However, these changes cause some (expected) compilation failures in some of the internal classes within some Oracle specific JDK classes. Those tests aren't accessible for external contributors. I will be updating that code to address those failures. That would mean that the integration of this PR will have to be co-ordinated. Can you issue a `/integrate delegate` command on this PR so that it then allows me to do the actual integration along with the Oracle side of changes? - PR Review: https://git.openjdk.org/jdk/pull/16952#pullrequestreview-2153714037 PR Comment: https://git.openjdk.org/jdk/pull/16952#issuecomment-2203081198
Re: RFR: 8332522: SwitchBootstraps::mappedEnumLookup constructs unused array [v4]
On Thu, 27 Jun 2024 19:30:39 GMT, Jan Lahoda wrote:
>> For general pattern matching switches, the `SwitchBootstraps` class
>> currently generates a cascade of `if`-like statements, computing the correct
>> target case index for the given input.
>>
>> There is one special case which permits a relatively easy faster handling,
>> and that is when all the case labels case enum constants (but the switch is
>> still a "pattern matching" switch, as tranditional enum switches do not go
>> through `SwitchBootstraps`). Like:
>>
>>
>> enum E {A, B, C}
>> E e = ...;
>> switch (e) {
>> case null -> {}
>> case A a -> {}
>> case C c -> {}
>> case B b -> {}
>> }
>>
>>
>> We can create an array mapping the runtime ordinal to the appropriate case
>> number, which is somewhat similar to what javac is doing for ordinary
>> switches over enums.
>>
>> The `SwitchBootstraps` class was trying to do so, when the restart index is
>> zero, but failed to do so properly, so that code is not used (and does not
>> actually work properly).
>>
>> This patch is trying to fix that - when all the case labels are enum
>> constants, an array mapping the runtime enum ordinals to the case number
>> will be created (lazily), for restart index == 0. And this map will then be
>> used to quickly produce results for the given input. E.g. for the case
>> above, the mapping will be `{0 -> 0, 1 -> 2, 2 -> 1}` (meaning `{A -> 0, B
>> -> 2, C -> 1}`).
>>
>> When the restart index is != 0 (i.e. when there's a guard in the switch, and
>> the guard returned `false`), the if cascade will be generated lazily and
>> used, as in the general case. If it would turn out there are significant
>> enum-only switches with guards/restart index != 0, we could improve there as
>> well, by generating separate mappings for every (used) restart index.
>>
>> I believe the current tests cover the code functionally sufficiently - see
>> `SwitchBootstrapsTest.testEnums`. It is only that the tests do not (and
>> regression tests cannot easily, I think) differentiate whether the
>> special-case or generic implementation is used.
>>
>> I've added a new microbenchmark attempting to demonstrate the difference.
>> There are two benchmarks, both having only enum constants as case labels.
>> One, `enumSwitchTraditional` is an "old" switch, desugared fully by javac,
>> the other, `enumSwitchWithBootstrap` is an equivalent switch that uses the
>> `SwitchBootstraps`. Before this patch, I was getting values like:
>>
>> Benchmark Mode Cnt Score Error Units
>> SwitchEnum.enumSwitchTraditionalavgt 15 11.719 ± 0.333 ns/op
>> Swi...
>
> Jan Lahoda has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Clone the labels.
Any input on the current version of the patch?
-
PR Comment: https://git.openjdk.org/jdk/pull/19906#issuecomment-2203073610
RFR: 8303884: jlink --add-options plugin does not allow GNU style options to be provided
We cannot pass GNU style options like `--enable-preview` to `jlink --add-option`. It is hard to use for complex application. We have workaround for this issue (see JBS), but I think it is better to fix on JDK side. - Commit messages: - 8303884: jlink --add-options plugin does not allow GNU style options to be provided Changes: https://git.openjdk.org/jdk/pull/19987/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19987&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8303884 Stats: 8 lines in 2 files changed: 0 ins; 2 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/19987.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19987/head:pull/19987 PR: https://git.openjdk.org/jdk/pull/19987
Re: RFR: 8335475: ClassBuilder incorrectly calculates max_locals in some cases
On Mon, 1 Jul 2024 22:54:16 GMT, Chen Liang wrote: > Trivial fix for the bug where `StackMapGenerator` is pre-allocating the > locals incorrectly, affecting static methods with 0 locals. `StackCounter` > was not affected. Looks good to me, thanks for the fix. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19981#pullrequestreview-2153129835
