Re: RFR: JDK-8329624: Add visitors for preview language features
On Wed, 3 Apr 2024 20:17:39 GMT, Joe Darcy wrote: > When new language features are added, the javax.lang.model may need to be > updated. For certain classes of features, the API update includes introducing > a new set of concrete visitors to handle the language feature. > > The API scaffolding to support the new feature tends to be considerably > larger than the API specifically for the new feature. > > To aid work in progress (such as https://github.com/openjdk/jdk/pull/18509) > and anticipated in the future, I think it would be helpful to introduce a > persistent set of preview visitors independent of any particular language > change. Looks good to me, thanks! - Marked as reviewed by jlahoda (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18609#pullrequestreview-1978870962
Re: RFR: JDK-8329624: Add visitors for preview language features
On Thu, 4 Apr 2024 05:48:25 GMT, Alan Bateman wrote: > The visitor classes are themselves preview APIs. Is this the first time that > preview APIs have been proposed without a JEP? Just wondering how much > visibility this preview API will get. To clarify the intention, the visitor classes are meant to be persistent preview APIs to help host specific preview APIs associated with modeling upcoming new language features. In particular, these visitor classes are expected to get small updates as part of the work being done in https://github.com/openjdk/jdk/pull/18509 The preview visitor classes as such don't need much visibility as their use will be through various previewing language features. - PR Comment: https://git.openjdk.org/jdk/pull/18609#issuecomment-2036304517
Re: RFR: JDK-8329624: Add visitors for preview language features
On Wed, 3 Apr 2024 20:17:39 GMT, Joe Darcy wrote: > When new language features are added, the javax.lang.model may need to be > updated. For certain classes of features, the API update includes introducing > a new set of concrete visitors to handle the language feature. > > The API scaffolding to support the new feature tends to be considerably > larger than the API specifically for the new feature. > > To aid work in progress (such as https://github.com/openjdk/jdk/pull/18509) > and anticipated in the future, I think it would be helpful to introduce a > persistent set of preview visitors independent of any particular language > change. The visitor classes are themselves preview APIs. Is this the first time that preview APIs have been proposed without a JEP? Just wondering how much visibility this preview API will get. - PR Comment: https://git.openjdk.org/jdk/pull/18609#issuecomment-2036238254
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v23]
On Tue, 19 Mar 2024 16:55:14 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Move CreateLinkableRuntimePlugin to build folder > > Keep runtime link supporting classes in package > jdk.tools.jlink.internal.runtimelink Thanks for the investigation w.r.t. extending jimage. It does not seem worth the effort in pursuing the support of adding resources to an existing jimage file. To me, putting the diff data under `lib` directory as a private file is a simpler and acceptable solution. It allows the second jlink invocation to avoid the plugin pipeline if the per-module metadata is generated in normal jlink invocation. (An observation: The current implementation requires the diffs to be generated as a plugin. For this approach, it may be possible to implement with a separate main class that calls JLink API and generates the diffs.) - PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-2035719255
Re: RFR: JDK-8329624: Add visitors for preview language features
On Wed, 3 Apr 2024 21:38:03 GMT, Joe Darcy wrote: >> src/java.compiler/share/classes/javax/lang/model/util/SimpleElementVisitorPreview.java >> line 2: >> >>> 1: /* >>> 2: * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights >>> reserved. >> >> general, please check the copyright years > > Starting with 2019 was intentional as I copied the corresponding FooVisitor14 > file into FooVisitorPreview and then began editing. ok - PR Review Comment: https://git.openjdk.org/jdk/pull/18609#discussion_r1550548814
Re: RFR: JDK-8329624: Add visitors for preview language features
On Wed, 3 Apr 2024 21:05:20 GMT, Vicente Romero wrote: >> When new language features are added, the javax.lang.model may need to be >> updated. For certain classes of features, the API update includes >> introducing a new set of concrete visitors to handle the language feature. >> >> The API scaffolding to support the new feature tends to be considerably >> larger than the API specifically for the new feature. >> >> To aid work in progress (such as https://github.com/openjdk/jdk/pull/18509) >> and anticipated in the future, I think it would be helpful to introduce a >> persistent set of preview visitors independent of any particular language >> change. > > src/java.compiler/share/classes/javax/lang/model/util/SimpleElementVisitorPreview.java > line 2: > >> 1: /* >> 2: * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights >> reserved. > > general, please check the copyright years Starting with 2019 was intentional as I copied the corresponding FooVisitor14 file into FooVisitorPreview and then began editing. - PR Review Comment: https://git.openjdk.org/jdk/pull/18609#discussion_r1550538694
Re: RFR: JDK-8329624: Add visitors for preview language features
On Wed, 3 Apr 2024 20:17:39 GMT, Joe Darcy wrote: > When new language features are added, the javax.lang.model may need to be > updated. For certain classes of features, the API update includes introducing > a new set of concrete visitors to handle the language feature. > > The API scaffolding to support the new feature tends to be considerably > larger than the API specifically for the new feature. > > To aid work in progress (such as https://github.com/openjdk/jdk/pull/18509) > and anticipated in the future, I think it would be helpful to introduce a > persistent set of preview visitors independent of any particular language > change. looks good src/java.compiler/share/classes/javax/lang/model/util/SimpleElementVisitorPreview.java line 2: > 1: /* > 2: * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights > reserved. general, please check the copyright years - Marked as reviewed by vromero (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18609#pullrequestreview-1978047611 PR Review Comment: https://git.openjdk.org/jdk/pull/18609#discussion_r1550497836
Re: RFR: JDK-8329624: Add visitors for preview language features
On Wed, 3 Apr 2024 20:17:39 GMT, Joe Darcy wrote:
> When new language features are added, the javax.lang.model may need to be
> updated. For certain classes of features, the API update includes introducing
> a new set of concrete visitors to handle the language feature.
>
> The API scaffolding to support the new feature tends to be considerably
> larger than the API specifically for the new feature.
>
> To aid work in progress (such as https://github.com/openjdk/jdk/pull/18509)
> and anticipated in the future, I think it would be helpful to introduce a
> persistent set of preview visitors independent of any particular language
> change.
> When new language features are added, the javax.lang.model may need to be
> updated. For certain classes of features, the API update includes introducing
> a new set of concrete visitors to handle the language feature.
>
> The API scaffolding to support the new feature tends to be considerably
> larger than the API specifically for the new feature.
>
> To aid work in progress (such as #18509) and anticipated in the future, I
> think it would be helpful to introduce a persistent set of preview visitors
> independent of any particular language change.
When one of the preview visitors is subclassed, javac emits a warning like:
javac VisitorTest.java
VisitorTest.java:2: warning: [preview] ElementKindVisitorPreview is a
reflective preview API and may be removed in a future release.
public class VisitorTest extends
javax.lang.model.util.ElementKindVisitorPreview {
^
1 warning
Once these preview visitors are present, the particular API to support language
change P can be added and reviewed without being dwarfed in the overall
scaffolding. The anticipated usage would be:
Methods particular to supporting preview feature P can be annotated accordingly
to indicate that usage.
When P goes to a final feature, a new set of JDK $N visitors would be inserted
between to latest numbered set of visitors and the preview visitors. Methods
particular to P would be moved up into the new JDK $N visitors.
For example, if feature P when final in JDK 25, there would be a new set of JDK
25 concrete visitors extended the existing JDK 14 visitors and superclass of
the preview visitors would be changed from the JDK 14 visitor to the
corresponding JDK 25 visitor. Methods associated with feature P would be moved
up from the preview visitors into the JDK 25 visitors.
-
PR Comment: https://git.openjdk.org/jdk/pull/18609#issuecomment-2035521547
RFR: JDK-8329624: Add visitors for preview language features
When new language features are added, the javax.lang.model may need to be updated. For certain classes of features, the API update includes introducing a new set of concrete visitors to handle the language feature. The API scaffolding to support the new feature tends to be considerably larger than the API specifically for the new feature. To aid work in progress (such as https://github.com/openjdk/jdk/pull/18509) and anticipated in the future, I think it would be helpful to introduce a persistent set of preview visitors independent of any particular language change. - Commit messages: - Remove accidentally added file. - JDK-8329624: Add visitors for preview language features Changes: https://git.openjdk.org/jdk/pull/18609/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18609&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8329624 Stats: 724 lines in 11 files changed: 715 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/18609.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18609/head:pull/18609 PR: https://git.openjdk.org/jdk/pull/18609
Integrated: JDK-8329564: [JVMCI] TranslatedException::debugPrintStackTrace does not work in the libjvmci compiler.
On Wed, 3 Apr 2024 07:02:31 GMT, Tomáš Zezula wrote: > Problem: > The debugging stack traces in `jdk.internal.vm.TranslatedException` do not > work in libjvmci because they are enabled via the > `jdk.internal.vm.TranslatedException.debug` system property. However, HotSpot > system properties are not accessible via `System.getProperty()` in libjvmci. > > Fix: > The value of `jdk.internal.vm.TranslatedException.debug` is passed from the > VM via a boolean flag to `VMSupport::decodeAndThrowThrowable()`. This pull request has now been integrated. Changeset: 8267d656 Author:Tomas Zezula Committer: Doug Simon URL: https://git.openjdk.org/jdk/commit/8267d6565d17c8db8f5b50a37482610ffe0a8a5c Stats: 32 lines in 5 files changed: 8 ins; 1 del; 23 mod 8329564: [JVMCI] TranslatedException::debugPrintStackTrace does not work in the libjvmci compiler. Reviewed-by: dnsimon - PR: https://git.openjdk.org/jdk/pull/18591
Re: RFR: JDK-8319678: Several tests from corelibs areas ignore VM flags
On Wed, 3 Apr 2024 11:47:45 GMT, Mahendra Chhipa wrote:
> Updated following tests to use ProcessTools methods:
> test/jdk/java/lang/Thread/UncaughtExceptionsTest.java
> test/jdk/java/lang/annotation/LoaderLeakTest.java
> test/jdk/java/rmi/reliability/benchmark/bench/rmi/Main.java
> test/jdk/java/time/nontestng/java/time/chrono/HijrahConfigTest.java
> test/jdk/javax/naming/spi/providers/InitialContextTest.java
Changes in `HijrahConfigTest.java` look good
test/jdk/java/time/nontestng/java/time/chrono/HijrahConfigTest.java line 71:
> 69: // Run tests
> 70: Path launcher = outputPath.resolve("bin").resolve("java");
> 71: OutputAnalyzer analyzer =
> ProcessTools.executeCommand(launcher.toAbsolutePath().toString(),"-ea",
> "-esa", "HijrahConfigCheck");
Nit: need a space before `"-ea"`
test/jdk/java/time/nontestng/java/time/chrono/HijrahConfigTest.java line 72:
> 70: Path launcher = outputPath.resolve("bin").resolve("java");
> 71: OutputAnalyzer analyzer =
> ProcessTools.executeCommand(launcher.toAbsolutePath().toString(),"-ea",
> "-esa", "HijrahConfigCheck");
> 72: analyzer.shouldHaveExitValue(0);
The variable `analyzer` may not be needed
-
PR Review: https://git.openjdk.org/jdk/pull/18602#pullrequestreview-1977861951
PR Review Comment: https://git.openjdk.org/jdk/pull/18602#discussion_r1550389295
PR Review Comment: https://git.openjdk.org/jdk/pull/18602#discussion_r1550389971
Re: RFR: JDK-8329564: [JVMCI] TranslatedException::debugPrintStackTrace does not work in the libjvmci compiler. [v3]
On Wed, 3 Apr 2024 19:57:21 GMT, Tomáš Zezula wrote: >> Problem: >> The debugging stack traces in `jdk.internal.vm.TranslatedException` do not >> work in libjvmci because they are enabled via the >> `jdk.internal.vm.TranslatedException.debug` system property. However, >> HotSpot system properties are not accessible via `System.getProperty()` in >> libjvmci. >> >> Fix: >> The value of `jdk.internal.vm.TranslatedException.debug` is passed from the >> VM via a boolean flag to `VMSupport::decodeAndThrowThrowable()`. > > Tomáš Zezula has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8329564: Fixed vmSymbols.hpp formatting. Marked as reviewed by dnsimon (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18591#pullrequestreview-1977858058
Re: RFR: 8328183: Minor mistakes in docs of PrintStream.append()
On Wed, 3 Apr 2024 19:01:14 GMT, Brian Burkhalter wrote: > Clarify the behavior of `append` for a `null` `CharSequence` parameter and > clean up a couple of other typos. Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18607#pullrequestreview-1977836146
Re: RFR: JDK-8329564: [JVMCI] TranslatedException::debugPrintStackTrace does not work in the libjvmci compiler. [v3]
> Problem: > The debugging stack traces in `jdk.internal.vm.TranslatedException` do not > work in libjvmci because they are enabled via the > `jdk.internal.vm.TranslatedException.debug` system property. However, HotSpot > system properties are not accessible via `System.getProperty()` in libjvmci. > > Fix: > The value of `jdk.internal.vm.TranslatedException.debug` is passed from the > VM via a boolean flag to `VMSupport::decodeAndThrowThrowable()`. Tomáš Zezula has updated the pull request incrementally with one additional commit since the last revision: JDK-8329564: Fixed vmSymbols.hpp formatting. - Changes: - all: https://git.openjdk.org/jdk/pull/18591/files - new: https://git.openjdk.org/jdk/pull/18591/files/3a34ce27..de30637b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18591&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18591&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18591.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18591/head:pull/18591 PR: https://git.openjdk.org/jdk/pull/18591
Re: RFR: 8328183: Minor mistakes in docs of PrintStream.append()
On Wed, 3 Apr 2024 19:01:14 GMT, Brian Burkhalter wrote: > Clarify the behavior of `append` for a `null` `CharSequence` parameter and > clean up a couple of other typos. Marked as reviewed by iris (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18607#pullrequestreview-1977785394
RFR: 8328183: Minor mistakes in docs of PrintStream.append()
Clarify the behavior of `append` for a `null` `CharSequence` parameter and clean up a couple of other typos. - Commit messages: - 8328183: Minor mistakes in docs of PrintStream.append() Changes: https://git.openjdk.org/jdk/pull/18607/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18607&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8328183 Stats: 17 lines in 5 files changed: 5 ins; 1 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/18607.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18607/head:pull/18607 PR: https://git.openjdk.org/jdk/pull/18607
Re: RFR: 8328183: Minor mistakes in docs of PrintStream.append()
On Wed, 3 Apr 2024 19:01:14 GMT, Brian Burkhalter wrote: > Clarify the behavior of `append` for a `null` `CharSequence` parameter and > clean up a couple of other typos. Item 4 in [this comment](https://bugs.openjdk.org/browse/JDK-8328183?focusedId=14657943&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14657943) is not addressed as this would be a change to very longstanding behavior and might cause unexpected results. - PR Comment: https://git.openjdk.org/jdk/pull/18607#issuecomment-2035372741
Integrated: JDK-8327474 Review use of java.io.tmpdir in jdk tests
On Mon, 18 Mar 2024 16:47:24 GMT, Bill Huang wrote: > This task addresses an essential aspect of our testing infrastructure: the > proper handling and cleanup of temporary files and socket files created > during test execution. The motivation behind these changes is to prevent the > accumulation of unnecessary files in the default temporary directory, which > can affect the system's storage and potentially influence subsequent test > runs. > > Our review identified that several tests create temporary files or socket > files without ensuring their removal post-execution. > - Direct calls to java.io.File.createTempFile and > java.nio.file.Files.createTempFile without adequate cleanup. > - Tests using NIO socket channels with StandardProtocolFamily.UNIX, not > explicitly removing socket files post-use. This pull request has now been integrated. Changeset: 375bfac8 Author:Bill Huang URL: https://git.openjdk.org/jdk/commit/375bfac8e7ff3f871e2d986876f91a5fba200c83 Stats: 193 lines in 11 files changed: 66 ins; 13 del; 114 mod 8327474: Review use of java.io.tmpdir in jdk tests Reviewed-by: michaelm, jpai - PR: https://git.openjdk.org/jdk/pull/18352
Integrated: JDK-8329557: Fix statement around MathContext.DECIMAL128 rounding
On Tue, 2 Apr 2024 23:43:24 GMT, Joe Darcy wrote: > Happened to notice a semantic typo in the description of > MathContext.DECIMAL128, use of "decimal64" where "decimal128" was intended, > and added some additional information to make the related descriptions more > informative. This pull request has now been integrated. Changeset: 233619b3 Author:Joe Darcy URL: https://git.openjdk.org/jdk/commit/233619b3fb2916ca6216f9d16f70fedf35837a43 Stats: 7 lines in 1 file changed: 0 ins; 0 del; 7 mod 8329557: Fix statement around MathContext.DECIMAL128 rounding Reviewed-by: bpb, iris, rgiulietti - PR: https://git.openjdk.org/jdk/pull/18587
Re: RFR: 8291966: SwitchBootstrap.typeSwitch could be faster [v3]
On Mon, 29 May 2023 07:25:26 GMT, Jan Lahoda wrote:
>> The pattern matching switches are using a bootstrap method
>> `SwitchBootstrap.typeSwitch` to implement the jumps in the switch.
>> Basically, for a switch like:
>>
>> switch (obj) {
>> case String s when s.isEmpty() -> {}
>> case String s -> {}
>> case CharSequence cs -> {}
>> ...
>> }
>>
>>
>> this method will produce a MethodHandle that will be analyze the provided
>> selector value (`obj` in the example), and will return the case index to
>> which the switch should jump. This method also accepts a (re)start index for
>> the search, which is used to handle guards. For example, if the
>> `s.isEmpty()` guard in the above sample returns false, the matching is
>> restarted on the next case.
>>
>> The current implementation is fairly slow, it basically goes through the
>> labels in a loop. The proposal here is to replace that with a MethodHandle
>> structure like this:
>>
>> obj == null ? -1
>> : switch (restartIndex) {
>> case 0 -> obj instanceof String ? 0 : obj instanceof
>> CharSequence ? 2 : ... ;
>> case 1 -> obj instanceof String ? 1 : obj instanceof
>> CharSequence ? 2 : ... ;
>> case 2 -> obj instanceof CharSequence ? 2 : ... ;
>> ...
>> default -> ;
>> }
>>
>>
>> This appear to run faster than the current implementation, using testcase
>> similar to the one used for https://github.com/openjdk/jdk/pull/9746 , these
>> are the results
>>
>> PatternsOptimizationTest.testLegacyIndyLongSwitch thrpt 25 1515989.562
>> ± 32047.918 ops/s
>> PatternsOptimizationTest.testHandleIndyLongSwitch thrpt 25 2630707.585
>> ± 37202.210 ops/s
>>
>> PatternsOptimizationTest.testLegacyIndyShortSwitch thrpt 25 6789310.900
>> ± 61921.636 ops/s
>> PatternsOptimizationTest.testHandleIndyShortSwitch thrpt 25 10771729.464
>> ± 69607.467 ops/s
>>
>>
>> The "LegacyIndy" is the current implementation, "HandleIndy" is the one
>> proposed here. The translation in javac used is the one from #9746 in all
>> cases.
>
> Jan Lahoda has updated the pull request with a new target base due to a merge
> or a rebase. The pull request now contains six commits:
>
> - Reflecting review feedback.
> - Merge branch 'master' into JDK-8291966
> - Adding comments
> - Improving performance
> - Merge branch 'master' into JDK-8291966
> - 8291966: SwitchBootstrap.typeSwitch could be faster
src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 340:
> 338:
>createRepeatIndexSwitch(lookup, labels),
> 339:
>MethodHandles.insertArguments(MAPPED_ENUM_LOOKUP, 1, lookup, enumClass,
> labels, new EnumMap(;
> 340: target = MethodHandles.permuteArguments(body,
> MethodType.methodType(int.class, Object.class, int.class), 1, 0);
This `guardWithTest` does the opposite to what’s described in the above comment.
src/java.base/share/classes/java/lang/runtime/SwitchBootstraps.java line 389:
> 387: }
> 388: }
> 389: return enumMap.map[value.ordinal()];
`enumMap.map` never gets set before this line.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/9779#discussion_r1550058760
PR Review Comment: https://git.openjdk.org/jdk/pull/9779#discussion_r1550057327
Re: RFR: JDK-8319516 - Native library suffix impact on the library loading in AIX- Java Class Loader [v9]
On Wed, 3 Apr 2024 12:32:21 GMT, Suchismith Roy wrote: >> Allow support for both .a and .so files in AIX. >> If .so file is not found, allow fallback to .a extension. >> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516) > > Suchismith Roy has updated the pull request incrementally with one additional > commit since the last revision: > > coding style libsysteminfo.a is another option. - PR Comment: https://git.openjdk.org/jdk/pull/17945#issuecomment-2035024141
Re: RFR: JDK-8329564: [JVMCI] TranslatedException::debugPrintStackTrace does not work in the libjvmci compiler. [v2]
On Wed, 3 Apr 2024 13:58:23 GMT, Tomáš Zezula wrote: >> Problem: >> The debugging stack traces in `jdk.internal.vm.TranslatedException` do not >> work in libjvmci because they are enabled via the >> `jdk.internal.vm.TranslatedException.debug` system property. However, >> HotSpot system properties are not accessible via `System.getProperty()` in >> libjvmci. >> >> Fix: >> The value of `jdk.internal.vm.TranslatedException.debug` is passed from the >> VM via a boolean flag to `VMSupport::decodeAndThrowThrowable()`. > > Tomáš Zezula has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8329564: Fixed TestTranslatedException tests. Marked as reviewed by dnsimon (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18591#pullrequestreview-1977216173
Re: RFR: JDK-8329557: Fix statement around MathContext.DECIMAL128 rounding
On Wed, 3 Apr 2024 14:55:07 GMT, Raffaello Giulietti
wrote:
>> Happened to notice a semantic typo in the description of
>> MathContext.DECIMAL128, use of "decimal64" where "decimal128" was intended,
>> and added some additional information to make the related descriptions more
>> informative.
>
> src/java.base/share/classes/java/math/MathContext.java line 106:
>
>> 104: * rounding mode of {@link RoundingMode#HALF_EVEN HALF_EVEN}.
>> 105: * Note the exponent range of decimal128 (min exponent of -6143,
>> 106: * max exponent of 6144) is not used for rounding.
>
> The exponent ranges are all correct, but... why mention them in the first
> place if they are not used?
My thinking was to mention the ranges to show how different than are from the
approx. +/- MAX_INT range of BigDecimal.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18587#discussion_r1549980224
Re: RFR: 8329331: Intrinsify Unsafe::setMemory [v4]
> This code makes an intrinsic stub for `Unsafe::setMemory` for x86_64. See > [this PR](https://github.com/openjdk/jdk/pull/16760) for discussion around > this change. > > Overall, making this an intrinsic improves overall performance of > `Unsafe::setMemory` by up to 4x for all buffer sizes. > > Tested with tier-1 (and full CI). I've added a table of the before and after > numbers for the JMH I ran (`MemorySegmentZeroUnsafe`). > > [setMemoryBM.txt](https://github.com/openjdk/jdk/files/14808974/setMemoryBM.txt) Scott Gibbons has updated the pull request incrementally with one additional commit since the last revision: Fix Windows - Changes: - all: https://git.openjdk.org/jdk/pull/18555/files - new: https://git.openjdk.org/jdk/pull/18555/files/3aa60a48..8bed1561 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18555&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18555&range=02-03 Stats: 8 lines in 1 file changed: 6 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18555.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18555/head:pull/18555 PR: https://git.openjdk.org/jdk/pull/18555
Re: RFR: JDK-8329089: Empty immutable list throws the wrong exception type for remove(first | last) operations
On Tue, 2 Apr 2024 10:37:02 GMT, Per Minborg wrote: > This PR proposes to make empty immutable lists always throw UOE on > `removeFirst` and `removeLast`. test/jdk/java/util/Collection/MOAT.java line 573: > 571: c::removeLast); > 572: } > 573: Would this test better if inserted in `testImmutableCollection(final Collection c, T t)`, Line 477'ish. Or in `testImmutableList(final List c)`, line 519. - PR Review Comment: https://git.openjdk.org/jdk/pull/18581#discussion_r1549940454
Re: RFR: JDK-8319516 - Native library suffix impact on the library loading in AIX- Java Class Loader [v8]
On Wed, 3 Apr 2024 07:35:46 GMT, Suchismith Roy wrote: >> Hello @suchismith1993, >> >>> > I adjust the comments which also answer your question. Please add an >>> > AIX-only test to verify this behavior. >>> >>> By test you mean provide the use case for pure .a files ? >> >> In the JDK repo, changes like these are backed by jtreg tests >> https://openjdk.org/guide/#jtreg. So for this change, you would need to add >> a jtreg test which only runs on AIX (the jtreg documentation explains how to >> do that) and will exercise the `System.loadLibrary` call for a AIX specific >> library and verify that the library can be loaded as per the expectations >> that are set in the changes proposed and finalized in this PR. >> >> For example, the test should verify that, after the changes proposed in this >> PR, calling `System.loadLibrary()` on AIX will load from >> `.a` if `.so` is absent. >> >> For reference, you can take a look at the existing tests for loadLibrary in >> the `test/jdk/java/lang/RuntimeTests/loadLibrary` directory. > >> test/jdk/java/lang/RuntimeTests/loadLibrary > > I see. Thank you. Got to check some test cases files and how they are > structured. > For this case, i feel the use case would be to have a pure .a file. Usually > there is not such pure .a files that comes packaged with the OS. They are > provided by some applications. So i am not sure how i can make the tests > consistent for such cases. > Do you have any suggestions here ? We are anyways passing it down to the > hotspot code , which is taking care of it. Hello @suchismith1993, > Usually there is not such pure .a files that comes packaged with the OS. They > are provided by some applications. So i am not sure how i can make the tests > consistent for such cases. > Do you have any suggestions here ? Within the JDK repo, some tests require test specific native libraries (like in your case here). It appears that the JDK build infrastructure has ability to compile native code (`foo.c` for example) into a native library. However that build infrastructure doesn't appear to be flexible enough to support the case where you would have to run AIX specific commands to generate a `.a` file. So the approach of generating a test specific `.a` file doesn't look promising right now (it could still be possible, but I don't know how big a build change that would require). So taking a step back - I see that https://bugs.openjdk.org/browse/JDK-8313616 introduced the ability to dealing with `.a` files on AIX from within the hotspot area. In that commit https://github.com/openjdk/jdk/pull/15204/files I see that there are references to a couple of `.a` files - for example `libodm.a` and `libperfstat.a`. Are those `.a` expected to be present on certain AIX systems? Perhaps your new test could use either of those in the test? There was also a mention of `libclang.a` on AIX, previously in this discussion. Maybe that could be used in the test? - PR Comment: https://git.openjdk.org/jdk/pull/17945#issuecomment-2034865328
Re: RFR: JDK-8329557: Fix statement around MathContext.DECIMAL128 rounding
On Tue, 2 Apr 2024 23:43:24 GMT, Joe Darcy wrote: > Happened to notice a semantic typo in the description of > MathContext.DECIMAL128, use of "decimal64" where "decimal128" was intended, > and added some additional information to make the related descriptions more > informative. Marked as reviewed by rgiulietti (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18587#pullrequestreview-1977103640
Re: RFR: JDK-8329557: Fix statement around MathContext.DECIMAL128 rounding
On Tue, 2 Apr 2024 23:43:24 GMT, Joe Darcy wrote:
> Happened to notice a semantic typo in the description of
> MathContext.DECIMAL128, use of "decimal64" where "decimal128" was intended,
> and added some additional information to make the related descriptions more
> informative.
src/java.base/share/classes/java/math/MathContext.java line 106:
> 104: * rounding mode of {@link RoundingMode#HALF_EVEN HALF_EVEN}.
> 105: * Note the exponent range of decimal128 (min exponent of -6143,
> 106: * max exponent of 6144) is not used for rounding.
The exponent ranges are all correct, but... why mention them in the first place
if they are not used?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18587#discussion_r1549913966
Re: RFR: JDK-8329564: [JVMCI] TranslatedException::debugPrintStackTrace does not work in the libjvmci compiler. [v2]
> Problem: > The debugging stack traces in `jdk.internal.vm.TranslatedException` do not > work in libjvmci because they are enabled via the > `jdk.internal.vm.TranslatedException.debug` system property. However, HotSpot > system properties are not accessible via `System.getProperty()` in libjvmci. > > Fix: > The value of `jdk.internal.vm.TranslatedException.debug` is passed from the > VM via a boolean flag to `VMSupport::decodeAndThrowThrowable()`. Tomáš Zezula has updated the pull request incrementally with one additional commit since the last revision: JDK-8329564: Fixed TestTranslatedException tests. - Changes: - all: https://git.openjdk.org/jdk/pull/18591/files - new: https://git.openjdk.org/jdk/pull/18591/files/5e43f5f7..3a34ce27 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18591&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18591&range=00-01 Stats: 7 lines in 1 file changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/18591.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18591/head:pull/18591 PR: https://git.openjdk.org/jdk/pull/18591
Re: RFR: JDK-8319516 - Native library suffix impact on the library loading in AIX- Java Class Loader [v9]
> Allow support for both .a and .so files in AIX. > If .so file is not found, allow fallback to .a extension. > JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516) Suchismith Roy has updated the pull request incrementally with one additional commit since the last revision: coding style - Changes: - all: https://git.openjdk.org/jdk/pull/17945/files - new: https://git.openjdk.org/jdk/pull/17945/files/1ad4b33d..c6ec69a9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17945&range=08 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17945&range=07-08 Stats: 10 lines in 1 file changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/17945.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17945/head:pull/17945 PR: https://git.openjdk.org/jdk/pull/17945
Re: RFR: 8329099: Undocumented exception thrown by Instruction factory methods accepting Opcode [v3]
> `IllegalArgumentException` thrown by some static factory methods of the > following `java.lang.classfile.instruction` interfaces are not documented: > > - `ArrayLoadInstruction` > - `ArrayStoreInstruction` > - `BranchInstruction` > - `ConvertInstruction` > - `DiscontinuedInstruction` > - `FieldInstruction` > - `InvokeInstruction` > - `LoadInstruction` > - `MonitorInstruction` > - `NewPrimitiveArrayInstruction` > - `OperatorInstruction` > - `ReturnInstruction` > - `StackInstruction` > - `StoreInstruction` > - `TypeCheckInstruction` > > `NewPrimitiveArrayInstruction::of` factory method also does not perform the > check for invalid argument. > > This patch adds all the missing `@throws` Javadoc tags and fixes > `NewPrimitiveArrayInstruction::of` factory method to perform the argument > check. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: added missing @throws to ConstantInstruction - Changes: - all: https://git.openjdk.org/jdk/pull/18578/files - new: https://git.openjdk.org/jdk/pull/18578/files/68949904..71165f1e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18578&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18578&range=01-02 Stats: 6 lines in 1 file changed: 6 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18578.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18578/head:pull/18578 PR: https://git.openjdk.org/jdk/pull/18578
Re: RFR: 8329099: Undocumented exception thrown by Instruction factory methods accepting Opcode [v2]
On Wed, 3 Apr 2024 11:42:04 GMT, ExE Boss wrote: > The following factory methods are also missing `@throws > IllegalArgumentException`: > > * `ConstantInstruction::ofIntrinsic` > * `ConstantInstruction::ofArgument` > * `ConstantInstruction::ofLoad` Great catch, I'll add them, thank you! - PR Comment: https://git.openjdk.org/jdk/pull/18578#issuecomment-2034372250
RFR: JDK-8319678: Several tests from corelibs areas ignore VM flags
Updated following tests to use ProcessTools methods: test/jdk/java/lang/Thread/UncaughtExceptionsTest.java test/jdk/java/lang/annotation/LoaderLeakTest.java test/jdk/java/rmi/reliability/benchmark/bench/rmi/Main.java test/jdk/java/time/nontestng/java/time/chrono/HijrahConfigTest.java test/jdk/javax/naming/spi/providers/InitialContextTest.java - Commit messages: - JDK-8319678: Several tests from corelibs areas ignore VM flags Changes: https://git.openjdk.org/jdk/pull/18602/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18602&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8319678 Stats: 138 lines in 5 files changed: 45 ins; 55 del; 38 mod Patch: https://git.openjdk.org/jdk/pull/18602.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18602/head:pull/18602 PR: https://git.openjdk.org/jdk/pull/18602
Re: RFR: 8329099: Undocumented exception thrown by Instruction factory methods accepting Opcode [v2]
On Wed, 3 Apr 2024 10:06:31 GMT, Adam Sotona wrote: >> `IllegalArgumentException` thrown by some static factory methods of the >> following `java.lang.classfile.instruction` interfaces are not documented: >> >> - `ArrayLoadInstruction` >> - `ArrayStoreInstruction` >> - `BranchInstruction` >> - `ConvertInstruction` >> - `DiscontinuedInstruction` >> - `FieldInstruction` >> - `InvokeInstruction` >> - `LoadInstruction` >> - `MonitorInstruction` >> - `NewPrimitiveArrayInstruction` >> - `OperatorInstruction` >> - `ReturnInstruction` >> - `StackInstruction` >> - `StoreInstruction` >> - `TypeCheckInstruction` >> >> `NewPrimitiveArrayInstruction::of` factory method also does not perform the >> check for invalid argument. >> >> This patch adds all the missing `@throws` Javadoc tags and fixes >> `NewPrimitiveArrayInstruction::of` factory method to perform the argument >> check. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > Apply suggestions from code review > > Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> The following factory methods are also missing `@throws IllegalArgumentException`: - `ConstantInstruction::ofIntrinsic` - `ConstantInstruction::ofArgument` - `ConstantInstruction::ofLoad` - PR Comment: https://git.openjdk.org/jdk/pull/18578#issuecomment-2034350493
Re: RFR: 8329099: Undocumented exception thrown by Instruction factory methods accepting Opcode [v2]
> `IllegalArgumentException` thrown by some static factory methods of the > following `java.lang.classfile.instruction` interfaces are not documented: > > - `ArrayLoadInstruction` > - `ArrayStoreInstruction` > - `BranchInstruction` > - `ConvertInstruction` > - `DiscontinuedInstruction` > - `FieldInstruction` > - `InvokeInstruction` > - `LoadInstruction` > - `MonitorInstruction` > - `NewPrimitiveArrayInstruction` > - `OperatorInstruction` > - `ReturnInstruction` > - `StackInstruction` > - `StoreInstruction` > - `TypeCheckInstruction` > > `NewPrimitiveArrayInstruction::of` factory method also does not perform the > check for invalid argument. > > This patch adds all the missing `@throws` Javadoc tags and fixes > `NewPrimitiveArrayInstruction::of` factory method to perform the argument > check. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: Apply suggestions from code review Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> - Changes: - all: https://git.openjdk.org/jdk/pull/18578/files - new: https://git.openjdk.org/jdk/pull/18578/files/185dda1a..68949904 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18578&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18578&range=00-01 Stats: 3 lines in 1 file changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18578.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18578/head:pull/18578 PR: https://git.openjdk.org/jdk/pull/18578
Re: RFR: JDK-8303689: javac -Xlint could/should report on "dangling" doc comments
On Wed, 27 Mar 2024 22:04:30 GMT, Jonathan Gibbons wrote:
> Please review the updates to support a proposed new
> `-Xlint:dangling-doc-comments` option.
>
> The work can be thought of as in 3 parts:
>
> 1. An update to the `javac` internal class `DeferredLintHandler` so that it
> is possible to specify the appropriately configured `Lint` object when it is
> time to consider whether to generate the diagnostic or not.
>
> 2. Updates to the `javac` front end to record "dangling docs comments" found
> near the beginning of a declaration, and to report them using an instance of
> `DeferredLintHandler`. This allows the warnings to be enabled or disabled
> using the standard mechanisms for `-Xlint` and `@SuppressWarnings`. The
> procedure for handling dangling doc comments is described in this comment in
> `JavacParser`.
>
> * Dangling documentation comments are handled as follows.
> * 1. {@code Scanner} adds all doc comments to a queue of
> * recent doc comments. The queue is flushed whenever
> * it is known that the recent doc comments should be
> * ignored and should not cause any warnings.
> * 2. The primary documentation comment is the one obtained
> * from the first token of any declaration.
> * (using {@code token.getDocComment()}.
> * 3. At the end of the "signature" of the declaration
> * (that is, before any initialization or body for the
> * declaration) any other "recent" comments are saved
> * in a map using the primary comment as a key,
> * using this method, {@code saveDanglingComments}.
> * 4. When the tree node for the declaration is finally
> * available, and the primary comment, if any,
> * is "attached", (in {@link #attach}) any related
> * dangling comments are also attached to the tree node
> * by registering them using the {@link #deferredLintHandler}.
> * 5. (Later) Warnings may be genereated for the dangling
> * comments, subject to the {@code -Xlint} and
> * {@code @SuppressWarnings}.
>
>
> 3. Updates to the make files to disable the warnings in modules for which
> the
> warning is generated. This is often because of the confusing use of `/**` to
> create box or other standout comments.
The build changes look okay.
Do you have any plan of going through all the Java modules and fixing the
issues, or opening JBS issues to have them fixed? Or will these lint warnings
remain disabled for the foreseeable future?
-
Marked as reviewed by ihse (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18527#pullrequestreview-1976255818
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v2]
On Tue, 2 Apr 2024 19:19:59 GMT, Volodymyr Paprotski wrote: >> Performance. Before: >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt ScoreError Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt3 6443.934 ± 6.491 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt3 6152.979 ± 4.954 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt3 1895.410 ± 36.979 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt3 1878.955 ± 45.487 ops/s >> Benchmark(algorithm) >> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units >> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1357.810 ± 26.584 ops/s >> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1352.119 ± 23.547 ops/s >> Benchmark (isMontBench) Mode Cnt Score >> Error Units >> PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± >> 10.970 ops/s >> >> Performance, no intrinsic: >> >> Benchmark(algorithm) (dataSize) (keyLength) >> (provider) Mode Cnt Score Error Units >> SignatureBench.ECDSA.signSHA256withECDSA1024 256 >> thrpt3 6529.839 ± 42.420 ops/s >> SignatureBench.ECDSA.signSHA256withECDSA 16384 256 >> thrpt3 6199.747 ± 133.566 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA1024 256 >> thrpt3 1973.676 ± 54.071 ops/s >> SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 >> thrpt3 1932.127 ± 35.920 ops/s >> Benchmark(algorithm) >> (keyLength) (kpgAlgorithm) (provider) Mode Cnt ScoreError Units >> o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1355.788 ± 29.858 ops/s >> o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH >> 256 EC thrpt3 1346.523 ± 28.722 ops/s >> Benchmark (isMontBench) Mode Cnt Score >> Error Units >> PolynomialP256Bench.benchMultiply true thrpt3 1919.57... > > Volodymyr Paprotski has updated the pull request incrementally with one > additional commit since the last revision: > > remove use of jdk.crypto.ec Build changes are trivially fine. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18583#pullrequestreview-1976208258
Result: New Core Libraries Group Member: Per-Ake Minborg
Hi, The vote for Per-Ake Minborg (pminborg) [1] is now closed. Yes: 11 Veto: 0 Abstain: 0 According to the Bylaws definition of Lazy Consensus, this is sufficient to approve the nomination. best regards, -- daniel [1] https://mail.openjdk.org/pipermail/core-libs-dev/2024-March/120617.html
Re: RFR: 8329099: Undocumented exception thrown by Instruction factory methods accepting Opcode
On Tue, 2 Apr 2024 09:37:52 GMT, Adam Sotona wrote:
> `IllegalArgumentException` thrown by some static factory methods of the
> following `java.lang.classfile.instruction` interfaces are not documented:
>
> - `ArrayLoadInstruction`
> - `ArrayStoreInstruction`
> - `BranchInstruction`
> - `ConvertInstruction`
> - `DiscontinuedInstruction`
> - `FieldInstruction`
> - `InvokeInstruction`
> - `LoadInstruction`
> - `MonitorInstruction`
> - `NewPrimitiveArrayInstruction`
> - `OperatorInstruction`
> - `ReturnInstruction`
> - `StackInstruction`
> - `StoreInstruction`
> - `TypeCheckInstruction`
>
> `NewPrimitiveArrayInstruction::of` factory method also does not perform the
> check for invalid argument.
>
> This patch adds all the missing `@throws` Javadoc tags and fixes
> `NewPrimitiveArrayInstruction::of` factory method to perform the argument
> check.
>
> Please review.
>
> Thank you,
> Adam
src/java.base/share/classes/java/lang/classfile/instruction/NewPrimitiveArrayInstruction.java
line 31:
> 29: import java.lang.classfile.Instruction;
> 30: import java.lang.classfile.TypeKind;
> 31: import static java.util.Objects.requireNonNull;
Suggestion:
src/java.base/share/classes/java/lang/classfile/instruction/NewPrimitiveArrayInstruction.java
line 59:
> 57: */
> 58: static NewPrimitiveArrayInstruction of(TypeKind typeKind) {
> 59: if (requireNonNull(typeKind).newarraycode() < 0) {
Suggestion:
// Implicit null-check:
if (typeKind.newarraycode() < 0) {
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18578#discussion_r1549343564
PR Review Comment: https://git.openjdk.org/jdk/pull/18578#discussion_r1549341918
RFR: 8325371: Missing ClassFile.Option in package summary
Some of the Class-File API options were not mentioned in the package summary and one exception mentioned `ClassFile.DeadLabelsOption` javadoc was wrong. This patch fixes the javadoc. Please review. Thank you, Adam - Commit messages: - 8325371: Missing ClassFile.Option in package summary Changes: https://git.openjdk.org/jdk/pull/18594/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18594&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8325371 Stats: 23 lines in 2 files changed: 12 ins; 6 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/18594.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18594/head:pull/18594 PR: https://git.openjdk.org/jdk/pull/18594
RFR: 8329527: Opcode.IFNONNULL.primaryTypeKind() is not ReferenceType
`Opcode.IFNONNULL.primaryTypeKind()` wrongly returned `IntType` instead of the right `ReferenceType`. Primary type kinds of `BRANCH` opcodes have yet no functional effect in the Class-File API. This simple patch fixes the typo. Please review. Thank you, Adam - Commit messages: - 8329527: Opcode.IFNONNULL.primaryTypeKind() is not ReferenceType Changes: https://git.openjdk.org/jdk/pull/18593/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18593&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8329527 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18593.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18593/head:pull/18593 PR: https://git.openjdk.org/jdk/pull/18593
Re: RFR: JDK-8319516 - Native library suffix impact on the library loading in AIX- Java Class Loader [v8]
On Tue, 2 Apr 2024 08:36:15 GMT, Jaikiran Pai wrote: > test/jdk/java/lang/RuntimeTests/loadLibrary I see. Thank you. Got to check some test cases files and how they are structured. For this case, i feel the use case would be to have a pure .a file. Usually there is not such pure .a files that comes packaged with the OS. They are provided by some applications. So i am not sure how i can make the tests consistent for such cases. Do you have any suggestions here ? We are anyways passing it down to the hotspot code , which is taking care of it. - PR Comment: https://git.openjdk.org/jdk/pull/17945#issuecomment-2033773591
Re: RFR: 8324651: Compiler Implementation for Derived Record Creation (Preview) [v5]
On Tue, 2 Apr 2024 12:50:19 GMT, ExE Boss wrote:
>> Jan Lahoda has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixing tests.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java
> line 1415:
>
>> 1413:
>> canonicalConstructorTypes,
>> 1414: List.nil());
>> 1415: createNew.constructor = init;
>
> Maybe instead of hardcoding the constructor that is canonical at compile time
> (in case new components get added), it might be better to go through some
> sort of `indy` callsite like (at least when not in the same compilation unit):
>
> DynamicCallSiteDesc[java.lang.runtime.RecordSupport::derivedConstructor(modified
> component names...):(T extends Record, Class... /* modified component
> types */)T]
>
>
> with the `derivedConstructor` bootstrap method:
>
> public static MethodHandle derivedConstructor(MethodHandles.Lookup lookup,
> String unused, MethodType type, String... modifiedComponents) throws
> ReflectiveOperationException {
> requireNonNull(lookup);
> requireNonNull(type);
> // implicit null-check:
> List modifiedComponentNames = List.of(modifiedComponents);
>
> Class rtype = type.returnType();
> if (
> !rtype.isRecord()
> || type.parameterCount() != modifiedComponents.length + 1
> || type.parameterType(0) != rtype
> ) {
> throw new IllegalArgumentException("...");
> }
>
> Set remainingComponentNames = new
> HashSet(modifiedComponentNames);
> if (remainingComponentNames.size() != modifiedComponentNames.size()) {
> throw new IllegalArgumentException("Duplicate component names
> in modifiedComponents");
> }
>
> RecordComponent[] recordComponents = rtype.getRecordComponents();
>
> var componentTypes = new Class[recordComponents.length];
> var filters = new MethodHandle[recordComponents.length];
> var reorder = new int[recordComponents.length];
>
> for (int i = 0, j = 1; i < recordComponents.length; i++) {
> var component = recordComponents[i];
> componentTypes[i] = component.getType();
>
> var getter = lookup.findVirtual(rtype, component.getName(),
> MethodType.methodType(component.getType()));
> if (modifiedComponentNames.contains(component.getName())) {
> remainingComponentNames.remove(component.getName());
> filters[i] = null;
> reorder[i] = j++;
> } else {
> filters[i] = getter;
> reorder[i] = 0;
> }
> }
>
> if (!remainingComponentNames.isEmpty()) {
> throw new IllegalArgumentException("Components " +
> remainingComponentNames + " are not present in the record " + rtype);
> }
>
> var canonicalConstructor = lookup.findConstructor(rtype,
> MethodType.methodType(rtype, componentTypes);
> ...
While this is very smart, I would prefer not to do that. JLS 13.4.27 talks
about binary compatibility for record, and warns that adding or removing
components may break record usages. So I don't think we need to provide too
much support for adding or removing record components.
Overall, I believe the current code adheres to what the JLS draft says. If the
author of the record wants to add or remove components, they can make it so
that the existing compiled code still works (by adding constructor overloads
and keeping the accessors for removed component). It is better, IMO, to keep
the behavior predictable, and leave adjustments after record component
modifications up to the record author, than to introduce some magic.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1549052694
RFR: JDK-8329564: [JVMCI] TranslatedException::debugPrintStackTrace does not work in the libjvmci compiler.
Problem: The debugging stack traces in the `jdk.internal.vm.TranslatedException` do not work in the libjvmci compiler becuase they are enabled using the system property `jdk.internal.vm.TranslatedException.debug`. However, the libjvmci compiler does not copy HotSpot VM system properties. Instead, the HotSpot system properties are copied only into properties returned by `Services::getSavedProperties()`. Fix: A debug boolean flag is passed to the `VMSupport::decodeAndThrowThrowable()` method. - Commit messages: - JDK-8329564: jdk.internal.vm.TranslatedException::debugPrintStackTrace does not work in the libjvmci compiler. Changes: https://git.openjdk.org/jdk/pull/18591/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18591&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8329564 Stats: 25 lines in 4 files changed: 8 ins; 1 del; 16 mod Patch: https://git.openjdk.org/jdk/pull/18591.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18591/head:pull/18591 PR: https://git.openjdk.org/jdk/pull/18591
