Re: RFR: JDK-8236128: Allow jpackage create installers for services
On Fri, 11 Mar 2022 23:37:06 GMT, Alexey Semenyuk wrote: > Implementation of [JDK-8275062: "Allow jpackage create installers for > services"](https://bugs.openjdk.java.net/browse/JDK-8275062) > CSR Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7793
Re: RFR: 8282507: Add a separate license file for hsdis [v2]
On Mon, 14 Mar 2022 22:27:29 GMT, Man Cao wrote: >> Hi all, >> >> Could anyone help review the addition of LICENSE file to hsdis directory? >> >> -Man > > Man Cao has updated the pull request incrementally with one additional commit > since the last revision: > > Rename license file to hsdis-license.txt Thank you for helping navigate through this situation! I surely do not want to drag you into the complex legal approval process. Also same here, as an engineer and not a lawyer, my comments are only based on my observations and understandings, and have no legal guarantee. Renaming to "hsdis-license.txt" works perfectly fine for our script and tool. I renamed the file and updated RFE and PR. Let me know if we can submit this. If you still need to go through legal approval, then I'll abandon this PR. - PR: https://git.openjdk.java.net/jdk/pull/7649
Re: RFR: 8282507: Add LICENSE file for hsdis [v2]
> Hi all, > > Could anyone help review the addition of LICENSE file to hsdis directory? > > -Man Man Cao has updated the pull request incrementally with one additional commit since the last revision: Rename license file to hsdis-license.txt - Changes: - all: https://git.openjdk.java.net/jdk/pull/7649/files - new: https://git.openjdk.java.net/jdk/pull/7649/files/d2a940fd..ebe23827 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7649=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7649=00-01 Stats: 0 lines in 1 file changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7649.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7649/head:pull/7649 PR: https://git.openjdk.java.net/jdk/pull/7649
Re: RFR: 8253495: CDS generates non-deterministic output [v6]
On Fri, 11 Mar 2022 06:55:23 GMT, Ioi Lam wrote:
>> This patch makes the result of "java -Xshare:dump" deterministic:
>> - Disabled new Java threads from launching. This is harmless. See comments
>> in jvm.cpp
>> - Fixed a problem in hashtable ordering in heapShared.cpp
>> - BasicHashtableEntry has a gap on 64-bit platforms that may contain random
>> bits. Added code to zero it.
>> - Enabled checking of $JAVA_HOME/lib/server/classes.jsa in
>> make/scripts/compare.sh
>>
>> Note: $JAVA_HOME/lib/server/classes_ncoops.jsa is still non-deterministic.
>> This will be fixed in
>> [JDK-8282828](https://bugs.openjdk.java.net/browse/JDK-8282828).
>>
>> Testing under way:
>> - tier1~tier5
>> - Run all *-cmp-baseline jobs 20 times each (linux-aarch64-cmp-baseline,
>> windows-x86-cmp-baseline, etc).
>
> Ioi Lam has updated the pull request incrementally with one additional commit
> since the last revision:
>
> Added helper function CollectedHeap::zap_filler_array_with
CDS changes look good. One minor comment on a test.
test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDumpAgent.java line 65:
> 63: if (elapsed >= timeout) {
> 64: System.out.println("This JVM may decide to not
> launch any Java threads during -Xshare:dump.");
> 65: System.out.println("This is OK because no string
> objects be in a locked state during heap dump.");
Should `no string objects be` be `no string objects could be`?
-
Marked as reviewed by ccheung (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7748
Integrated: 8283062: Uninitialized warnings in libgtest with GCC 11.2
On Sat, 12 Mar 2022 03:26:29 GMT, Mikael Vidstedt wrote:
> Background, from JBS:
>
> In file included from
> googletest-release-1.8.1/googletest/src/gtest-all.cc:42:
> googletest-release-1.8.1/googletest/src/gtest-death-test.cc: In function
> 'bool testing::internal::StackGrowsDown()':
> googletest-release-1.8.1/googletest/src/gtest-death-test.cc:1224:24: error:
> 'dummy' may be used uninitialized [-Werror=maybe-uninitialized]
> 1224 | StackLowerThanAddress(, );
> | ~^
> googletest-release-1.8.1/googletest/src/gtest-death-test.cc:1214:13: note: by
> argument 1 of type 'const void*' to 'void
> testing::internal::StackLowerThanAddress(const void*, bool*)' declared here
> 1214 | static void StackLowerThanAddress(const void* ptr, bool* result) {
> | ^
> googletest-release-1.8.1/googletest/src/gtest-death-test.cc:1222:7: note:
> 'dummy' declared here
> 1222 | int dummy;
>
>
> Details:
>
> Since googletest is external code this change disable the relevant warning.
>
> Testing:
>
> tier1, builds-tier{2,3,4,5}
This pull request has now been integrated.
Changeset: a244051a
Author:Mikael Vidstedt
URL:
https://git.openjdk.java.net/jdk/commit/a244051a8c967039d7639afcaf83f7d92af49613
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
8283062: Uninitialized warnings in libgtest with GCC 11.2
Reviewed-by: jiefu, erikj
-
PR: https://git.openjdk.java.net/jdk/pull/7798
Re: Supporting alternative toolchains on Windows
On 2022-03-11 23:17, Julian Waters wrote: I meant in toolchain.m4, which allows gcc for macOS Apple used to ship GCC as part of Xcode, and the original Mac port of OpenJDK used that GCC. This was eventually removed from Xcode and the OpenJDK build was changed to use Clang instead. To make the transition smoother, as we can't expect all OpenJDK builders to change Xcode versions at the same time, we left dual support in the build, and we never got around to removing the GCC support. There is still a GCC launcher in Xcode, but it's just a wrapper for Clang. AFAIK, we have never supported any other compiler than the one in Xcode on Macos. So while toolchain.m4 may make it look like we support GCC on Macos, that isn't actually the case. We only support Xcode, and I very much doubt we can still build with an Xcode old enough to still include an actual GCC. /Erik best regards, Julian On Sat, Mar 12, 2022 at 4:32 AM Magnus Ihse Bursie < magnus.ihse.bur...@oracle.com> wrote: On 2022-03-11 20:02, Julian Waters wrote: I understand the concerns, seems like I grossly underestimated the complexity such a task would entail. Though I would say the following can only really be known if it's already been implemented: Most likely there will be strange bugs with anything that requires OS interaction (like dll loading and whatsnot) To my knowledge the versions of said compilers that link against the universal CRT utilize exactly the same Windows APIs (And corresponding dlls) for OS related stuff that Visual C++ itself uses (Minus vcruntime, which is specific to MSVC), without any POSIX compatibility layers on top. I've tried rewiring the build system incrementally in the meantime on my end to see what areas would be of interest and it's now failing when hitting POSIX specific includes during make, hinting that (At least for gcc) compatibility has been traded for full native support for Windows APIs within the compiler, which may mitigate any issues slightly (That's also why I suggested initially to only allow for versions that link against the ucrt without any compatibility layers- Cygwin's toolchains which link against newlib and old MinGW binaries which link against msvcrt would just be an unnecessary headache). That said, whether aforementioned bugs will actually surface should the attempt be successful I don't really know yet Actually, I would seriously assume that any other compiler than VS on Windows will give much worse results. The VS compiler is battle-hardened. gcc and clang are only used by a very small enthusiastic hobbyist minority. The Windows ports of both compilers do keep the same optimizations and code generation quality as they do on other platforms from what I know though, it's mainly the linkers that have been modified to generate the PE format instead of the ELF on Linux. I digress, I don't have any results to show for this yet I'm not sure I get the part about macOS strictly mapping to clang. Isn't there the option to swap to using gcc for macOS? No, Apple removed that option years ago. /Magnus best regards, Julian On Sat, Mar 12, 2022 at 1:50 AM Magnus Ihse Bursie < magnus.ihse.bur...@oracle.com> wrote: On 2022-03-11 14:34, Julian Waters wrote: Darn, seems like it'll be much harder than I expected. Since multiple toolchains are supported for macOS and Linux, I assumed a slight patch would help get it to work on Windows. Looking through the stuff in make though, it appears a lot of the build system implicitly expects the compiler for Windows to always be Visual C++, which doesn't really help that much (Though the fact that we can exclude many versions of gcc, such as Cygwin's and old MinGW binaries helps a lot). The build process for the newer Windows ports of gcc are surprisingly similar to Visual C++ though (Eg rc can be swapped out for windres) so this might hopefully be something I can try exploring in the future (Gonna look a bit harder at make and write what I can find back to this mailing list in the meantime). It'd be interesting if benchmarks of the JVM compiled with different compilers on Windows can be compared side by side on the off chance this becomes a reality though In theory OS and compiler toolchain are separate things. In practice, in the JDK, they are not. There is basically a 1-to-1 mapping between toolchain and OS: Windows <-> Visual Studio macOS <-> XCode/clang linux <-> gcc (The one possible exception is that clang on linux is probably feasible.) After years and years on this, all kinds of assumptions has been hard-coded, even if people try to do the right thing. But sometimes it is not clear if you are checking for toolchain or os; perhaps when linking with a specific library, or setting some define. Any attempt to change this is, as I said, a *huge* undertaking. *And* there will be tons of negative consequences for the code base as a whole, when trying to differentiate between toolchain and OS. So this will
Re: RFR: 8283062: Uninitialized warnings in libgtest with GCC 11.2
On Sat, 12 Mar 2022 03:26:29 GMT, Mikael Vidstedt wrote:
> Background, from JBS:
>
> In file included from
> googletest-release-1.8.1/googletest/src/gtest-all.cc:42:
> googletest-release-1.8.1/googletest/src/gtest-death-test.cc: In function
> 'bool testing::internal::StackGrowsDown()':
> googletest-release-1.8.1/googletest/src/gtest-death-test.cc:1224:24: error:
> 'dummy' may be used uninitialized [-Werror=maybe-uninitialized]
> 1224 | StackLowerThanAddress(, );
> | ~^
> googletest-release-1.8.1/googletest/src/gtest-death-test.cc:1214:13: note: by
> argument 1 of type 'const void*' to 'void
> testing::internal::StackLowerThanAddress(const void*, bool*)' declared here
> 1214 | static void StackLowerThanAddress(const void* ptr, bool* result) {
> | ^
> googletest-release-1.8.1/googletest/src/gtest-death-test.cc:1222:7: note:
> 'dummy' declared here
> 1222 | int dummy;
>
>
> Details:
>
> Since googletest is external code this change disable the relevant warning.
>
> Testing:
>
> tier1, builds-tier{2,3,4,5}
Marked as reviewed by erikj (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/7798
Integrated: 8283017: GHA: Workflows break with update release versions
On Fri, 11 Mar 2022 09:27:55 GMT, Aleksey Shipilev wrote: > Current GHA workflow only takes `VERSION_FEATURE` to deduce the bundle names, > which means the test jobs in GHA workflows are unable to run when update > releases have versions beyond just 11, 17, 18. > > For example, in JDK 18u GHA runs, build step produce: > > > Creating jdk-18.0.1-internal+0_linux-x64_bin.tar.gz > Creating jdk-18.0.1-internal+0_linux-x64_bin-symbols.tar.gz > Creating jdk-18.0.1-internal+0_linux-x64_bin-tests-demos.tar.gz > > > Persist step fails to find it: > > > Warning: No files were found with the provided path: > jdk/build/linux-x64/bundles/jdk-18-internal+0_linux-x64_bin.tar.gz > > > ...because it looks for "18", not "18.0.1". > > 17u and 11u hacked the GHA workflow to get tests to work (see > [JDK-8276130](https://bugs.openjdk.java.net/browse/JDK-8276130)), but this > would keep breaking in update releases as 19.0.1, 20.0.1, etc. fork out of > the mainline. We should instead fix that in the mainline workflow config. > > Additional testing: > - [x] GHA passes with "fake" 19.0.1 > - [ ] GHA passes with "normal" 19 This pull request has now been integrated. Changeset: 01570ca9 Author:Aleksey Shipilev URL: https://git.openjdk.java.net/jdk/commit/01570ca92d234481df2d540027e320b91af415a0 Stats: 29 lines in 1 file changed: 18 ins; 0 del; 11 mod 8283017: GHA: Workflows break with update release versions Reviewed-by: erikj, ihse - PR: https://git.openjdk.java.net/jdk/pull/7785
Re: RFR: 8283017: GHA: Workflows break with update release versions
On Fri, 11 Mar 2022 09:27:55 GMT, Aleksey Shipilev wrote: > Current GHA workflow only takes `VERSION_FEATURE` to deduce the bundle names, > which means the test jobs in GHA workflows are unable to run when update > releases have versions beyond just 11, 17, 18. > > For example, in JDK 18u GHA runs, build step produce: > > > Creating jdk-18.0.1-internal+0_linux-x64_bin.tar.gz > Creating jdk-18.0.1-internal+0_linux-x64_bin-symbols.tar.gz > Creating jdk-18.0.1-internal+0_linux-x64_bin-tests-demos.tar.gz > > > Persist step fails to find it: > > > Warning: No files were found with the provided path: > jdk/build/linux-x64/bundles/jdk-18-internal+0_linux-x64_bin.tar.gz > > > ...because it looks for "18", not "18.0.1". > > 17u and 11u hacked the GHA workflow to get tests to work (see > [JDK-8276130](https://bugs.openjdk.java.net/browse/JDK-8276130)), but this > would keep breaking in update releases as 19.0.1, 20.0.1, etc. fork out of > the mainline. We should instead fix that in the mainline workflow config. > > Additional testing: > - [x] GHA passes with "fake" 19.0.1 > - [ ] GHA passes with "normal" 19 Thank you! - PR: https://git.openjdk.java.net/jdk/pull/7785
