[jdk23] RFR: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS
Please review this clean backport of #19708, to make javap recover and continue after encountering undefined access flag bits set while still exiting with a code of error, allowing it to error against improper bits while still providing as much output as possible. - Commit messages: - Backport 7e55ed3b106ed08956d2d38b7c99fb81704667c9 Changes: https://git.openjdk.org/jdk/pull/19839/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19839=00 Issue: https://bugs.openjdk.org/browse/JDK-8333748 Stats: 228 lines in 4 files changed: 168 ins; 44 del; 16 mod Patch: https://git.openjdk.org/jdk/pull/19839.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19839/head:pull/19839 PR: https://git.openjdk.org/jdk/pull/19839
Integrated: 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS
On Thu, 13 Jun 2024 17:50:38 GMT, Chen Liang wrote: > Currently, javap crashes for class files that have set non-zero values for > undefined access flag bits, as > `java.lang.reflect.AccessFlag.maskToAccessFlag` and > `java.lang.classfile.AccessFlags.flags` fail. In contrast, the JVMS, though > asking for these bits to be set to 0, requires VM to proceed and ignore these > bits. javap should emulate the VM behavior and proceed rendering, ignoring > these undefined bits. > > In addition, a few bytecode generation utilities in the JDK set unused bits, > such as in > `java.lang.invoke.MethodHandleImpl.BindCaller#generateInvokerTemplate` and > `java.lang.invoke.GenerateJLIClassesHelper#generateCodeBytesForLFs`. Those > can be updated in a later cleanup. This pull request has now been integrated. Changeset: 7e55ed3b Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/7e55ed3b106ed08956d2d38b7c99fb81704667c9 Stats: 228 lines in 4 files changed: 168 ins; 44 del; 16 mod 8333748: javap crash - Fatal error: Unmatched bit position 0x2 for location CLASS Reviewed-by: asotona - PR: https://git.openjdk.org/jdk/pull/19708
[jdk23] Integrated: 8333854: IllegalAccessError with proxies after JDK-8332457
On Thu, 20 Jun 2024 20:11:06 GMT, Chen Liang wrote: > Please review this patch, which is a backport of the fix in #19615 to JDK 23. > > This is not a clean patch, because the old patch was done on JDK-8333479 > (#19585) which was absent in JDK 23; however, the conflicts were small, and > the only real changes were that `methodTypeDesc` and `classDesc` from > `ConstantUtils` were not available, so the original approaches were retained. > There is also import adjustments. > > Testing: tier 1-3 This pull request has now been integrated. Changeset: 1dbad805 Author:Chen Liang URL: https://git.openjdk.org/jdk/commit/1dbad8058b79c4e31e389913a7259323a9b8ab93 Stats: 238 lines in 2 files changed: 161 ins; 9 del; 68 mod 8333854: IllegalAccessError with proxies after JDK-8332457 Reviewed-by: asotona Backport-of: 91bd85d65dff9cea91b88da7ef241be5c7b85f94 - PR: https://git.openjdk.org/jdk/pull/19815
Re: RFR: 8334708: FFM: two javadoc problems
On Fri, 21 Jun 2024 07:40:31 GMT, Hannes Greule wrote: > Addresses two simple problems regarding javadocs in the FFM API. Yes, javadoc fixes such as this can be backported. Just use the `/backport` command: https://wiki.openjdk.org/display/SKARA/Pull+Request+Commands#PullRequestCommands-/backport - PR Comment: https://git.openjdk.org/jdk/pull/19820#issuecomment-2183478163
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v8]
On Thu, 20 Jun 2024 17:25:33 GMT, Alan Bateman wrote: >> Something like that yes > > An altermative is to use ResolvedModule::reference to get a ModuleReference, > then use its open method to open the contents of the module to get a > ModuleReader. That will give you a stream over the names of all resources in > the module. It will work for all exploded and packaged modules. I took a look at this. I had to refactor a bit and introduce an intermediate abstraction (`ClassFileSource`) to abstract over classes loaded from a `JarFile` (for the class path) and a `ModuleReader` (for module path). At that point, it was also easy to add a third option for directories of class files on that class path, so I've done that as well. See: https://github.com/openjdk/jdk/pull/19774/commits/a046f6fe9465feda63609dd50014338ca218923f - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649434411
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v8]
> This PR adds a new JDK tool, called `jnativescan`, that can be used to find > code that accesses native functionality. Currently this includes `native` > method declarations, and methods marked with `@Restricted`. > > The tool accepts a list of class path and module path entries through > `--class-path` and `--module-path`, and a set of root modules through > `--add-modules`, as well as an optional target release with `--release`. > > The default mode is for the tool to report all uses of `@Restricted` methods, > and `native` method declaration in a tree-like structure: > > > app.jar (ALL-UNNAMED): > main.Main: > main.Main::main(String[])void references restricted methods: > java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment > main.Main::m()void is a native method declaration > > > The `--print-native-access` option can be used print out all the module names > of modules doing native access in a comma separated list. For class path > code, this will print out `ALL-UNNAMED`. > > Testing: > - `langtools_jnativescan` tests. > - Running the tool over jextract's libclang bindings, which use the FFM API, > and thus has a lot of references to `@Restricted` methods. > - tier 1-3 Jorn Vernee has updated the pull request incrementally with three additional commits since the last revision: - Add support for module directories + class path directories - sort output for easier diffs - Jan comments - Changes: - all: https://git.openjdk.org/jdk/pull/19774/files - new: https://git.openjdk.org/jdk/pull/19774/files/a4723494..a046f6fe Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19774=07 - incr: https://webrevs.openjdk.org/?repo=jdk=19774=06-07 Stats: 275 lines in 8 files changed: 156 ins; 58 del; 61 mod Patch: https://git.openjdk.org/jdk/pull/19774.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19774/head:pull/19774 PR: https://git.openjdk.org/jdk/pull/19774
Re: RFR: 8331342: Convert Base64 tests to JUnit [v2]
> Please review this test-only clean up PR which converts the java.util.Base64 > tests to run under JUnit. > > In general, this allows for the tests to run independently, separates the > data providers from the tests, as well being able to utilize the built in > JUnit utility test methods to reduce boilerplate testing code. Justin Lu 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 five additional commits since the last revision: - Merge branch 'master' into java.util.Base64-JUnit-conversions - Additional clarifying comments - convert TestBase64.java - convert TestBase64Golden.java - convert Base64GetEncoderTest.java - Changes: - all: https://git.openjdk.org/jdk/pull/19337/files - new: https://git.openjdk.org/jdk/pull/19337/files/945bc482..706293d4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19337=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19337=00-01 Stats: 166493 lines in 2916 files changed: 110047 ins; 41299 del; 15147 mod Patch: https://git.openjdk.org/jdk/pull/19337.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19337/head:pull/19337 PR: https://git.openjdk.org/jdk/pull/19337
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v7]
On Fri, 21 Jun 2024 18:38:07 GMT, Jorn Vernee wrote:
>> test/langtools/tools/jnativescan/TestJNativeScan.java line 174:
>>
>>> 172: "-add-modules",
>>> "org.singlejar,org.myapp",
>>> 173: "--print-native-access"))
>>> 174: .stdoutShouldContain("org.singlejar")
>>
>> It is a small thing, bu was there a consideration for a stronger assert on
>> the output, checking that the output is precisely something like
>> `--enable-native-access org.lib,org.service,org.singlejar`? Would require
>> that the output is stable, which may be tricky, but also not a bad property.
>> Just an idea for consideration.
>
> Good point. The result should be 'clean' and directly forward-able to
> `--enable-native-access`. (So in this case it should be exactly
> `org.singlejar`)
I'm fixing this test, and also making the output sorted. That is required for
the output stability, but I also thought that would be a nice property in case
a user wants to diff the tool's outputs.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649418854
Re: Incorrect note in Javadoc for a few RandomGenerator methods
Thanks, filed 9077206. Den fre. 21. jun. 2024 kl. 21.06 skrev Raffaello Giulietti < raffaello.giulie...@oracle.com>: > Hi, > > your observation seems correct. > > In order to file a properly tracked bug report, please proceed according > to this guide: > > > https://docs.oracle.com/en/java/javase/22/troubleshoot/submit-bug-report.html > > Thanks > > > > On 2024-06-21 19:12, Stig Rohde Døssing wrote: > > Hi, > > > > The Javadoc for RandomGenerator.nextLong(long origin, long bound) has > > this to say: > > > > Implementation Requirements: > > The default implementation checks that |origin| and |bound| are positive > > |longs| > > > > |This doesn't seem to be true. The default implementation checks that > > origin and bound are a valid range (that bound >= origin). The > > implementation doesn't reject negative inputs.| > > | > > | > > |The same note appears in the two-arg version of nextInt.| > > | > > | > > |The note for e.g. nextDouble is correct and says "|The default > > implementation verifies that the |origin| and |bound| are valid" > > > > I'm wondering if the notes for nextLong and nextInt should be updated to > > match? >
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote: >> This patch contains a set of changes to improve static builds. They will >> pave the way for implementing a full static-only java launcher. The changes >> here will: >> >> 1) Make sure non-exported symbols are made local in the static libraries. >> This means that the risk of symbol conflict is the same for static libraries >> as for dynamic libraries (i.e. in practice zero, as long as a consistent >> naming scheme is used for exported functions). >> >> 2) Remove the work-arounds to exclude duplicated symbols. >> >> 3) Fix some code in hotspot and the JDK libraries that did not work properly >> with a static java launcher. >> >> The latter fixes are copied from or inspired by the work done by >> @jianglizhou and her team as part of the Project Leyden [Hermetic >> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add dummy implementation of os::lookup_function for Windows I've looked through all JDK and VM changes and left comments in various places. All the rest changes in PR look good. Thanks again for extracting these changes from the leyden/hermeticJava branch and integrating with mainline! My other main question is why the `javastatic` linking work is not included in the PR together with these runtime changes. IIUC from our meetings and mailing list discussions, the initial integration PR needs to include the part for statically linking the `javastatic`. That's a minimum requirement for testing/verifying the runtime changes when integrating into the mainline, which is also the reason why we haven't starting integrating any of the runtime changes so far. Has that been changed? - PR Review: https://git.openjdk.org/jdk/pull/19478#pullrequestreview-2133328296
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote:
>> This patch contains a set of changes to improve static builds. They will
>> pave the way for implementing a full static-only java launcher. The changes
>> here will:
>>
>> 1) Make sure non-exported symbols are made local in the static libraries.
>> This means that the risk of symbol conflict is the same for static libraries
>> as for dynamic libraries (i.e. in practice zero, as long as a consistent
>> naming scheme is used for exported functions).
>>
>> 2) Remove the work-arounds to exclude duplicated symbols.
>>
>> 3) Fix some code in hotspot and the JDK libraries that did not work properly
>> with a static java launcher.
>>
>> The latter fixes are copied from or inspired by the work done by
>> @jianglizhou and her team as part of the Project Leyden [Hermetic
>> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime).
>
> Magnus Ihse Bursie has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Add dummy implementation of os::lookup_function for Windows
src/java.base/unix/native/libjli/java_md.c line 316:
> 314: SetExecname(*pargv);
> 315:
> 316: if (!JLI_IsStaticallyLinked()) {
Any reason this is diverted from the change in hermetic Java branch,
https://github.com/openjdk/leyden/blob/c1c5fc686c1452550e1b3663a320fba652248505/src/java.base/unix/native/libjli/java_md.c#L300?
I think the setenv part below is not needed for static/hermetic support
either. There is no $JRE/lib with a single executable image. All natives are
statically linked with the executable image.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r1649356484
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote: >> This patch contains a set of changes to improve static builds. They will >> pave the way for implementing a full static-only java launcher. The changes >> here will: >> >> 1) Make sure non-exported symbols are made local in the static libraries. >> This means that the risk of symbol conflict is the same for static libraries >> as for dynamic libraries (i.e. in practice zero, as long as a consistent >> naming scheme is used for exported functions). >> >> 2) Remove the work-arounds to exclude duplicated symbols. >> >> 3) Fix some code in hotspot and the JDK libraries that did not work properly >> with a static java launcher. >> >> The latter fixes are copied from or inspired by the work done by >> @jianglizhou and her team as part of the Project Leyden [Hermetic >> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add dummy implementation of os::lookup_function for Windows src/java.base/macosx/native/libjli/java_md_macosx.m line 1: > 1: /* In the mailing list email discussion thread on hermetic Java, you mentioned running on macosx with a build from hermtic Java branch crashed for you during startup. Is that fully resolved with the changes in this PR? The hermetic Java branch does not have any changes for macosx port. What tests are done for the macosx port for static support? - PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r1649348777
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote:
>> This patch contains a set of changes to improve static builds. They will
>> pave the way for implementing a full static-only java launcher. The changes
>> here will:
>>
>> 1) Make sure non-exported symbols are made local in the static libraries.
>> This means that the risk of symbol conflict is the same for static libraries
>> as for dynamic libraries (i.e. in practice zero, as long as a consistent
>> naming scheme is used for exported functions).
>>
>> 2) Remove the work-arounds to exclude duplicated symbols.
>>
>> 3) Fix some code in hotspot and the JDK libraries that did not work properly
>> with a static java launcher.
>>
>> The latter fixes are copied from or inspired by the work done by
>> @jianglizhou and her team as part of the Project Leyden [Hermetic
>> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime).
>
> Magnus Ihse Bursie has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Add dummy implementation of os::lookup_function for Windows
src/hotspot/share/utilities/zipLibrary.cpp line 63:
> 61:
> 62: static void* dll_lookup(const char* name, const char* path, bool
> vm_exit_on_failure) {
> 63: if (vm_is_statically_linked()) {
I like this change. It is cleaner than the hermetic Java branch change that
does the `if` static check in `store_function_pointers`
(https://github.com/openjdk/leyden/blob/c1c5fc686c1452550e1b3663a320fba652248505/src/hotspot/share/utilities/zipLibrary.cpp#L75).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r1649341844
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote:
>> This patch contains a set of changes to improve static builds. They will
>> pave the way for implementing a full static-only java launcher. The changes
>> here will:
>>
>> 1) Make sure non-exported symbols are made local in the static libraries.
>> This means that the risk of symbol conflict is the same for static libraries
>> as for dynamic libraries (i.e. in practice zero, as long as a consistent
>> naming scheme is used for exported functions).
>>
>> 2) Remove the work-arounds to exclude duplicated symbols.
>>
>> 3) Fix some code in hotspot and the JDK libraries that did not work properly
>> with a static java launcher.
>>
>> The latter fixes are copied from or inspired by the work done by
>> @jianglizhou and her team as part of the Project Leyden [Hermetic
>> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime).
>
> Magnus Ihse Bursie has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Add dummy implementation of os::lookup_function for Windows
src/hotspot/share/runtime/os.cpp line 521:
> 519: char ebuf[1024];
> 520:
> 521: if (vm_is_statically_linked()) {
This block can be moved before the two variable declarations above, since they
are not needed in the static case.
https://github.com/openjdk/leyden/blob/c1c5fc686c1452550e1b3663a320fba652248505/src/hotspot/share/runtime/os.cpp#L507
handles it that way.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r164982
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v7]
On Fri, 21 Jun 2024 18:37:01 GMT, Jorn Vernee wrote: >> src/jdk.jdeps/share/classes/com/sun/tools/jdeprscan/Main.java line 417: >> >>> 415: return false; >>> 416: } >>> 417: JavaFileManager fm = >>> pp.getPlatformTrusted(release).getFileManager(); >> >> Not sure if this change is necessary. I believe `release` is verified to be >> a valid platform name at this point, so even with the new check, it should >> still work. (And `getPlatformTrusted` could possibly be eliminated.) But >> maybe I am missing something? > > I wanted to avoid adding exception handling code here, since > `PlatformNotSupported` is a checked exception. Ah, OK, I didn't realize that. Seems fine then! - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649315863
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v7]
On Fri, 21 Jun 2024 18:31:00 GMT, Jan Lahoda wrote:
>> Jorn Vernee has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> add extra test for missing root modules
>
> src/jdk.jdeps/share/classes/com/sun/tools/jdeprscan/Main.java line 417:
>
>> 415: return false;
>> 416: }
>> 417: JavaFileManager fm =
>> pp.getPlatformTrusted(release).getFileManager();
>
> Not sure if this change is necessary. I believe `release` is verified to be a
> valid platform name at this point, so even with the new check, it should
> still work. (And `getPlatformTrusted` could possibly be eliminated.) But
> maybe I am missing something?
I wanted to avoid adding exception handling code here, since
`PlatformNotSupported` is a checked exception.
> test/langtools/tools/jnativescan/TestJNativeScan.java line 174:
>
>> 172: "-add-modules",
>> "org.singlejar,org.myapp",
>> 173: "--print-native-access"))
>> 174: .stdoutShouldContain("org.singlejar")
>
> It is a small thing, bu was there a consideration for a stronger assert on
> the output, checking that the output is precisely something like
> `--enable-native-access org.lib,org.service,org.singlejar`? Would require
> that the output is stable, which may be tricky, but also not a bad property.
> Just an idea for consideration.
Good point. The result should be 'clean' and directly forward-able to
`--enable-native-access`. (So in this case it should be exactly `org.singlejar`)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649312391
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649313272
Re: RFR: 8333268: Fixes for static build [v4]
On Wed, 19 Jun 2024 15:15:43 GMT, Magnus Ihse Bursie wrote: >> This patch contains a set of changes to improve static builds. They will >> pave the way for implementing a full static-only java launcher. The changes >> here will: >> >> 1) Make sure non-exported symbols are made local in the static libraries. >> This means that the risk of symbol conflict is the same for static libraries >> as for dynamic libraries (i.e. in practice zero, as long as a consistent >> naming scheme is used for exported functions). >> >> 2) Remove the work-arounds to exclude duplicated symbols. >> >> 3) Fix some code in hotspot and the JDK libraries that did not work properly >> with a static java launcher. >> >> The latter fixes are copied from or inspired by the work done by >> @jianglizhou and her team as part of the Project Leyden [Hermetic >> Java](https://github.com/openjdk/leyden/tree/hermetic-java-runtime). > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add dummy implementation of os::lookup_function for Windows src/hotspot/os/bsd/os_bsd.cpp line 1: > 1: /* The changes in os_bsd.cpp are new and are not from https://github.com/openjdk/leyden/tree/hermetic-java-runtime/. Have you tested the bsd port? - PR Review Comment: https://git.openjdk.org/jdk/pull/19478#discussion_r1649312808
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 17:44:54 GMT, Alan Bateman wrote: >> src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/JNativeScanTask.java >> line 113: >> >>> 111: // Class-Path attribute specifies that jars that >>> 112: // are not found are simply ignored. Do the same >>> here >>> 113: classPathJars.offer(otherJar); >> >> The expansion of the class path looks right but the Class-Path attribute is >> awkward to deal with as its relative URI rather than a file path. More on >> this this later. > > Not important for now but the expansion will probably need to"visited" set to > detect cycles (yes, it is possible). > > In addition to cycles, it is possible to have several JAR files with the same > Class-Path value, e.g. lib/logging.jar, so a visited set would be useful for > that too. FWIW, javac has `com.sun.tools.javac.file.FSInfo.getJarClassPath`, which is not transitive, and is implemented a bit differently, but I wonder if there's a chance for some code sharing. Maybe not now, but eventually. - PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649296334
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v7]
On Thu, 20 Jun 2024 19:45:29 GMT, Jorn Vernee wrote:
>> This PR adds a new JDK tool, called `jnativescan`, that can be used to find
>> code that accesses native functionality. Currently this includes `native`
>> method declarations, and methods marked with `@Restricted`.
>>
>> The tool accepts a list of class path and module path entries through
>> `--class-path` and `--module-path`, and a set of root modules through
>> `--add-modules`, as well as an optional target release with `--release`.
>>
>> The default mode is for the tool to report all uses of `@Restricted`
>> methods, and `native` method declaration in a tree-like structure:
>>
>>
>> app.jar (ALL-UNNAMED):
>> main.Main:
>> main.Main::main(String[])void references restricted methods:
>> java.lang.foreign.MemorySegment::reinterpret(long)MemorySegment
>> main.Main::m()void is a native method declaration
>>
>>
>> The `--print-native-access` option can be used print out all the module
>> names of modules doing native access in a comma separated list. For class
>> path code, this will print out `ALL-UNNAMED`.
>>
>> Testing:
>> - `langtools_jnativescan` tests.
>> - Running the tool over jextract's libclang bindings, which use the FFM API,
>> and thus has a lot of references to `@Restricted` methods.
>> - tier 1-3
>
> Jorn Vernee has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add extra test for missing root modules
Overall looks great to me. Some minor comments inline.
src/jdk.compiler/share/classes/com/sun/tools/javac/platform/JDKPlatformProvider.java
line 87:
> 85: @Override
> 86: public PlatformDescription getPlatform(String platformName, String
> options) throws PlatformNotSupported {
> 87: if (Source.lookup(platformName) == null) {
`Source.lookup` is probably not the right way to check whether the platform is
supported - the `Source` enum may (and does) contain versions for which we
don't have the historical API record. I think
`SUPPORTED_JAVA_PLATFORM_VERSIONS.contains(platformName)` would work better,
since the content is read directly from `ct.sym`.
src/jdk.jdeps/share/classes/com/sun/tools/jdeprscan/Main.java line 417:
> 415: return false;
> 416: }
> 417: JavaFileManager fm =
> pp.getPlatformTrusted(release).getFileManager();
Not sure if this change is necessary. I believe `release` is verified to be a
valid platform name at this point, so even with the new check, it should still
work. (And `getPlatformTrusted` could possibly be eliminated.) But maybe I am
missing something?
src/jdk.jdeps/share/classes/com/sun/tools/jnativescan/ClassResolver.java line
125:
> 123: private static Map
> packageToSystemModule(JavaFileManager platformFileManager) {
> 124: try {
> 125: Set locations =
> platformFileManager.listLocationsForModules(
FWIW: +1 on using the modules from the `ct.sym` rather than runtime modules
here!
test/langtools/tools/jnativescan/TestJNativeScan.java line 174:
> 172: "-add-modules",
> "org.singlejar,org.myapp",
> 173: "--print-native-access"))
> 174: .stdoutShouldContain("org.singlejar")
It is a small thing, bu was there a consideration for a stronger assert on the
output, checking that the output is precisely something like
`--enable-native-access org.lib,org.service,org.singlejar`? Would require that
the output is stable, which may be tricky, but also not a bad property. Just an
idea for consideration.
-
PR Review: https://git.openjdk.org/jdk/pull/19774#pullrequestreview-2133198642
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649303668
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649307310
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649297021
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649294137
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v7]
On Thu, 20 Jun 2024 16:58:55 GMT, Alan Bateman wrote:
>> src/jdk.compiler/share/classes/com/sun/tools/javac/platform/JDKPlatformProvider.java
>> line 93:
>>
>>> 91: }
>>> 92:
>>> 93: public PlatformDescription getPlatformTrusted(String platformName) {
>>
>> I noticed that `getPlatform` was not throwing an exception if the release
>> was not valid, which then later results in an NPE. I've added an explicit
>> check here instead. The caller can then catch the exception.
>
> I see the "options" arg is not used so maybe another approach here is a
> one-arg getPlatform that throws PlatformNotSupported and then migrate the
> other usages at another time.
>
> (This is just a passing comment, not important for the core of this feature).
We probably can eliminate the `options` (which is a historical artifact), but I
don't think it needs to be done here
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19774#discussion_r1649307870
Re: Incorrect note in Javadoc for a few RandomGenerator methods
Hi, your observation seems correct. In order to file a properly tracked bug report, please proceed according to this guide: https://docs.oracle.com/en/java/javase/22/troubleshoot/submit-bug-report.html Thanks On 2024-06-21 19:12, Stig Rohde Døssing wrote: Hi, The Javadoc for RandomGenerator.nextLong(long origin, long bound) has this to say: Implementation Requirements: The default implementation checks that |origin| and |bound| are positive |longs| |This doesn't seem to be true. The default implementation checks that origin and bound are a valid range (that bound >= origin). The implementation doesn't reject negative inputs.| | | |The same note appears in the two-arg version of nextInt.| | | |The note for e.g. nextDouble is correct and says "|The default implementation verifies that the |origin| and |bound| are valid" I'm wondering if the notes for nextLong and nextInt should be updated to match?
Incorrect note in Javadoc for a few RandomGenerator methods
Hi, The Javadoc for RandomGenerator.nextLong(long origin, long bound) has this to say: Implementation Requirements:The default implementation checks that origin and bound are positive longs This doesn't seem to be true. The default implementation checks that origin and bound are a valid range (that bound >= origin). The implementation doesn't reject negative inputs. The same note appears in the two-arg version of nextInt. The note for e.g. nextDouble is correct and says "The default implementation verifies that the origin and bound are valid" I'm wondering if the notes for nextLong and nextInt should be updated to match?
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v8]
On Fri, 21 Jun 2024 02:10:59 GMT, lingjun-cg wrote: > If do that ,there is some performance degradation. Thanks for taking the time to do benchmarks. If that is the case, then lets stick with the proxy solution then. - PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2183118752
[jdk23] Integrated: 8334333: MissingResourceCauseTestRun.java fails if run by root
On Fri, 21 Jun 2024 00:49:33 GMT, SendaoYan wrote: > Hi all, > > This pull request contains a backport of commit > [de8ee977](https://github.com/openjdk/jdk/commit/de8ee97718d7e12b541b310cf5b67f3e10e91ad9) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by SendaoYan on 20 Jun 2024 and was > reviewed by Naoto Sato and Justin Lu. > > Thanks! This pull request has now been integrated. Changeset: 21514931 Author:SendaoYan Committer: Naoto Sato URL: https://git.openjdk.org/jdk/commit/215149310c49b01dee671afde7ad2da47ee3b8e4 Stats: 9 lines in 1 file changed: 6 ins; 0 del; 3 mod 8334333: MissingResourceCauseTestRun.java fails if run by root Reviewed-by: naoto Backport-of: de8ee97718d7e12b541b310cf5b67f3e10e91ad9 - PR: https://git.openjdk.org/jdk/pull/19817
Re: [jdk23] RFR: 8334333: MissingResourceCauseTestRun.java fails if run by root
On Fri, 21 Jun 2024 00:49:33 GMT, SendaoYan wrote: > Hi all, > > This pull request contains a backport of commit > [de8ee977](https://github.com/openjdk/jdk/commit/de8ee97718d7e12b541b310cf5b67f3e10e91ad9) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by SendaoYan on 20 Jun 2024 and was > reviewed by Naoto Sato and Justin Lu. > > Thanks! Thanks for the sponsor. - PR Comment: https://git.openjdk.org/jdk/pull/19817#issuecomment-2183087424
Integrated: 8334441: Mark tests in jdk_security_infra group as manual
On Thu, 20 Jun 2024 18:35:00 GMT, Rajan Halade wrote: > Updated all the tests that depend on external infrastructure services as > manual. These tests may fail with external reasons, for instance - change in > CA test portal, certificate status updates, or network issues. This pull request has now been integrated. Changeset: 8e1d2b09 Author:Rajan Halade URL: https://git.openjdk.org/jdk/commit/8e1d2b091c9a311d98a0b886a803fb18d4405d8a Stats: 160 lines in 10 files changed: 5 ins; 2 del; 153 mod 8334441: Mark tests in jdk_security_infra group as manual Reviewed-by: clanger, mullan - PR: https://git.openjdk.org/jdk/pull/19814
Re: [jdk23] RFR: 8334333: MissingResourceCauseTestRun.java fails if run by root
On Fri, 21 Jun 2024 00:49:33 GMT, SendaoYan wrote: > Hi all, > > This pull request contains a backport of commit > [de8ee977](https://github.com/openjdk/jdk/commit/de8ee97718d7e12b541b310cf5b67f3e10e91ad9) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by SendaoYan on 20 Jun 2024 and was > reviewed by Naoto Sato and Justin Lu. > > Thanks! Thanks for review. - PR Comment: https://git.openjdk.org/jdk/pull/19817#issuecomment-2183078007
Re: RFR: 8334441: Mark tests in jdk_security_infra group as manual [v2]
On Fri, 21 Jun 2024 16:11:34 GMT, Rajan Halade wrote: >> Updated all the tests that depend on external infrastructure services as >> manual. These tests may fail with external reasons, for instance - change in >> CA test portal, certificate status updates, or network issues. > > Rajan Halade has updated the pull request incrementally with one additional > commit since the last revision: > > fix typos Marked as reviewed by mullan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19814#pullrequestreview-2133029865
Re: [jdk23] RFR: 8334333: MissingResourceCauseTestRun.java fails if run by root
On Fri, 21 Jun 2024 00:49:33 GMT, SendaoYan wrote: > Hi all, > > This pull request contains a backport of commit > [de8ee977](https://github.com/openjdk/jdk/commit/de8ee97718d7e12b541b310cf5b67f3e10e91ad9) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by SendaoYan on 20 Jun 2024 and was > reviewed by Naoto Sato and Justin Lu. > > Thanks! Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19817#pullrequestreview-2133028593
Re: RFR: 8334441: Mark tests in jdk_security_infra group as manual [v2]
On Fri, 21 Jun 2024 13:10:00 GMT, Christoph Langer wrote: >> Rajan Halade has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix typos > > test/jdk/security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java > line 30: > >> 28: * @library /test/lib >> 29: * @build jtreg.SkippedException ValidatePathWithURL CAInterop >> 30: * @run main/othervm/manual/manual -Djava.security.debug=certpath,ocsp > > What is the reason why manual is written twice? (Here and in all other places > in this file...) good catch! I have fixed it. - PR Review Comment: https://git.openjdk.org/jdk/pull/19814#discussion_r1649163406
Re: RFR: 8334441: Mark tests in jdk_security_infra group as manual [v2]
> Updated all the tests that depend on external infrastructure services as > manual. These tests may fail with external reasons, for instance - change in > CA test portal, certificate status updates, or network issues. Rajan Halade has updated the pull request incrementally with one additional commit since the last revision: fix typos - Changes: - all: https://git.openjdk.org/jdk/pull/19814/files - new: https://git.openjdk.org/jdk/pull/19814/files/5f1889af..2fdaa890 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19814=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19814=00-01 Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/19814.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19814/head:pull/19814 PR: https://git.openjdk.org/jdk/pull/19814
RFR: 8334734: Remove specialized readXxxEntry methods from ClassReader
`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. - Commit messages: - Copyright years - 8334734: Remove specialized readXxxEntry methods from ClassReader Changes: https://git.openjdk.org/jdk/pull/19833/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19833=00 Issue: https://bugs.openjdk.org/browse/JDK-8334734 Stats: 169 lines in 9 files changed: 1 ins; 111 del; 57 mod Patch: https://git.openjdk.org/jdk/pull/19833.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19833/head:pull/19833 PR: https://git.openjdk.org/jdk/pull/19833
Re: RFR: 8334287: Man page update for jstatd deprecation
On Fri, 21 Jun 2024 14:05:51 GMT, Kevin Walls wrote: > Man page update for JDK-8327793 which marked jstatd as deprecated for removal > in JDK 24. The updated man page looks like: JSTATD(1) JDK CommandsJSTATD(1) NAME jstatd - monitor the creation and termination of instrumented Java HotSpot VMs SYNOPSIS WARNING: This command is deprecated and will be removed in a future release. Note: This command is experimental and unsupported. jstatd [options] ...etc... - PR Comment: https://git.openjdk.org/jdk/pull/19829#issuecomment-2182976031
Re: RFR: 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. Looks good to me, thanks. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19832#pullrequestreview-2132887472
RFR: 8334726: Remove accidentally exposed individual methods from Class-File API
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. - Commit messages: - 8334726: Remove accidentally exposed individual methods from Class-File API Changes: https://git.openjdk.org/jdk/pull/19832/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19832=00 Issue: https://bugs.openjdk.org/browse/JDK-8334726 Stats: 37 lines in 7 files changed: 0 ins; 30 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/19832.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19832/head:pull/19832 PR: https://git.openjdk.org/jdk/pull/19832
RFR: 8334397: RISC-V: verify perf of ReverseBytesS/US
Hi, Can you help to review this patch? Thanks! This is similar with previous JDK-8334396. Added some tests. ### Test | Tests | Scores | Errors | Unit -- | -- | -- | -- | -- Intrinsic, +zbb, +rvv | Characters.reverseBytes | 1654.535 | 69.36 | ns/op | Shorts.reverseBytes | 1795.403 | 44.015 | ns/op Intrinsic, +zbb, -rvv | Characters.reverseBytes | 1649.752 | 74.965 | ns/op | Shorts.reverseBytes | 1798.637 | 49.52 | ns/op Intrinsic, -zbb, +rvv | Characters.reverseBytes | 2279.588 | 44.222 | ns/op | Shorts.reverseBytes | 2441.674 | 63.895 | ns/op Intrinsic, -zbb, -rvv | Characters.reverseBytes | 2288.876 | 49.099 | ns/op | Shorts.reverseBytes | 2454.454 | 94.004 | ns/op No intrinsic | Characters.reverseBytes | 1629.722 | 23.656 | ns/op | Shorts.reverseBytes | 2108.81 | 43.378 | ns/op - Commit messages: - Initial commit Changes: https://git.openjdk.org/jdk/pull/19830/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19830=00 Issue: https://bugs.openjdk.org/browse/JDK-8334397 Stats: 129 lines in 4 files changed: 98 ins; 31 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19830.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19830/head:pull/19830 PR: https://git.openjdk.org/jdk/pull/19830
Integrated: 8333867: SHA3 performance can be improved
On Mon, 10 Jun 2024 15:01:55 GMT, Ferenc Rakoczi wrote: > This PR removes some unnecessary conversions between byte arrays and long > arrays during SHA3 digest computations. This pull request has now been integrated. Changeset: 75bea280 Author:Ferenc Rakoczi Committer: Weijun Wang URL: https://git.openjdk.org/jdk/commit/75bea280b9adb6dac9fefafbb3f4b212f100fbb5 Stats: 86 lines in 3 files changed: 24 ins; 35 del; 27 mod 8333867: SHA3 performance can be improved Reviewed-by: kvn, valeriep - PR: https://git.openjdk.org/jdk/pull/19632
Re: RFR: 8334708: FFM: two javadoc problems
On Fri, 21 Jun 2024 07:40:31 GMT, Hannes Greule wrote: > Addresses two simple problems regarding javadocs in the FFM API. I think we can safely backport this to 23, as this is a doc-only fix (I've labeled the JBS issue noreg-doc as well) allowed by JEP-3 for RDP 1 and 2. - PR Comment: https://git.openjdk.org/jdk/pull/19820#issuecomment-2182836082
RFR: 8334287: Man page update for jstatd deprecation
Man page update for JDK-8327793 which marked jstatd as deprecated for removal in JDK 24. - Commit messages: - 8334287: Man page update for jstatd deprecation Changes: https://git.openjdk.org/jdk/pull/19829/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19829=00 Issue: https://bugs.openjdk.org/browse/JDK-8334287 Stats: 4 lines in 1 file changed: 3 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19829.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19829/head:pull/19829 PR: https://git.openjdk.org/jdk/pull/19829
Re: RFR: 8334708: FFM: two javadoc problems
On Fri, 21 Jun 2024 07:40:31 GMT, Hannes Greule wrote: > Addresses two simple problems regarding javadocs in the FFM API. Thank you for your review. > Should we backport to 23? I leave that up to you, I don't know how those things are handled normally. But a backport would be very simple in this case. - PR Comment: https://git.openjdk.org/jdk/pull/19820#issuecomment-2182825539
Integrated: 8327793: Deprecate jstatd for removal
On Tue, 11 Jun 2024 13:09:06 GMT, Kevin Walls wrote: > jstatd is an RMI server application which monitors HotSpot VMs, and provides > an interface to the monitoring tool jstat, for use across a remote RMI > connection. > > RMI is not how modern applications communicate. It is an old transport with > long term security concerns, and configuration difficulties with firewalls. > > The jstatd tool should be removed. Deprecating and removing jstatd will not > affect usage of jstat for monitoring local VMs using the Attach API. This pull request has now been integrated. Changeset: 9f8de221 Author:Kevin Walls URL: https://git.openjdk.org/jdk/commit/9f8de221d7f0186718411ab3f5217e3883237e84 Stats: 6 lines in 3 files changed: 3 ins; 0 del; 3 mod 8327793: Deprecate jstatd for removal Reviewed-by: alanb, cjplummer - PR: https://git.openjdk.org/jdk/pull/19658
Re: RFR: 8334653: ISO 4217 Amendment 177 Update [v3]
On Thu, 20 Jun 2024 20:12:39 GMT, Justin Lu wrote: >> Please review this PR which incorporates the ISO 4217 Amendment 177 Update. >> >> Specifically, the introduction of the new currency, Zimbabwe Gold. > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > reflect review: change txt name Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19813#pullrequestreview-2132659325
Re: RFR: 8327793: Deprecate jstatd for removal [v7]
On Fri, 21 Jun 2024 13:13:38 GMT, Kevin Walls wrote: >> jstatd is an RMI server application which monitors HotSpot VMs, and provides >> an interface to the monitoring tool jstat, for use across a remote RMI >> connection. >> >> RMI is not how modern applications communicate. It is an old transport with >> long term security concerns, and configuration difficulties with firewalls. >> >> The jstatd tool should be removed. Deprecating and removing jstatd will not >> affect usage of jstat for monitoring local VMs using the Attach API. > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > caps /INTEGRATE - PR Comment: https://git.openjdk.org/jdk/pull/19658#issuecomment-2182738142
Re: RFR: 8334441: Mark tests in jdk_security_infra group as manual
On Thu, 20 Jun 2024 18:35:00 GMT, Rajan Halade wrote: > Updated all the tests that depend on external infrastructure services as > manual. These tests may fail with external reasons, for instance - change in > CA test portal, certificate status updates, or network issues. Looks good, although I don't see why sometimes the directive is @run main/othervm/manual/manual (double occurence of manual). test/jdk/security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java line 30: > 28: * @library /test/lib > 29: * @build jtreg.SkippedException ValidatePathWithURL CAInterop > 30: * @run main/othervm/manual/manual -Djava.security.debug=certpath,ocsp What is the reason why manual is written twice? (Here and in all other places in this file...) - Marked as reviewed by clanger (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19814#pullrequestreview-2132623582 PR Review Comment: https://git.openjdk.org/jdk/pull/19814#discussion_r1648943324
Re: RFR: 8327793: Deprecate jstatd for removal [v7]
> jstatd is an RMI server application which monitors HotSpot VMs, and provides > an interface to the monitoring tool jstat, for use across a remote RMI > connection. > > RMI is not how modern applications communicate. It is an old transport with > long term security concerns, and configuration difficulties with firewalls. > > The jstatd tool should be removed. Deprecating and removing jstatd will not > affect usage of jstat for monitoring local VMs using the Attach API. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: caps - Changes: - all: https://git.openjdk.org/jdk/pull/19658/files - new: https://git.openjdk.org/jdk/pull/19658/files/1a240155..db53d078 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19658=06 - incr: https://webrevs.openjdk.org/?repo=jdk=19658=05-06 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19658.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19658/head:pull/19658 PR: https://git.openjdk.org/jdk/pull/19658
Re: RFR: 8327793: Deprecate jstatd for removal [v4]
On Fri, 21 Jun 2024 12:28:20 GMT, Alan Bateman wrote: >> For the Security Manager, the warning was worded a little differently: >> >> "WARNING: The Security Manager is deprecated and will be removed in a future >> release" >> >> I think that wording is more clear. The current wording could be confusing >> because JDK 24 is when jstatd is deprecated for removal, and not a future >> release. The wording above is more definitive for those that may not >> understand what "deprecated for removal" means. Thus, I suggest: >> >> "WARNING: jstatd is deprecated and will be removed in a future release" > > @kevinjwalls Are you planning to move to "WARNING" as has been suggested in > the previous comments. No, I was just pausing while the pre-submit checks ran again. I did investigate, the JDK seems fairly evenly split on WARNING vs Warning. This message should not be around for many releases. - PR Review Comment: https://git.openjdk.org/jdk/pull/19658#discussion_r1648922812
Re: RFR: 8317611: Add a tool like jdeprscan to find usage of restricted methods [v5]
On Thu, 20 Jun 2024 19:37:23 GMT, Jorn Vernee wrote: > I've massaged the parsing code to where the help message now looks like this: It is better but it might mean looking at HelpFormatter as you mentioned,. Right now the usage message `--add-modules ` whereas the java --help will print `--add-modules [,...]` and javac --help will print `--add-modules (,)*`. Not super important for now, I think the main thing with this PR is getting agreement to add a tool with a short shelf life. It's clear there is a longer term story here to have one tool to do static analysis and replace (or front) `jdeps`, `jdeprscan` and this the new tool here. - PR Comment: https://git.openjdk.org/jdk/pull/19774#issuecomment-2182708610
Re: RFR: 8334714: Class-File API leaves preview
On Fri, 21 Jun 2024 12:31:05 GMT, Alan Bateman wrote: > Looks like this has been accidentally created as a sub-task of the JEP issue, > I assume you'll fix that. Will there be another issue to update the tests and > drop `@enablePreview`? I thought the implementation is usually created as a subtask of JEP, however I can make it a standalone bug if neede. Yes, there will be follow up task to clean all the `@enablePreview` mess, I'll create it. - PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2182673557
Re: RFR: 8334714: Class-File API leaves preview
On Fri, 21 Jun 2024 11:56:37 GMT, Adam Sotona wrote: > Class-File API is leaving preview. > This is a removal of all `@PreviewFeature` annotations from Class-File API. > It also bumps all `@since` tags and removes > `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`. > > Please review. > > Thanks, > Adam Any potential API changes should be merged first, to avoid impression of changing a non-preview API. However any discussion and approval processes of API and documentation changes should not block the JEP process (and each other). Cleaning up preview-enabled tests has a lower priority and should follow. - PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-218256
Re: RFR: 8334714: Class-File API leaves preview
On Fri, 21 Jun 2024 11:56:37 GMT, Adam Sotona wrote: > Class-File API is leaving preview. > This is a removal of all `@PreviewFeature` annotations from Class-File API. > It also bumps all `@since` tags and removes > `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`. > > Please review. > > Thanks, > Adam Looks like this has been accidentally created as a sub-task of the JEP issue, I assume you'll fix that. Will there be another issue to update the tests and drop `@enablePreview`? - PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2182663458
Re: RFR: 8327793: Deprecate jstatd for removal [v4]
On Tue, 11 Jun 2024 21:09:14 GMT, Sean Mullan wrote: >> I also think a period at the end is not necessary. > > For the Security Manager, the warning was worded a little differently: > > "WARNING: The Security Manager is deprecated and will be removed in a future > release" > > I think that wording is more clear. The current wording could be confusing > because JDK 24 is when jstatd is deprecated for removal, and not a future > release. The wording above is more definitive for those that may not > understand what "deprecated for removal" means. Thus, I suggest: > > "WARNING: jstatd is deprecated and will be removed in a future release" @kevinjwalls Are you planning to move to "WARNING" as has been suggested in the previous comments. - PR Review Comment: https://git.openjdk.org/jdk/pull/19658#discussion_r1648897498
Re: RFR: 8334714: Class-File API leaves preview
On Fri, 21 Jun 2024 11:56:37 GMT, Adam Sotona wrote: > Class-File API is leaving preview. > This is a removal of all `@PreviewFeature` annotations from Class-File API. > It also bumps all `@since` tags and removes > `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`. > > Please review. > > Thanks, > Adam Do we intend to do it after we finish our planned API changes or as soon as possible? Also we might need to look at the preview-enabled test treatments. - PR Comment: https://git.openjdk.org/jdk/pull/19826#issuecomment-2182652273
RFR: 8334714: Class-File API leaves preview
Class-File API is leaving preview. This is a removal of all `@PreviewFeature` annotations from Class-File API. It also bumps all `@since` tags and removes `jdk.internal.javac.PreviewFeature.Feature.CLASSFILE_API`. Please review. Thanks, Adam - Commit messages: - bumped @since tag - 8334714: Class-File API leaves preview Changes: https://git.openjdk.org/jdk/pull/19826/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19826=00 Issue: https://bugs.openjdk.org/browse/JDK-8334714 Stats: 718 lines in 166 files changed: 0 ins; 479 del; 239 mod Patch: https://git.openjdk.org/jdk/pull/19826.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19826/head:pull/19826 PR: https://git.openjdk.org/jdk/pull/19826
Re: [jdk23] RFR: 8333854: IllegalAccessError with proxies after JDK-8332457
On Thu, 20 Jun 2024 20:11:06 GMT, Chen Liang wrote: > Please review this patch, which is a backport of the fix in #19615 to JDK 23. > > This is not a clean patch, because the old patch was done on JDK-8333479 > (#19585) which was absent in JDK 23; however, the conflicts were small, and > the only real changes were that `methodTypeDesc` and `classDesc` from > `ConstantUtils` were not available, so the original approaches were retained. > There is also import adjustments. > > Testing: tier 1-3 It looks good to me, thanks. - Marked as reviewed by asotona (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19815#pullrequestreview-2132482150
Re: RFR: 8327793: Deprecate jstatd for removal [v6]
> jstatd is an RMI server application which monitors HotSpot VMs, and provides > an interface to the monitoring tool jstat, for use across a remote RMI > connection. > > RMI is not how modern applications communicate. It is an old transport with > long term security concerns, and configuration difficulties with firewalls. > > The jstatd tool should be removed. Deprecating and removing jstatd will not > affect usage of jstat for monitoring local VMs using the Attach API. Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: wording - Changes: - all: https://git.openjdk.org/jdk/pull/19658/files - new: https://git.openjdk.org/jdk/pull/19658/files/3c7e86a1..1a240155 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19658=05 - incr: https://webrevs.openjdk.org/?repo=jdk=19658=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19658.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19658/head:pull/19658 PR: https://git.openjdk.org/jdk/pull/19658
Re: RFR: 8327793: Deprecate jstatd for removal [v5]
> jstatd is an RMI server application which monitors HotSpot VMs, and provides > an interface to the monitoring tool jstat, for use across a remote RMI > connection. > > RMI is not how modern applications communicate. It is an old transport with > long term security concerns, and configuration difficulties with firewalls. > > The jstatd tool should be removed. Deprecating and removing jstatd will not > affect usage of jstat for monitoring local VMs using the Attach API. Kevin Walls 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 10 additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into 8327793_jstatd_deprecate - Remove annotations - Replace (C) - Remove annotations - No annotations for src/jdk.jstatd/share/classes/sun/jvmstat/monitor/remote - Remove tmp file - Annotations - Annotations - Merge remote-tracking branch 'upstream/master' into 8327793_jstatd_deprecate - Basic deprecation of module, jstatd prints warning. - Changes: - all: https://git.openjdk.org/jdk/pull/19658/files - new: https://git.openjdk.org/jdk/pull/19658/files/ed386505..3c7e86a1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19658=04 - incr: https://webrevs.openjdk.org/?repo=jdk=19658=03-04 Stats: 27075 lines in 733 files changed: 18445 ins; 5705 del; 2925 mod Patch: https://git.openjdk.org/jdk/pull/19658.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19658/head:pull/19658 PR: https://git.openjdk.org/jdk/pull/19658
Re: RFR: 8334328: Reduce object allocation for FloatToDecimal and DoubleToDecimal [v8]
On Sun, 16 Jun 2024 21:00:41 GMT, Claes Redestad wrote:
>> Shaojin Wen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> code format
>
> src/java.base/share/classes/jdk/internal/math/ToDecimal.java line 171:
>
>> 169: /* Using the deprecated constructor enhances performance */
>> 170: @SuppressWarnings("deprecation")
>> 171: static String charsToString(byte[] str, int index) {
>
> This should be named something like `asciiBytesToString` now. Perhaps
> `ToDecimal.LATIN1` can be renamed `ASCII`, too.
In JDK internal string processing, LATIN1 and UTF16 appear in pairs, so I think
LATIN1 is better.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19730#discussion_r1648846026
[jdk23] Integrated: 8333344: JMX attaching of Subject does not work when security manager not allowed
On Thu, 20 Jun 2024 15:24:35 GMT, Kevin Walls wrote: > 844: JMX attaching of Subject does not work when security manager not > allowed This pull request has now been integrated. Changeset: 23f2c97f Author:Kevin Walls URL: https://git.openjdk.org/jdk/commit/23f2c97f4ce42316fd51290175b01b41bcc24f6b Stats: 162 lines in 17 files changed: 113 ins; 3 del; 46 mod 844: JMX attaching of Subject does not work when security manager not allowed Reviewed-by: dfuchs Backport-of: bcf4bb4882e06d8c52f6eb4e9c4e027ba0622c5f - PR: https://git.openjdk.org/jdk/pull/19810
Re: [jdk23] RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed
On Thu, 20 Jun 2024 15:24:35 GMT, Kevin Walls wrote: > 844: JMX attaching of Subject does not work when security manager not > allowed LGTM - Marked as reviewed by dfuchs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19810#pullrequestreview-2132226211
Re: [jdk23] RFR: 8333344: JMX attaching of Subject does not work when security manager not allowed
On Thu, 20 Jun 2024 15:24:35 GMT, Kevin Walls wrote: > 844: JMX attaching of Subject does not work when security manager not > allowed Thanks Daniel. Testing automated and manual looks good in 24 and in this 23 backport, so I'll get this integrated. - PR Comment: https://git.openjdk.org/jdk/pull/19810#issuecomment-2182392102
Re: RFR: 8334708: FFM: two javadoc problems
On Fri, 21 Jun 2024 07:40:31 GMT, Hannes Greule wrote: > Addresses two simple problems regarding javadocs in the FFM API. Thanks! Should we backport to 23? - Marked as reviewed by mcimadamore (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19820#pullrequestreview-2132155399
RFR: 8334708: FFM: two javadoc problems
Addresses two simple problems regarding javadocs in the FFM API. - Commit messages: - fix two javadoc problems in FFM Changes: https://git.openjdk.org/jdk/pull/19820/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19820=00 Issue: https://bugs.openjdk.org/browse/JDK-8334708 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/19820.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19820/head:pull/19820 PR: https://git.openjdk.org/jdk/pull/19820
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v10]
On Fri, 21 Jun 2024 07:25:27 GMT, lingjun-cg wrote:
>> ### Performance regression of DecimalFormat.format
>> From the output of perf, we can see the hottest regions contain atomic
>> instructions. But when run with JDK 11, there is no such problem. The
>> reason is the removed biased locking.
>> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
>> contains many synchronized methods.
>> So I added support for some new methods that accept StringBuilder which is
>> lock-free.
>>
>> ### Benchmark testcase
>>
>> @BenchmarkMode(Mode.AverageTime)
>> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class JmhDecimalFormat {
>>
>> private DecimalFormat format;
>>
>> @Setup(Level.Trial)
>> public void setup() {
>> format = new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testNewAndFormat() throws InterruptedException {
>> new DecimalFormat("#0.0").format(9524234.1236457);
>> }
>>
>> @Benchmark
>> public void testNewOnly() throws InterruptedException {
>> new DecimalFormat("#0.0");
>> }
>>
>> @Benchmark
>> public void testFormatOnly() throws InterruptedException {
>> format.format(9524234.1236457);
>> }
>> }
>>
>>
>> ### Test result
>> Current JDK before optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>>
>>
>>
>> Current JDK after optimize
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>>
>>
>> ### JDK 11
>>
>> Benchmark Mode CntScore Error Units
>> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
>> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
>> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
>
> lingjun-cg has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 896: Performance regression of DecimalFormat.format
Updates:
1. Add package private interface `StringBuf` into `Format` as an inner class.
Like `java.text.Format.FieldDelegate` that used by `Format` implemetations.
2. Add `boolean isInternalSubclass` into all `Format` implemetations that in
package `java.text`.
-
PR Comment: https://git.openjdk.org/jdk/pull/19513#issuecomment-2182167568
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v10]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/9a6f646f..67c724c7
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=09
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=08-09
Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v9]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/1fde6a60..9a6f646f
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=19513=08
- incr: https://webrevs.openjdk.org/?repo=jdk=19513=07-08
Stats: 348 lines in 11 files changed: 198 ins; 65 del; 85 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
