Re: RFR: 8352015: LIBVERIFY_OPTIMIZATION remove special optimization settings
On Fri, 14 Mar 2025 11:15:26 GMT, Matthias Baesken wrote: > On Linux there are some special settings for LIBVERIFY_OPTIMIZATION that are > most likely not needed any more and could be removed. > The removal (on Linux) brings the lib optimization level de facto from LOW to > HIGH. Example lib sizes in jdk head opt build on Linux x86_64 with gcc 13.2.0 are 68K without the patch and 72K with the patch, so fortunately no large size increase is seen. - PR Comment: https://git.openjdk.org/jdk/pull/24054#issuecomment-2724370568
RFR: 8352064: AIX: now also able to build static-jdk image with a statically linked launcher
After "JDK-8339480: Build static-jdk image with a statically linked launcher" AIX was not able to build the new target. Therefore with "JDK-8345590 AIX 'make all' fails after JDK-8339480" the new target was disabled again. Now with this change we can enable the statically linked launcher target again. There are 3 things to do. 1. Modify `dladdr()`. Because this API does not exist on AIX it is implemented based on the `loadquery()` API. Unfortunately, this API does only return the name of the executable, but not its path. Beforehand this was no problem, because we asked for a loaded library, for which the API provides the path. But now we are asking for the executable itself. 2. `dladdr()` is differently implemented three times in the openjdk code. In the static case I supressed now the usage of the additional modules containing version two and three. I know this shouldn't be the final version. Magnus mentioned that they have discussed from time to time to have a "basic JDK utility lib" that can be shared between hotspot and the JDK libraries. I think this is a good idea for the future, but far beyond the scope of this PR. The second best thing Magnus proposed is to only have the `dladdr()` functionality in Hotspot and then export it. Let's see how the community decides. 3. Because we lack a linker flag like `whole-archive`, I had to force the export of all symbols by creating export files containing all symbols of the static libs. I introduced the new rule for the export file generation as "raw" make recipes. Magnus claimed to use the `SetupExecute`. Unfortunately I was not able to make it function. So I still have my "raw" solution in place, but my last try with `SetupExecute` as comment beneath. Help is welcome. - Commit messages: - JDK-8352064 Changes: https://git.openjdk.org/jdk/pull/24062/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24062&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8352064 Stats: 66 lines in 5 files changed: 60 ins; 3 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/24062.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24062/head:pull/24062 PR: https://git.openjdk.org/jdk/pull/24062
Re: RFR: 8352015: LIBVERIFY_OPTIMIZATION remove special optimization settings
On Fri, 14 Mar 2025 11:34:08 GMT, Magnus Ihse Bursie wrote: > What testing have you run? I put it into our testing queue, this runs jtreg tier 1 - 4 and some more stuff over the weekend on all our platforms. - PR Comment: https://git.openjdk.org/jdk/pull/24054#issuecomment-2724577126
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public
On Fri, 14 Mar 2025 13:38:51 GMT, Alan Bateman wrote:
>> Alternative approach to #24012
>>
>> This keeps the current handling of *.pdb vs *.stripped.pdb which allows
>> debugging at the cost of a little hack in jlink. Maybe the code in jlink can
>> be improved, e.g. make it more conditional.
>>
>> I'm running this through our testing still to see whether it'll resolve all
>> of the test issues and does not introduce regressions.
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 226:
>
>> 224: if (Files.exists(strippedPath)) {
>> 225: path = strippedPath;
>> 226: }
>
> This is super weird to put here. I could imagine introducing a Windows
> specific plugin or option for jlink to help with this setup but I don't think
> we should rush into a change to JRTArchive to workaround this.
A jlink plugin sounds like a reasonable approach to me.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/24057#discussion_r1995629113
Re: RFR: 8352044: Add --with-import-jvms to configure
On Fri, 14 Mar 2025 17:58:50 GMT, Magnus Ihse Bursie wrote: >> This change copies `libjvm.so` _and_ sibling `.jsa` files, right? >> >> If so, then one thing is missing: regenerating CDS archives that have >> opinions on `modules` filesizes/dates for fingerprinting their CDS archives. >> My frankensteining scripts do that by invoking new JVM explicitly with >> `-Xshare:dump`. But build system should already know how to do that, as it >> does it at the end of the build. >> >> Try to import a JVM and do `java -Xshare:on Hello`? > >> Try to import a JVM and do `java -Xshare:on Hello`? > > The problem is that we can't really count on -Xshare being supported on an > imported JVM. @magicus I'm trying to see the big picture here of all the changes you are proposing and I'm really not seeing it. This seems to be making it more difficult to create a JDK/JRE with multiple VMs. - PR Comment: https://git.openjdk.org/jdk/pull/24063#issuecomment-2725744927
RFR: 8352084: Add more test code in TestSetupAOT.java
I modified TestSetupAOT.java to exercise more functionalities in the JDK so that we can have a more substantial AOT cache when running tests with `AOT_JDK=true`. E.g: make test JTREG=AOT_JDK=true \ TEST=open/test/jdk/java/util/TimeZone/ListTimeZones.java Before: the generated AOT cache was about 20 MB, with 2245 classes and 125 resolved indies After: the generated AOT cache is about 34 MB, with 4703 classes and 912 resolved indies I verified with Mach5 tiers 4, 5, 6, 10 with all tests tasks that have the label `.*aot.jdkcache.*` - Commit messages: - 8352084: Add more test code in TestSetupAOT.java Changes: https://git.openjdk.org/jdk/pull/24067/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24067&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8352084 Stats: 172 lines in 5 files changed: 158 ins; 0 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/24067.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24067/head:pull/24067 PR: https://git.openjdk.org/jdk/pull/24067
Re: RFR: 8351542: LIBMANAGEMENT_OPTIMIZATION remove special optimization settings [v5]
On Thu, 13 Mar 2025 17:41:28 GMT, Kevin Walls wrote: > Yes seems likely the same history for libverify, so should be good to do the > same update there. I created https://bugs.openjdk.org/browse/JDK-8352015 . - PR Comment: https://git.openjdk.org/jdk/pull/23966#issuecomment-2723988627
Re: RFR: JDK-8348598 : Update Libpng to 1.6.47 [v3]
On Fri, 14 Mar 2025 02:02:58 GMT, Sergey Bylokhov wrote: >> make/modules/java.desktop/lib/ClientLibraries.gmk line 249: >> >>> 247: DISABLED_WARNINGS_clang_dgif_lib.c := sign-compare, \ >>> 248: DISABLED_WARNINGS_clang_gzwrite.c := format-nonliteral, \ >>> 249: DISABLED_WARNINGS_clang_png.c := unused-function, \ >> >> This warning was disabled to fix the following build issue on linux and >> macOS. >> >> >> open/src/java.desktop/share/native/libsplashscreen/libpng/png.c:1558:1: >> error: 'png_icc_profile_error' defined but not used [-Werror=unused-function] >> [2025-03-12T23:57:44,575Z] 1558 | png_icc_profile_error(png_const_structrp >> png_ptr, png_const_charp name, >> [2025-03-12T23:57:44,575Z] | ^ > > Might this be a bug in PNG? All usages of this function are now guarded by > "PNG_READ_iCCP_SUPPORTED"? It would be useful to report this upstream. @mrserb I see, what you meant is that the function `png_icc_profile_error` should be guarded with PNG_READ_iCCP_SUPPORTED instead of PNG_iCCP_SUPPORTED. Reported it to upstream - https://github.com/pnggroup/libpng/issues/668 - PR Review Comment: https://git.openjdk.org/jdk/pull/24021#discussion_r1996304716
Re: RFR: 8352084: Add more test code in TestSetupAOT.java
On Fri, 14 Mar 2025 20:13:57 GMT, Ioi Lam wrote: > I modified TestSetupAOT.java to exercise more functionalities in the JDK so > that we can have a more substantial AOT cache when running tests with > `AOT_JDK=true`. E.g: > > > make test JTREG=AOT_JDK=true \ > TEST=open/test/jdk/java/util/TimeZone/ListTimeZones.java > > > Before: the generated AOT cache was about 20 MB, with 2245 classes and 125 > resolved indies > After: the generated AOT cache is about 34 MB, with 4703 classes and 912 > resolved indies > > I verified with Mach5 tiers 4, 5, 6, 10 with all tests tasks that have the > label `.*aot.jdkcache.*` Good. - Marked as reviewed by kvn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24067#pullrequestreview-2687063032
Re: RFR: 8352084: Add more test code in TestSetupAOT.java
On Fri, 14 Mar 2025 20:13:57 GMT, Ioi Lam wrote:
> I modified TestSetupAOT.java to exercise more functionalities in the JDK so
> that we can have a more substantial AOT cache when running tests with
> `AOT_JDK=true`. E.g:
>
>
> make test JTREG=AOT_JDK=true \
> TEST=open/test/jdk/java/util/TimeZone/ListTimeZones.java
>
>
> Before: the generated AOT cache was about 20 MB, with 2245 classes and 125
> resolved indies
> After: the generated AOT cache is about 34 MB, with 4703 classes and 912
> resolved indies
>
> I verified with Mach5 tiers 4, 5, 6, 10 with all tests tasks that have the
> label `.*aot.jdkcache.*`
Changes requested by lmesnik (Reviewer).
make/RunTests.gmk line 738:
> 736: $1_AOT_JDK_CACHE := $$($1_TEST_SUPPORT_DIR)/aot/jdk.aotcache
> 737: $1_AOT_JDK_LOG := $$($1_TEST_SUPPORT_DIR)/aot/TestSetupAOT.log
> 738: $1_AOT_JDK_OUTPUT_DIR := $$($1_TEST_SUPPORT_DIR)/aot
Better to move it before line 735, and reuse in other definitions instead of
$$($1_TEST_SUPPORT_DIR)/aot
test/setup_aot/TestSetupAOT.java line 147:
> 145:
> 146:
> 147: static void streamOps(String args[]) {
Can you please add a comment with explanation of purpose of 'streamOps' method.
test/setup_aot/TestSetupAOT.java line 201:
> 199: String CSCC = "string" + s + "string" + c;
> 200:
> 201: long l = System.currentTimeMillis();
the l should be logged and allow to be defined using property.
So any testing could be repeated.
long l = Long.getLong("test.aot.seed", System.currentTimeMillis());
-
PR Review: https://git.openjdk.org/jdk/pull/24067#pullrequestreview-2687055469
PR Review Comment: https://git.openjdk.org/jdk/pull/24067#discussion_r1996419066
PR Review Comment: https://git.openjdk.org/jdk/pull/24067#discussion_r1996422306
PR Review Comment: https://git.openjdk.org/jdk/pull/24067#discussion_r1996424173
Re: RFR: 8352084: Add more test code in TestSetupAOT.java
On Fri, 14 Mar 2025 20:13:57 GMT, Ioi Lam wrote: > I modified TestSetupAOT.java to exercise more functionalities in the JDK so > that we can have a more substantial AOT cache when running tests with > `AOT_JDK=true`. E.g: > > > make test JTREG=AOT_JDK=true \ > TEST=open/test/jdk/java/util/TimeZone/ListTimeZones.java > > > Before: the generated AOT cache was about 20 MB, with 2245 classes and 125 > resolved indies > After: the generated AOT cache is about 34 MB, with 4703 classes and 912 > resolved indies > > I verified with Mach5 tiers 4, 5, 6, 10 with all tests tasks that have the > label `.*aot.jdkcache.*` make/test/BuildTestSetupAOT.gmk line 62: > 60: JavacBenchApp$$FileManager.class \ > 61: JavacBenchApp$$SourceFile.class \ > 62: TestSetupAOT.class, \ Listing all class files like this is cumbersome. Perhaps we should build this into a jar file instead now that there are more classes? It should be as easy as adding `JAR := ` to the SetupJavaCompilation call above and then copy that file. Or would that interfere with the classpath requirements when using the tool? - PR Review Comment: https://git.openjdk.org/jdk/pull/24067#discussion_r1996434862
Re: RFR: 8352064: AIX: now also able to build static-jdk image with a statically linked launcher
On Fri, 14 Mar 2025 15:41:56 GMT, Joachim Kern wrote:
> After "JDK-8339480: Build static-jdk image with a statically linked launcher"
> AIX was not able to build the new target. Therefore with "JDK-8345590 AIX
> 'make all' fails after JDK-8339480" the new target was disabled again.
>
> Now with this change we can enable the statically linked launcher target
> again.
> There are 3 things to do.
> 1.Modify `dladdr()`. Because this API does not exist on AIX it is
> implemented based on the `loadquery()` API. Unfortunately, this API does only
> return the name of the executable, but not its path. Beforehand this was no
> problem, because we asked for a loaded library, for which the API provides
> the path. But now we are asking for the executable itself.
> 2.`dladdr()` is differently implemented three times in the openjdk code.
> In the static case I supressed now the usage of the additional modules
> containing version two and three. I know this shouldn't be the final version.
> Magnus mentioned that they have discussed from time to time to have a "basic
> JDK utility lib" that can be shared between hotspot and the JDK libraries. I
> think this is a good idea for the future, but far beyond the scope of this
> PR. The second best thing Magnus proposed is to only have the `dladdr()`
> functionality in Hotspot and then export it. Let's see how the community
> decides.
> 3.Because we lack a linker flag like `whole-archive`, I had to force the
> export of all symbols by creating export files containing all symbols of the
> static libs. I introduced the new rule for the export file generation as
> "raw" make recipes. Magnus claimed to use the `SetupExecute`. Unfortunately I
> was not able to make it function. So I still have my "raw" solution in place,
> but my last try with `SetupExecute` as comment beneath. Help is welcome.
make/StaticLibs.gmk line 116:
> 114: #DEPS := $(STATIC_LIB_FILE), \
> 115: #OUTPUT_FILE := $(STATIC_LIB_EXPORT_FILES), \
> 116: #COMMAND := $(AR) $(ARFLAGS) -w $(patsubst %.exp, %, $@) | $(GREP)
> -v '^\.' | $(AWK) '{print $$1}' | sort -u >$@, \
The problem with using SetupExecute here is using `$@` variables in the command
line. To get this to work you need to delay resolving such variables. It may
work by adding an adequate number of extra `$`, but I'm not sure how many would
be needed or if it would actually work. You could try this, but it's probably
trickier to get right than just doubling:
Suggestion:
#COMMAND := $(AR) $(ARFLAGS) -w $$(patsubst %.exp, %, $$@) | $(GREP) -v
'^.' | $(AWK) '{print 1}' | sort -u >$$@, \
You are probably better off using the explicit output file variable
`$(STATIC_LIB_EXPORT_FILES)`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/24062#discussion_r1996001131
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public
On Fri, 14 Mar 2025 12:29:22 GMT, Christoph Langer wrote:
> Alternative approach to #24012
>
> This keeps the current handling of *.pdb vs *.stripped.pdb which allows
> debugging at the cost of a little hack in jlink. Maybe the code in jlink can
> be improved, e.g. make it more conditional.
>
> I'm running this through our testing still to see whether it'll resolve all
> of the test issues and does not introduce regressions.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 226:
> 224: if (Files.exists(strippedPath)) {
> 225: path = strippedPath;
> 226: }
This is super weird to put here. I could imagine introducing a Windows specific
plugin or option for jlink to help with this setup but I don't think we should
rush into a change to JRTArchive to workaround this.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/24057#discussion_r1995601023
Re: RFR: 8351821: VMManagementImpl.c avoid switching off warnings
On Fri, 14 Mar 2025 10:06:32 GMT, Matthias Baesken wrote: > We switched off unused-variable warnings for the file VMManagementImpl.c, > this should be avoided. > This came up in [JDK-8351542](https://bugs.openjdk.org/browse/JDK-8351542) , We know GetOptionalSupport can't realistically fail, unless we call it with a bad pointer (we don't). This location is called once only on startup. We have not checked the return value since introduced in jdk5... So can't really object to not saving that value in a temp that we ignore, and simplifying the Makefile a little! - Marked as reviewed by kevinw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24052#pullrequestreview-2686805647
Re: RFR: 8351821: VMManagementImpl.c avoid switching off warnings
On Fri, 14 Mar 2025 10:06:32 GMT, Matthias Baesken wrote: > We switched off unused-variable warnings for the file VMManagementImpl.c, > this should be avoided. > This came up in [JDK-8351542](https://bugs.openjdk.org/browse/JDK-8351542) , In case someone has a good/better idea what to do with the ignored returned value of `jmm_interface->GetOptionalSupport()` (throwing an exception? printing out something?) we can do this of course too. - PR Comment: https://git.openjdk.org/jdk/pull/24052#issuecomment-2724321246
Re: RFR: 8351821: VMManagementImpl.c avoid switching off warnings
On Fri, 14 Mar 2025 10:06:32 GMT, Matthias Baesken wrote: > We switched off unused-variable warnings for the file VMManagementImpl.c, > this should be avoided. > This came up in [JDK-8351542](https://bugs.openjdk.org/browse/JDK-8351542) , Build changes look good to me. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24052#pullrequestreview-2685242504
Re: RFR: 8351603: Change to GCC 14.2.0 for building on Linux at Oracle [v2]
On Thu, 13 Mar 2025 22:55:33 GMT, Mikael Vidstedt wrote: >> Background: >> >> Oracle is updating the version of GCC for building the JDK on Linux to >> 14.2.0. >> >> This change updates the versions of the components used for the (Linux) >> devkit. Newer versions of ccache use cmake for the build, so some of the >> logic in `make/devkit/Tools.gmk` had to be updated to support cmake based >> components. This change also fixes JDK-8344272 (gcc devkit doesn't have >> lto-plugin where needed). >> >> Testing: >> >> Manual builds, tier1-5 + additional func & performance testing. > > Mikael Vidstedt has updated the pull request incrementally with one > additional commit since the last revision: > > Update jib-profiles.js You need to re-state the `/issue add JDK-8344272` command since Skara bots ignored it the first time around due to the PR title... (One could argue that this is a bug in Skara) - PR Comment: https://git.openjdk.org/jdk/pull/23975#issuecomment-2724418915
Re: RFR: 8351603: Change to GCC 14.2.0 for building on Linux at Oracle [v2]
On Thu, 13 Mar 2025 22:55:33 GMT, Mikael Vidstedt wrote: >> Background: >> >> Oracle is updating the version of GCC for building the JDK on Linux to >> 14.2.0. >> >> This change updates the versions of the components used for the (Linux) >> devkit. Newer versions of ccache use cmake for the build, so some of the >> logic in `make/devkit/Tools.gmk` had to be updated to support cmake based >> components. This change also fixes JDK-8344272 (gcc devkit doesn't have >> lto-plugin where needed). >> >> Testing: >> >> Manual builds, tier1-5 + additional func & performance testing. > > Mikael Vidstedt has updated the pull request incrementally with one > additional commit since the last revision: > > Update jib-profiles.js LGTM - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/23975#pullrequestreview-2685253856
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
On Thu, 13 Mar 2025 07:39:20 GMT, Christoph Langer wrote: >> This PR addresses an issue that can be observed when building on Windows >> with configure options `--enable-linkable-runtime` and >> `--with-external-symbols-in-bundles=public`. >> >> The problem is that with this special build configuration, we build two sets >> of .pdb files for the binaries. The first set is the standard debug symbols >> files named .pdb. The second set consists of stripped debug >> symbols file called .stripped.pdb which have less information >> but enough to present line numbers in hs-err files. >> >> During build we use the *.stripped.pdb files for compiling the jmods and >> also the bundle files. However, in the images folder, both sets of .pdb >> files exist. The tests for runtime linking will now, in the absence of jmod >> files, pick up the .pdb files (without *stripped*) from images, but in the >> runtime the hashes of the *stripped* files are stored. >> >> With this change, the standard .pdb files in the >> `--with-external-symbols-in-bundles=public` configuration are now the >> stripped files and we create a set of full pdb files named *.full.pdb. Jmods >> and Bundles still contain the stripped pdb files and we also fix the issue >> that the debug symbols bundle also contained stripped pdb files so far. With >> this fix, it will contain the full pdb files and extracting these over a JDK >> runtime will replace stripped pdbs with full pdbs. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Fix tests I did some tests by building the JDK's master branch (e.g. *without* this patch) with `--with-external-symbols-in-bundles=public` and then renaming the generated `jvm.dll.pdb` to `jvm.dll.full.pdb`. I left `jvm.dll.stripped.pdb` unchanged. Then using [WinDbg](https://learn.microsoft.com/en-us/windows-hardware/drivers/debugger/) to debug jshell, I could confirm that in fact WinDbg could *not* load symbols for `jvm.dll` neither from `jvm.dll.full.pdb` nor from `jvm.dll.stripped.dll`, It would resort to what it calls "exported symbols" for this module, which you can't use for doing much. Then I renamed `jvm.dll.full.pdb` back to `jvm.dll.pdb` and restarted the debugger and confirmed I could now set a breakpoint in the source file and have the debugger step in and highlight source lines properly, as expected. Finally I named `jvm.dll.stripped.pdb` as `jvm.dll.full.pdb` and reloaded everything; this time I could view the functions names in the jvm modules displayed properly in the debugger console when stepping, but could no longer follow its excecution from the source code view. Again this is expected since public symbols contain function names and entry point addresses but not source file and line number info. So it seems that contrary to what I suggested in my previous comment, it *is* important that the `.pdb` file from where the debugger is expected to load symbols from is the same name as was provided to the linker. Now the reason why I initially thought it didn't matter is because `WinDbg` automatically caches pdb files that it successfully loaded under `C:\ProgramData\dbg\sym` and reload them from there even if the original file no longer exist. So if you load the symbols once, then rename the file and start again, you may be misled into thinking that it successfully loaded the symbols from the renamed file while it in fact did so from its cache. Hope this helps, and sorry for the potential misleading info earlier. - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2722436351
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
On Thu, 13 Mar 2025 07:39:20 GMT, Christoph Langer wrote: >> This PR addresses an issue that can be observed when building on Windows >> with configure options `--enable-linkable-runtime` and >> `--with-external-symbols-in-bundles=public`. >> >> The problem is that with this special build configuration, we build two sets >> of .pdb files for the binaries. The first set is the standard debug symbols >> files named .pdb. The second set consists of stripped debug >> symbols file called .stripped.pdb which have less information >> but enough to present line numbers in hs-err files. >> >> During build we use the *.stripped.pdb files for compiling the jmods and >> also the bundle files. However, in the images folder, both sets of .pdb >> files exist. The tests for runtime linking will now, in the absence of jmod >> files, pick up the .pdb files (without *stripped*) from images, but in the >> runtime the hashes of the *stripped* files are stored. >> >> With this change, the standard .pdb files in the >> `--with-external-symbols-in-bundles=public` configuration are now the >> stripped files and we create a set of full pdb files named *.full.pdb. Jmods >> and Bundles still contain the stripped pdb files and we also fix the issue >> that the debug symbols bundle also contained stripped pdb files so far. With >> this fix, it will contain the full pdb files and extracting these over a JDK >> runtime will replace stripped pdbs with full pdbs. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Fix tests Given all of this, is there any possibility at all for --enable-linkable-runtime to handle both full and stripped pdb files, or do we need to accept that this is an inherent limitation for the linkable runtime? - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2724440077
Re: RFR: 8352015: LIBVERIFY_OPTIMIZATION remove special optimization settings
On Fri, 14 Mar 2025 11:15:26 GMT, Matthias Baesken wrote: > On Linux there are some special settings for LIBVERIFY_OPTIMIZATION that are > most likely not needed any more and could be removed. > The removal (on Linux) brings the lib optimization level de facto from LOW to > HIGH. In addition to testing, I assume this will require dig into why compile/other issue was being worked around at the time. - PR Comment: https://git.openjdk.org/jdk/pull/24054#issuecomment-2724448874
Re: RFR: 8352015: LIBVERIFY_OPTIMIZATION remove special optimization settings
On Fri, 14 Mar 2025 11:53:33 GMT, Alan Bateman wrote: >> On Linux there are some special settings for LIBVERIFY_OPTIMIZATION that are >> most likely not needed any more and could be removed. >> The removal (on Linux) brings the lib optimization level de facto from LOW >> to HIGH. > > In addition to testing, I assume this will require dig into why compile/other > issue was being worked around at the time. @AlanBateman This seems to be part of the same change which introduced "Full Debug Symbols" (FDS), and which took a conservative approach. See Erik's comment https://github.com/openjdk/jdk/pull/23966#issuecomment-2714883676. - PR Comment: https://git.openjdk.org/jdk/pull/24054#issuecomment-2724480497
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
On Fri, 14 Mar 2025 12:31:22 GMT, Christoph Langer wrote: > Given all of this, is there any possibility at all for > --enable-linkable-runtime to handle both full and stripped pdb files, or do > we need to accept that this is an inherent limitation for the linkable > runtime? I see a couple of options: 1. Do the same in JMODs that what is done in the image (*.pdb files are full, *.stripped.pdb are, well stripped). I.e. include them both. The problem should go away. I guess that's not desired for some reason? 2. @RealCLanger work-around for a work-around :) Cannot say, this is a nice solution. 3. Don't do the full *.pdb copy over stripped *.pdb files when building with `--with-external-symbols-in-bundles=public` and cater for the debugging case by providing a symbols bundle which has *.pdb (full) and *.stripped.pdb and copy them over the image. None of them seem ideal. My preference would be (3), but YMMV. - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2724864698
Re: RFR: 8352044: Add --with-import-jvms to configure
On Fri, 14 Mar 2025 15:49:44 GMT, Magnus Ihse Bursie wrote: > We should allow pre-built JVMs to be included in a build, so they are just > copied into place, and the jvm.cfg file properly updated. This change copies `libjvm.so` _and_ sibling `.jsa` files, right? If so, then one thing is missing: regenerating CDS archives that have opinions on `modules` filesizes/dates for fingerprinting their CDS archives. My frankensteining scripts do that by invoking new JVM explicitly with `-Xshare:dump`. But build system should already know how to do that, as it does it at the end of the build. - PR Review: https://git.openjdk.org/jdk/pull/24063#pullrequestreview-2686128808
Re: RFR: 8351603: Change to GCC 14.2.0 for building on Linux at Oracle [v2]
On Thu, 13 Mar 2025 22:55:33 GMT, Mikael Vidstedt wrote: >> Background: >> >> Oracle is updating the version of GCC for building the JDK on Linux to >> 14.2.0. >> >> This change updates the versions of the components used for the (Linux) >> devkit. Newer versions of ccache use cmake for the build, so some of the >> logic in `make/devkit/Tools.gmk` had to be updated to support cmake based >> components. This change also fixes JDK-8344272 (gcc devkit doesn't have >> lto-plugin where needed). >> >> Testing: >> >> Manual builds, tier1-5 + additional func & performance testing. > > Mikael Vidstedt has updated the pull request incrementally with one > additional commit since the last revision: > > Update jib-profiles.js Marked as reviewed by erikj (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/23975#pullrequestreview-2685424212
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public
On Fri, 14 Mar 2025 12:29:22 GMT, Christoph Langer wrote:
> Alternative approach to #24012
>
> This keeps the current handling of *.pdb vs *.stripped.pdb which allows
> debugging at the cost of a little hack in jlink. Maybe the code in jlink can
> be improved, e.g. make it more conditional.
>
> I'm running this through our testing still to see whether it'll resolve all
> of the test issues and does not introduce regressions.
Could we change the synopsis of the bug to something like `Handle Windows
specific combination of JEP 493 and --with-external-symbols-in-bundles=public`?
make/Bundles.gmk line 248:
> 246: %.stripped.pdb, \
> 247: $(call FindFiles, $(SYMBOLS_IMAGE_DIR)) \
> 248: )
Why filter *.stripped.pdb? Because they are useless for debugging? Then it
should be mentioned as a comment.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 220:
> 218: // Read from the base JDK image.
> 219: Path path = BASE.resolve(m.resPath);
> 220: // special case to handle stripped pdb files,
> e.g. --with-external-symbols-in-bundles=public
Suggestion:
// Windows debug symbols special case to handle
stripped pdb files. For example builds
// configured with
--with-external-symbols-in-bundles=public
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 224:
> 222: String strippedFile = m.resPath.substring(0,
> m.resPath.lastIndexOf(".pdb")) + ".stripped.pdb";
> 223: Path strippedPath =
> BASE.resolve(strippedFile);
> 224: if (Files.exists(strippedPath)) {
Suggestion:
// With --with-external-symbols-in-bundles=public
configuration, stripped
// pdb files will be present in the image *in
addition* to non-stripped pdb
// files. Since the JMODs at build time only
include stripped pdb files
// (without '.stripped.' in their name), we need to
do the same here so that
// actually the same file content gets checked.
Remember: *.stripped.pdb
// files will only exist for builds configured with
// --with-external-symbols-in-bundles=public
configured builds. Exists check
// is present to allow for regular windows builds
as well.
if (Files.exists(strippedPath)) {
-
PR Review: https://git.openjdk.org/jdk/pull/24057#pullrequestreview-2685668451
PR Review Comment: https://git.openjdk.org/jdk/pull/24057#discussion_r1995654219
PR Review Comment: https://git.openjdk.org/jdk/pull/24057#discussion_r1995652710
PR Review Comment: https://git.openjdk.org/jdk/pull/24057#discussion_r1995642566
RFR: 8352015: LIBVERIFY_OPTIMIZATION remove special optimization settings
On Linux there are some special settings for LIBVERIFY_OPTIMIZATION that are most likely not needed any more and could be removed. The removal (on Linux) brings the lib optimization level de facto from LOW to HIGH. - Commit messages: - JDK-8352015 Changes: https://git.openjdk.org/jdk/pull/24054/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24054&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8352015 Stats: 6 lines in 1 file changed: 0 ins; 5 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/24054.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24054/head:pull/24054 PR: https://git.openjdk.org/jdk/pull/24054
RFR: 8352044: Add --with-import-jvms to configure
We should allow pre-built JVMs to be included in a build, so they are just copied into place, and the jvm.cfg file properly updated. - Commit messages: - 8352044: Add --with-import-jvms to configure Changes: https://git.openjdk.org/jdk/pull/24063/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24063&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8352044 Stats: 79 lines in 4 files changed: 70 ins; 4 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/24063.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24063/head:pull/24063 PR: https://git.openjdk.org/jdk/pull/24063
RFR: 8351821: VMManagementImpl.c avoid switching off warnings
We switched off unused-variable warnings for the file VMManagementImpl.c, this should be avoided. This came up in [JDK-8351542](https://bugs.openjdk.org/browse/JDK-8351542) , - Commit messages: - JDK-8351821 Changes: https://git.openjdk.org/jdk/pull/24052/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24052&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8351821 Stats: 8 lines in 2 files changed: 0 ins; 6 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/24052.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24052/head:pull/24052 PR: https://git.openjdk.org/jdk/pull/24052
Re: RFR: 8352084: Add more test code in TestSetupAOT.java [v2]
On Fri, 14 Mar 2025 23:21:59 GMT, Leonid Mesnik wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments from @erikj79 and @lmesnik
>
> make/RunTests.gmk line 738:
>
>> 736: $1_AOT_JDK_CACHE := $$($1_TEST_SUPPORT_DIR)/aot/jdk.aotcache
>> 737: $1_AOT_JDK_LOG := $$($1_TEST_SUPPORT_DIR)/aot/TestSetupAOT.log
>> 738: $1_AOT_JDK_OUTPUT_DIR := $$($1_TEST_SUPPORT_DIR)/aot
>
> Better to move it before line 735, and reuse in other definitions instead of
> $$($1_TEST_SUPPORT_DIR)/aot
Fixed.
> test/setup_aot/TestSetupAOT.java line 147:
>
>> 145:
>> 146:
>> 147: static void streamOps(String args[]) {
>
> Can you please add a comment with explanation of purpose of 'streamOps'
> method.
I renamed the method to `invokedynamicTests()` and added comments.
> test/setup_aot/TestSetupAOT.java line 201:
>
>> 199: String CSCC = "string" + s + "string" + c;
>> 200:
>> 201: long l = System.currentTimeMillis();
>
> the l should be logged and allow to be defined using property.
> So any testing could be repeated.
> long l = Long.getLong("test.aot.seed", System.currentTimeMillis());
I changed it to not depend on system time. Now the program should be
deterministic.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/24067#discussion_r1996506289
PR Review Comment: https://git.openjdk.org/jdk/pull/24067#discussion_r1996506299
PR Review Comment: https://git.openjdk.org/jdk/pull/24067#discussion_r1996506308
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public
On Fri, 14 Mar 2025 13:55:26 GMT, Magnus Ihse Bursie wrote:
>> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line
>> 226:
>>
>>> 224: if (Files.exists(strippedPath)) {
>>> 225: path = strippedPath;
>>> 226: }
>>
>> This is super weird to put here. I could imagine introducing a Windows
>> specific plugin or option for jlink to help with this setup but I don't
>> think we should rush into a change to JRTArchive to workaround this.
>
> A jlink plugin sounds like a reasonable approach to me.
I don't see how a jlink plugin could help with this. The plugin-pipeline runs
after the archive resources have been processed (i.e. the latter feed into the
former). If archive processing fails you won't get to plugin processing.
With that said, I agree with Alan, this isn't a good place to deal with this.
We'd have two special cases - 1) in the build system that copies over full pdb
files 2) here in jlink to deal with the result of (1).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/24057#discussion_r1995732524
Re: RFR: 8352084: Add more test code in TestSetupAOT.java [v2]
On Sat, 15 Mar 2025 02:08:33 GMT, Ioi Lam wrote: >> I modified TestSetupAOT.java to exercise more functionalities in the JDK so >> that we can have a more substantial AOT cache when running tests with >> `AOT_JDK=true`. E.g: >> >> >> make test JTREG=AOT_JDK=true \ >> TEST=open/test/jdk/java/util/TimeZone/ListTimeZones.java >> >> >> Before: the generated AOT cache was about 20 MB, with 2245 classes and 125 >> resolved indies >> After: the generated AOT cache is about 34 MB, with 4703 classes and 912 >> resolved indies >> >> I verified with Mach5 tiers 4, 5, 6, 10 with all tests tasks that have the >> label `.*aot.jdkcache.*` > > Ioi Lam has updated the pull request incrementally with one additional commit > since the last revision: > > Review comments from @erikj79 and @lmesnik Thanks for fixing those things. - Marked as reviewed by lmesnik (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24067#pullrequestreview-2687294741
Re: RFR: 8352084: Add more test code in TestSetupAOT.java [v2]
> I modified TestSetupAOT.java to exercise more functionalities in the JDK so > that we can have a more substantial AOT cache when running tests with > `AOT_JDK=true`. E.g: > > > make test JTREG=AOT_JDK=true \ > TEST=open/test/jdk/java/util/TimeZone/ListTimeZones.java > > > Before: the generated AOT cache was about 20 MB, with 2245 classes and 125 > resolved indies > After: the generated AOT cache is about 34 MB, with 4703 classes and 912 > resolved indies > > I verified with Mach5 tiers 4, 5, 6, 10 with all tests tasks that have the > label `.*aot.jdkcache.*` Ioi Lam has updated the pull request incrementally with one additional commit since the last revision: Review comments from @erikj79 and @lmesnik - Changes: - all: https://git.openjdk.org/jdk/pull/24067/files - new: https://git.openjdk.org/jdk/pull/24067/files/7c486a8b..82b2c11d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=24067&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24067&range=00-01 Stats: 28 lines in 3 files changed: 7 ins; 7 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/24067.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24067/head:pull/24067 PR: https://git.openjdk.org/jdk/pull/24067
