Re: OpenJDK extension to AArch64 and Windows
Hi Magnus, Dropping hotspot-runtime-dev. As you posted the patch you mentioned before (see https://mail.openjdk.java.net/pipermail/build-dev/2020-July/027872.html), I want to share an updated patch for the build system rebased on top of your patch. (I understand your patch still needs to go through a review, but I hope this will give us a better reference point on what adding support for Windows-AArch64 would look like in the build system.) As I mentioned in your patch's discussion, I've broken down adding the support for Windows-AArch64 into 2 bits: [1] adding support for cross-compilation when targetting Windows, and [2] adding support for targetting Windows-AArch64. The two patches are available below. Is it more in line with what you have in mind? Thank you, -- Ludovic [1] Adding support for cross-compilation when targetting Windows ``` diff --git a/make/autoconf/basic.m4 b/make/autoconf/basic.m4 index 7248163242d..60b4097cba9 100644 --- a/make/autoconf/basic.m4 +++ b/make/autoconf/basic.m4 @@ -111,6 +111,16 @@ AC_DEFUN([BASIC_EVAL_DEVKIT_VARIABLE], fi ]) +### +# Evaluates platform specific overrides for build devkit variables. +# $1: Name of variable +AC_DEFUN([BASIC_EVAL_BUILD_DEVKIT_VARIABLE], +[ + if test "x[$]$1" = x; then +eval $1="\${$1_${OPENJDK_BUILD_CPU}}" + fi +]) + ### AC_DEFUN_ONCE([BASIC_SETUP_DEVKIT], [ diff --git a/make/autoconf/flags-ldflags.m4 b/make/autoconf/flags-ldflags.m4 index a2a52f98ef7..8af9a20b5e8 100644 --- a/make/autoconf/flags-ldflags.m4 +++ b/make/autoconf/flags-ldflags.m4 @@ -164,15 +164,14 @@ AC_DEFUN([FLAGS_SETUP_LDFLAGS_CPU_DEP], fi elif test "x$TOOLCHAIN_TYPE" = xmicrosoft; then -if test "x${OPENJDK_$1_CPU}" = "xx86"; then - $1_CPU_LDFLAGS="-safeseh" - # NOTE: Old build added -machine. Probably not needed. - $1_CPU_LDFLAGS_JVM_ONLY="-machine:I386" +if test "x${OPENJDK_$1_CPU_BITS}" = "x32"; then $1_CPU_EXECUTABLE_LDFLAGS="-stack:327680" -else - $1_CPU_LDFLAGS_JVM_ONLY="-machine:AMD64" +elif test "x${OPENJDK_$1_CPU_BITS}" = "x64"; then $1_CPU_EXECUTABLE_LDFLAGS="-stack:1048576" fi +if test "x${OPENJDK_$1_CPU}" = "xx86"; then + $1_CPU_LDFLAGS="-safeseh" +fi fi # JVM_VARIANT_PATH depends on if this is build or target... diff --git a/make/autoconf/flags.m4 b/make/autoconf/flags.m4 index 694d41052ba..8cbf306ab0c 100644 --- a/make/autoconf/flags.m4 +++ b/make/autoconf/flags.m4 @@ -204,32 +204,36 @@ AC_DEFUN_ONCE([FLAGS_SETUP_USER_SUPPLIED_FLAGS], # Param 1 - Optional prefix to all variables. (e.g BUILD_) AC_DEFUN([FLAGS_SETUP_SYSROOT_FLAGS], [ - if test "x[$]$1SYSROOT" != "x"; then -if test "x$TOOLCHAIN_TYPE" = xgcc; then - $1SYSROOT_CFLAGS="--sysroot=[$]$1SYSROOT" - $1SYSROOT_LDFLAGS="--sysroot=[$]$1SYSROOT" -elif test "x$TOOLCHAIN_TYPE" = xclang; then - $1SYSROOT_CFLAGS="-isysroot [$]$1SYSROOT" - $1SYSROOT_LDFLAGS="-isysroot [$]$1SYSROOT" -fi - fi - - if test "x$OPENJDK_TARGET_OS" = xmacosx; then -# We also need -iframework/System/Library/Frameworks -$1SYSROOT_CFLAGS="[$]$1SYSROOT_CFLAGS -iframework [$]$1SYSROOT/System/Library/Frameworks" -$1SYSROOT_LDFLAGS="[$]$1SYSROOT_LDFLAGS -iframework [$]$1SYSROOT/System/Library/Frameworks" -# These always need to be set, or we can't find the frameworks embedded in JavaVM.framework -# set this here so it doesn't have to be peppered throughout the forest -$1SYSROOT_CFLAGS="[$]$1SYSROOT_CFLAGS -F [$]$1SYSROOT/System/Library/Frameworks/JavaVM.framework/Frameworks" -$1SYSROOT_LDFLAGS="[$]$1SYSROOT_LDFLAGS -F [$]$1SYSROOT/System/Library/Frameworks/JavaVM.framework/Frameworks" - fi - # For the microsoft toolchain, we need to get the SYSROOT flags from the # Visual Studio environment. Currently we cannot handle this as a separate # build toolchain. - if test "x$1" = x && test "x$OPENJDK_BUILD_OS" = "xwindows" \ - && test "x$TOOLCHAIN_TYPE" = "xmicrosoft"; then -TOOLCHAIN_SETUP_VISUAL_STUDIO_ENV + if test "x$TOOLCHAIN_TYPE" = "xmicrosoft"; then +# The BUILD_* flags are setup in TOOLCHAIN_SETUP_BUILD_COMPILERS +if test "x$1" = x; then + TOOLCHAIN_SETUP_VISUAL_STUDIO_ENV +fi + +TOOLCHAIN_SETUP_VISUAL_STUDIO_SYSROOT_FLAGS([$1]) + else +if test "x[$]$1SYSROOT" != "x"; then + if test "x$TOOLCHAIN_TYPE" = xgcc; then +$1SYSROOT_CFLAGS="--sysroot=[$]$1SYSROOT" +$1SYSROOT_LDFLAGS="--sysroot=[$]$1SYSROOT" + elif test "x$TOOLCHAIN_TYPE" = xclang; then +$1SYSROOT_CFLAGS="-isysroot [$]$1SYSROOT" +$1SYSROOT_LDFLAGS="-isysroot [$]$1SYSROOT" + fi +fi + +if test "x$OPENJDK_TARGET_OS" = xmacosx; then + # We also need -iframework/System/Library/Frameworks + $1SYSROOT_CFLAGS="[$]$1SYSROOT_CFLAGS -iframework
Re: Preliminary review for new WINENV support
Hi, I tried out your changes locally, and with Yasumasa's change and the following diff [1], I can confirm that this works on my setup (VS2019 devkit w/ Cygwin). I'll give a spin with WSL1 and WSL2, as well as VS2017. [1] Diff to fix support for VS2019 --- a/make/autoconf/toolchain_microsoft.m4 +++ b/make/autoconf/toolchain_microsoft.m4 @@ -628,9 +606,9 @@ AC_DEFUN([TOOLCHAIN_SETUP_VS_RUNTIME_DLLS], fi AC_ARG_WITH(vcruntime-1-dll, [AS_HELP_STRING([--with-vcruntime-1-dll], [path to microsoft C++ runtime dll (vcruntime*_1.dll) (Windows x64 only) @<:@probed@:>@])]) - if test "x$VCRUNTIME_1_NAME" != "x" && test "x$OPENJDK_TARGET_BITS" = x64; then + if test "x$VCRUNTIME_1_NAME" != "x" && test "x$OPENJDK_TARGET_CPU_BITS" = x64; then if test "x$with_vcruntime_1_dll" != x; then # If given explicitly by user, do not probe. If not present, fail directly. TOOLCHAIN_CHECK_POSSIBLE_MSVC_DLL($VCRUNTIME_1_NAME, [$with_vcruntime_1_dll], I also want to propose integrating the bits to support cross-compilation with the microsoft toolchain as part of your change, even without adding the targetting for Windows-AArch64. The following diff [2] integrates such support for cross-compilation without adding Windows-AArch64: [2] Diff to add support for cross-compilation commit c23c78e33e57955d3f344383619592f34b84169b Author: Ludovic Henry Date: Tue Jul 7 11:35:41 2020 -0700 8248498: Add build system support for Windows AArch64 (Part 1) This adds support for cross-compilation on Windows, without adding the AArch64 specific bits. https://bugs.openjdk.java.net/browse/JDK-8248498 diff --git a/make/autoconf/basic.m4 b/make/autoconf/basic.m4 index 7248163242d..60b4097cba9 100644 --- a/make/autoconf/basic.m4 +++ b/make/autoconf/basic.m4 @@ -111,6 +111,16 @@ AC_DEFUN([BASIC_EVAL_DEVKIT_VARIABLE], fi ]) +### +# Evaluates platform specific overrides for build devkit variables. +# $1: Name of variable +AC_DEFUN([BASIC_EVAL_BUILD_DEVKIT_VARIABLE], +[ + if test "x[$]$1" = x; then +eval $1="\${$1_${OPENJDK_BUILD_CPU}}" + fi +]) + ### AC_DEFUN_ONCE([BASIC_SETUP_DEVKIT], [ diff --git a/make/autoconf/flags-ldflags.m4 b/make/autoconf/flags-ldflags.m4 index a2a52f98ef7..8af9a20b5e8 100644 --- a/make/autoconf/flags-ldflags.m4 +++ b/make/autoconf/flags-ldflags.m4 @@ -164,15 +164,14 @@ AC_DEFUN([FLAGS_SETUP_LDFLAGS_CPU_DEP], fi elif test "x$TOOLCHAIN_TYPE" = xmicrosoft; then -if test "x${OPENJDK_$1_CPU}" = "xx86"; then - $1_CPU_LDFLAGS="-safeseh" - # NOTE: Old build added -machine. Probably not needed. - $1_CPU_LDFLAGS_JVM_ONLY="-machine:I386" +if test "x${OPENJDK_$1_CPU_BITS}" = "x32"; then $1_CPU_EXECUTABLE_LDFLAGS="-stack:327680" -else - $1_CPU_LDFLAGS_JVM_ONLY="-machine:AMD64" +elif test "x${OPENJDK_$1_CPU_BITS}" = "x64"; then $1_CPU_EXECUTABLE_LDFLAGS="-stack:1048576" fi +if test "x${OPENJDK_$1_CPU}" = "xx86"; then + $1_CPU_LDFLAGS="-safeseh" +fi fi # JVM_VARIANT_PATH depends on if this is build or target... diff --git a/make/autoconf/flags.m4 b/make/autoconf/flags.m4 index 694d41052ba..8cbf306ab0c 100644 --- a/make/autoconf/flags.m4 +++ b/make/autoconf/flags.m4 @@ -204,32 +204,36 @@ AC_DEFUN_ONCE([FLAGS_SETUP_USER_SUPPLIED_FLAGS], # Param 1 - Optional prefix to all variables. (e.g BUILD_) AC_DEFUN([FLAGS_SETUP_SYSROOT_FLAGS], [ - if test "x[$]$1SYSROOT" != "x"; then -if test "x$TOOLCHAIN_TYPE" = xgcc; then - $1SYSROOT_CFLAGS="--sysroot=[$]$1SYSROOT" - $1SYSROOT_LDFLAGS="--sysroot=[$]$1SYSROOT" -elif test "x$TOOLCHAIN_TYPE" = xclang; then - $1SYSROOT_CFLAGS="-isysroot [$]$1SYSROOT" - $1SYSROOT_LDFLAGS="-isysroot [$]$1SYSROOT" -fi - fi - - if test "x$OPENJDK_TARGET_OS" = xmacosx; then -# We also need -iframework/System/Library/Frameworks -$1SYSROOT_CFLAGS="[$]$1SYSROOT_CFLAGS -iframework [$]$1SYSROOT/System/Library/Frameworks" -$1SYSROOT_LDFLAGS="[$]$1SYSROOT_LDFLAGS -iframework [$]$1SYSROOT/System/Library/Frameworks" -# These always need to be set, or we can't find the frameworks embedded in JavaVM.framework -# set this here so it doesn't have to be peppered throughout the forest -$1SYSROOT_CFLAGS="[$]$1SYSROOT_CFLAGS -F [$]$1SYSROOT/System/Library/Frameworks/JavaVM.framework/Frameworks" -$1SYSROOT_LDFLAGS="[$]$1SYSROOT_LDFLAGS -F [$]$1SYSROOT/System/Library/Frameworks/JavaVM.framework/Frameworks" - fi - # For the microsoft toolchain, we need to get the SYSROOT flags from the # Visual Studio environment. Currently we cannot handle this as a separate # build toolchain. - if test "x$1" = x && test "x$OPENJDK_BUILD_OS" = "xwindows" \ - && test "x$TOOLCHAIN_TYPE" = "xmicrosoft"; then -TOOLCHAIN_SETUP_VISUAL_STUDIO_ENV + if test
Re: RFR [XS]: 8248429: Add --enable-preview as VM argument when running microbenchmarks
Hi, I have found it very useful to be able to run benchmarks against multiple versions of the JDK. Build the benchmark jar once and to compare results. If all of the classes are built with --enable-preview, none of them will run against older JDKs. So an approach that only compiles those files that need preview would be more useful. Or perhaps, separate out preview based benchmarks into a separate jar. $.02, Roger On 7/7/20 10:26 AM, Peter Levart wrote: I suggest adding --enable-preview to JMH_JVM_ARGS in general now (it doesn't hurt even if classes are not compiled with --enable-preview) and then take time to devise an effective strategy for selectively compiling micro benchmarks with or without --enable-preview. At least so the benchmarks would work out-of-the-box when run via make test. WDYT? Regards, Peter On 6/30/20 10:15 PM, Claes Redestad wrote: On 2020-06-30 22:12, Magnus Ihse Bursie wrote: Second to that a solution in the build would be preferable - if we can come up with something that has infinitesimal impact to build times. Are we talking about many files? Could you consider listing those files explicitly in the makefile? That would make it cheap to filter them out from the normal compilation, and instead do a secondary compilation with them. Right now there's one micro using --enable-preview, so that'd be a very short list. /Claes
Re: RFR [XS]: 8248429: Add --enable-preview as VM argument when running microbenchmarks
On 30/06/2020 22:51, Erik Joelsson wrote: On 2020-06-30 13:15, Claes Redestad wrote: On 2020-06-30 22:12, Magnus Ihse Bursie wrote: Second to that a solution in the build would be preferable - if we can come up with something that has infinitesimal impact to build times. Are we talking about many files? Could you consider listing those files explicitly in the makefile? That would make it cheap to filter them out from the normal compilation, and instead do a secondary compilation with them. Right now there's one micro using --enable-preview, so that'd be a very short list. Having to update a list manually is no fun, but at least we can make the build fail if you forget. If you only disable the warning "preview" for the special compilation set, the build would fail if another microbenchmark was added that needed it. A little bit of an update on this. I've implemented the manual list of preview benchmarks [1], but there's a problem with the current patch. Both the BUILD_JDK_MICROBENCHMARK and the BUILD_JDK_MICROBENCHMARK_PREVIEW have the same output folder, and this is good, since we want only 1 benchmarks.jar, we want the compilation result to be combined. But the JMH annotation processor is also generating some metadata files in META-INF, namely BenchmarkList and CompilerHints. The problem is that both compilation tasks seem to be trying to write to these files at the same time, leading to e.g. the RecordsDeserialization benchmark not appearing in the final BenchmarkList, or seemingly resulting in a mangled file, crashing the JMH parser. I've tried to resolve this race be adding a dependency on BUILD_JDK_MICROBENCHMARK to BUILD_JDK_MICROBENCHMARK_PREVIEW (see the patch), but this doesn't resolve the problem. I'm still investigating this. Jorn [1] : http://cr.openjdk.java.net/~jvernee/8248429/webrev.02/ /Erik /Claes
Re: RFR [XS]: 8248429: Add --enable-preview as VM argument when running microbenchmarks
On 7/7/20 4:26 PM, Peter Levart wrote: I suggest adding --enable-preview to JMH_JVM_ARGS in general now (it doesn't hurt even if classes are not compiled with --enable-preview) and then take time to devise an effective strategy for selectively compiling micro benchmarks with or without --enable-preview. At least so the benchmarks would work out-of-the-box when run via make test. WDYT? Or maybe this variant is acceptable? http://cr.openjdk.java.net/~plevart/jdk-dev/8248429_jmh_enable_preview/webrev.01/ I took Jorn Vernee's patch and modified it a bit so that it does not need to find and grep the files but I rather specify relative source directories for EXCLUDES and INCLUDES parameters to the two separate SetupJavaCompilation tasks respectively. Benchmarks that need preview features (currently just one) then need to be in a package with special prefix org.openjdk.bench.preview so they are separately compiled with --enable-preview option. Other files are compiled without it. Peter Regards, Peter On 6/30/20 10:15 PM, Claes Redestad wrote: On 2020-06-30 22:12, Magnus Ihse Bursie wrote: Second to that a solution in the build would be preferable - if we can come up with something that has infinitesimal impact to build times. Are we talking about many files? Could you consider listing those files explicitly in the makefile? That would make it cheap to filter them out from the normal compilation, and instead do a secondary compilation with them. Right now there's one micro using --enable-preview, so that'd be a very short list. /Claes
Re: RFR: 8246032: Implementation of JEP 347: Adopt C++14 Language Features in HotSpot
> On Jul 7, 2020, at 9:59 AM, Erik Joelsson wrote: > > Doc changes look good. > > /Erik > > On 2020-07-06 18:10, Kim Barrett wrote: >> I just noticed that doc/building.{md,html} describes minimum toolchain >> versions, >> so also needs to be updated. Here’s the update, matching what’s now in the >> JEP: >> >> full: https://cr.openjdk.java.net/~kbarrett/8246032/open.05/ >> incr: https://cr.openjdk.java.net/~kbarrett/8246032/open.05.inc/ >> >> I also deleted a discussion of a problem one might run into when building >> with >> Visual Studio 2010, since that version is no longer relevant. Thanks.
Re: RFR: JDK-8248158 Configure fails with autoconf not found even though it's installed
On Mon, Jul 6, 2020 at 10:50 PM Simon Tooke wrote: > (Disclaimer: I am not a reviewer, so this is an opinion, not a review) > > I have tested this on Windows and it built without issue, although the > submit repo should be the final gate. I'd also like to add my void to > simply redefining 'WHICH' as it leads to less changes in the source > code, which would make life easier should this be backported to 11u > and/or 8u. So, you would just switch the UTIL_REQUIRE_PROGS call for WHICH to be `type ...` instead? > > -Simon > > On 2020-07-02 4:22 a.m., Galder Zamarreno wrote: > > On Thu, Jul 2, 2020 at 12:37 AM Magnus Ihse Bursie < > > magnus.ihse.bur...@oracle.com> wrote: > > > >> > >> On 2020-07-01 12:05, Galder Zamarreno wrote: > >>> Using `which` to check whether commands exist can result in confusing > >>> errors when `which` itself is not installed in the system. This is the > >> case > >>> with `autoconf`, where if `autoconf` is present but `which` isn't, the > >>> build system says that `autoconf` is missing, when in reality it is > >> `which` > >>> which is missing. The fix switches autoconf uses of `which` for `type > -p` > >>> instead, which is a Bash built-in command. > >>> > >>> I've tested the fix with a fedora docker container that had `autoconf` > >>> installed but `which`. When using `type -p` it correctly detects > >> `autoconf` > >>> installed and eventually fails saying that `which` is not installed, > >> which > >>> is the expected behaviour. > >>> > >>> `which` is still in use in make/autoconf/util_windows.m4. A possible > >> future > >>> improvement would be to see if `which` use there could be replaced as > >> well. > >>> Eventually, when no `which` uses remain, the presence check for `which` > >>> could be removed. > >> I agree that we should replace "which" with "type -p" everywhere. The > >> best way to do this is probably to replace the value of $WHICH with > >> "type -p". It's a bash built-in, so we can count on its presence. If you > >> want to fix that as part of this bug, I'm ok with it, otherwise we > >> should open a new bug to track this. I think there is also one or two > >> instances of "command" recently added as (better, but not as good as > >> "type -p") replacements for which. > >> > > I discovered one place in util.m4 where command was being used. > > > > There are other places outside of make/ where command is used but I feel > > those are a bit out of scope here. > > > > My main objective is that from a configure perspective, we'd try to > reduce > > the number of dependencies needed to build things. > > > > I'll send an updated webrev shortly. > > > > > >> /Magnus > >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8248158 > >>> WebRev: > >> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8248158/01/webrev/ > >>> Galder > >> > >
Re: RFR [XS]: 8248429: Add --enable-preview as VM argument when running microbenchmarks
I suggest adding --enable-preview to JMH_JVM_ARGS in general now (it doesn't hurt even if classes are not compiled with --enable-preview) and then take time to devise an effective strategy for selectively compiling micro benchmarks with or without --enable-preview. At least so the benchmarks would work out-of-the-box when run via make test. WDYT? Regards, Peter On 6/30/20 10:15 PM, Claes Redestad wrote: On 2020-06-30 22:12, Magnus Ihse Bursie wrote: Second to that a solution in the build would be preferable - if we can come up with something that has infinitesimal impact to build times. Are we talking about many files? Could you consider listing those files explicitly in the makefile? That would make it cheap to filter them out from the normal compilation, and instead do a secondary compilation with them. Right now there's one micro using --enable-preview, so that'd be a very short list. /Claes
Re: RFR: 8246032: Implementation of JEP 347: Adopt C++14 Language Features in HotSpot
Doc changes look good. /Erik On 2020-07-06 18:10, Kim Barrett wrote: I just noticed that doc/building.{md,html} describes minimum toolchain versions, so also needs to be updated. Here’s the update, matching what’s now in the JEP: full: https://cr.openjdk.java.net/~kbarrett/8246032/open.05/ incr: https://cr.openjdk.java.net/~kbarrett/8246032/open.05.inc/ I also deleted a discussion of a problem one might run into when building with Visual Studio 2010, since that version is no longer relevant.
Re: Change for 8248135: Build microbenchmarks with --enable-preview makes other non-preview JMH benchmarks to fail
Yes, Chris, this is the same thing. I wasn't aware of it. Peter On 7/7/20 11:00 AM, Chris Hegarty wrote: Peter, 8248429 [1] tracks this issue, right? There was a recent thread on build-dev relating to this, in the form of an RFR from Jorn : https://mail.openjdk.java.net/pipermail/build-dev/2020-June/thread.html#27804 Some great discussion was had, but I’m not sure that a conclusion was reached yet. 8248429 is the same issue, right? -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8248429 On 7 Jul 2020, at 09:41, Peter Levart wrote: A quick work-around for anyone wanting to run the microbenchmarks is to pass --enable-preview explicitly. For example: make test TEST="micro:java.util.stream.ops" MICRO="JAVA_OPTIONS=--enable-preview" Regards, Peter On 7/7/20 10:23 AM, Peter Levart wrote: On 7/7/20 10:13 AM, David Holmes wrote: Hi Peter, cc Claes On 7/07/2020 5:59 pm, Peter Levart wrote: Hi, Recently I proposed and pushed a change for [1] which adds --enable-preview option to javac compilation of JMH microbenchmarks in general to enable running a benchmark that uses preview feature (Records). This makes the class files produced marked with version 60.65535. The benchmark that uses preview feature executes without problems because it explicitly specifies the following in its code: @Fork(value = 1, warmups = 0, jvmArgsAppend = "--enable-preview") Recently I wanted to run JMH benchmarks for Stream ops with: make test TEST="micro:java.util.stream.ops" ...but all of them fail to run with the following exception: java.lang.UnsupportedClassVersionError: Preview features are not enabled for org/openjdk/bench/java/util/stream/ops/value/generated/NoneMatchShort_seq_start_jmhTest (class file version 60.65535). Try running with '--enable-preview' What shall we do? Add similar annotation to all of them? Is there a way to specify that all micro benchmarks should be run with --enable-preview option passed to java? So this breaks running all non-preview using benchmarks? If so I say we need to backout the change for 8248135 while a proper solution is found. I guess it does break (at least the way I tried to run them). The problem is that this little change: --- a/make/test/BuildMicrobenchmark.gmkWed Jun 24 01:02:19 2020 +0200 +++ b/make/test/BuildMicrobenchmark.gmkWed Jun 24 11:05:09 2020 +0200 @@ -90,10 +90,11 @@ TARGET_RELEASE := $(TARGET_RELEASE_NEWJDK_UPGRADED), \ SMALL_JAVA := false, \ CLASSPATH := $(MICROBENCHMARK_CLASSPATH), \ -DISABLED_WARNINGS := processing rawtypes cast serial, \ +DISABLED_WARNINGS := processing rawtypes cast serial preview, \ SRC := $(MICROBENCHMARK_SRC), \ BIN := $(MICROBENCHMARK_CLASSES), \ JAVA_FLAGS := --add-modules jdk.unsupported --limit-modules java.management, \ +JAVAC_FLAGS := --enable-preview, \ )) ...was pushed as part of larger fix for 8247532 which has already been forward and backported. So I think backing out the whole patch (which is perfectly OK by itself) would cause more problems then fixing this particular problem in a followup, given that we can find a fix quickly. Its has been 14 days since the above was pushed and nobody noticed until now, so I guess this is not a big problem? Regards, Peter David - [1] https://bugs.openjdk.java.net/browse/JDK-8248135 Regards, Peter
Re: Change for 8248135: Build microbenchmarks with --enable-preview makes other non-preview JMH benchmarks to fail
Peter, 8248429 [1] tracks this issue, right? There was a recent thread on build-dev relating to this, in the form of an RFR from Jorn : https://mail.openjdk.java.net/pipermail/build-dev/2020-June/thread.html#27804 Some great discussion was had, but I’m not sure that a conclusion was reached yet. 8248429 is the same issue, right? -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8248429 > On 7 Jul 2020, at 09:41, Peter Levart wrote: > > A quick work-around for anyone wanting to run the microbenchmarks is to pass > --enable-preview explicitly. For example: > > > make test TEST="micro:java.util.stream.ops" > MICRO="JAVA_OPTIONS=--enable-preview" > > > Regards, Peter > > > On 7/7/20 10:23 AM, Peter Levart wrote: >> >> On 7/7/20 10:13 AM, David Holmes wrote: >>> Hi Peter, >>> >>> cc Claes >>> >>> On 7/07/2020 5:59 pm, Peter Levart wrote: Hi, Recently I proposed and pushed a change for [1] which adds --enable-preview option to javac compilation of JMH microbenchmarks in general to enable running a benchmark that uses preview feature (Records). This makes the class files produced marked with version 60.65535. The benchmark that uses preview feature executes without problems because it explicitly specifies the following in its code: @Fork(value = 1, warmups = 0, jvmArgsAppend = "--enable-preview") Recently I wanted to run JMH benchmarks for Stream ops with: make test TEST="micro:java.util.stream.ops" ...but all of them fail to run with the following exception: java.lang.UnsupportedClassVersionError: Preview features are not enabled for org/openjdk/bench/java/util/stream/ops/value/generated/NoneMatchShort_seq_start_jmhTest (class file version 60.65535). Try running with '--enable-preview' What shall we do? Add similar annotation to all of them? Is there a way to specify that all micro benchmarks should be run with --enable-preview option passed to java? >>> >>> So this breaks running all non-preview using benchmarks? If so I say we >>> need to backout the change for 8248135 while a proper solution is found. >> >> >> I guess it does break (at least the way I tried to run them). The problem is >> that this little change: >> >> >> --- a/make/test/BuildMicrobenchmark.gmkWed Jun 24 01:02:19 2020 +0200 >> +++ b/make/test/BuildMicrobenchmark.gmkWed Jun 24 11:05:09 2020 +0200 >> @@ -90,10 +90,11 @@ >> TARGET_RELEASE := $(TARGET_RELEASE_NEWJDK_UPGRADED), \ >> SMALL_JAVA := false, \ >> CLASSPATH := $(MICROBENCHMARK_CLASSPATH), \ >> -DISABLED_WARNINGS := processing rawtypes cast serial, \ >> +DISABLED_WARNINGS := processing rawtypes cast serial preview, \ >> SRC := $(MICROBENCHMARK_SRC), \ >> BIN := $(MICROBENCHMARK_CLASSES), \ >> JAVA_FLAGS := --add-modules jdk.unsupported --limit-modules >> java.management, \ >> +JAVAC_FLAGS := --enable-preview, \ >> )) >> >> >> ...was pushed as part of larger fix for 8247532 which has already been >> forward and backported. So I think backing out the whole patch (which is >> perfectly OK by itself) would cause more problems then fixing this >> particular problem in a followup, given that we can find a fix quickly. Its >> has been 14 days since the above was pushed and nobody noticed until now, so >> I guess this is not a big problem? >> >> >> Regards, Peter >> >> >> >>> >>> David >>> - >>> [1] https://bugs.openjdk.java.net/browse/JDK-8248135 Regards, Peter
Re: Change for 8248135: Build microbenchmarks with --enable-preview makes other non-preview JMH benchmarks to fail
The following patch seems to fix this problem: === --- make/RunTests.gmk (revision 60064:00a964b6ab716706f0deab6dd04b43f31b1939c0) +++ make/RunTests.gmk (revision 60064+:00a964b6ab71+) @@ -693,6 +693,9 @@ # Set library path for native dependencies $1_JMH_JVM_ARGS := -Djava.library.path=$$(TEST_IMAGE_DIR)/micro/native + # Micro benchmarks are compiled with --enable-preview so we must run them with --enable-preview + $1_JMH_JVM_ARGS += --enable-preview + ifneq ($$(MICRO_VM_OPTIONS)$$(MICRO_JAVA_OPTIONS), ) $1_JMH_JVM_ARGS += $$(MICRO_VM_OPTIONS) $$(MICRO_JAVA_OPTIONS) endif ...I can prepare a formal RFR quickly if this is the way to go. Regards, Peter On 7/7/20 10:41 AM, Peter Levart wrote: A quick work-around for anyone wanting to run the microbenchmarks is to pass --enable-preview explicitly. For example: make test TEST="micro:java.util.stream.ops" MICRO="JAVA_OPTIONS=--enable-preview" Regards, Peter On 7/7/20 10:23 AM, Peter Levart wrote: On 7/7/20 10:13 AM, David Holmes wrote: Hi Peter, cc Claes On 7/07/2020 5:59 pm, Peter Levart wrote: Hi, Recently I proposed and pushed a change for [1] which adds --enable-preview option to javac compilation of JMH microbenchmarks in general to enable running a benchmark that uses preview feature (Records). This makes the class files produced marked with version 60.65535. The benchmark that uses preview feature executes without problems because it explicitly specifies the following in its code: @Fork(value = 1, warmups = 0, jvmArgsAppend = "--enable-preview") Recently I wanted to run JMH benchmarks for Stream ops with: make test TEST="micro:java.util.stream.ops" ...but all of them fail to run with the following exception: java.lang.UnsupportedClassVersionError: Preview features are not enabled for org/openjdk/bench/java/util/stream/ops/value/generated/NoneMatchShort_seq_start_jmhTest (class file version 60.65535). Try running with '--enable-preview' What shall we do? Add similar annotation to all of them? Is there a way to specify that all micro benchmarks should be run with --enable-preview option passed to java? So this breaks running all non-preview using benchmarks? If so I say we need to backout the change for 8248135 while a proper solution is found. I guess it does break (at least the way I tried to run them). The problem is that this little change: --- a/make/test/BuildMicrobenchmark.gmk Wed Jun 24 01:02:19 2020 +0200 +++ b/make/test/BuildMicrobenchmark.gmk Wed Jun 24 11:05:09 2020 +0200 @@ -90,10 +90,11 @@ TARGET_RELEASE := $(TARGET_RELEASE_NEWJDK_UPGRADED), \ SMALL_JAVA := false, \ CLASSPATH := $(MICROBENCHMARK_CLASSPATH), \ - DISABLED_WARNINGS := processing rawtypes cast serial, \ + DISABLED_WARNINGS := processing rawtypes cast serial preview, \ SRC := $(MICROBENCHMARK_SRC), \ BIN := $(MICROBENCHMARK_CLASSES), \ JAVA_FLAGS := --add-modules jdk.unsupported --limit-modules java.management, \ + JAVAC_FLAGS := --enable-preview, \ )) ...was pushed as part of larger fix for 8247532 which has already been forward and backported. So I think backing out the whole patch (which is perfectly OK by itself) would cause more problems then fixing this particular problem in a followup, given that we can find a fix quickly. Its has been 14 days since the above was pushed and nobody noticed until now, so I guess this is not a big problem? Regards, Peter David - [1] https://bugs.openjdk.java.net/browse/JDK-8248135 Regards, Peter
Re: Change for 8248135: Build microbenchmarks with --enable-preview makes other non-preview JMH benchmarks to fail
A quick work-around for anyone wanting to run the microbenchmarks is to pass --enable-preview explicitly. For example: make test TEST="micro:java.util.stream.ops" MICRO="JAVA_OPTIONS=--enable-preview" Regards, Peter On 7/7/20 10:23 AM, Peter Levart wrote: On 7/7/20 10:13 AM, David Holmes wrote: Hi Peter, cc Claes On 7/07/2020 5:59 pm, Peter Levart wrote: Hi, Recently I proposed and pushed a change for [1] which adds --enable-preview option to javac compilation of JMH microbenchmarks in general to enable running a benchmark that uses preview feature (Records). This makes the class files produced marked with version 60.65535. The benchmark that uses preview feature executes without problems because it explicitly specifies the following in its code: @Fork(value = 1, warmups = 0, jvmArgsAppend = "--enable-preview") Recently I wanted to run JMH benchmarks for Stream ops with: make test TEST="micro:java.util.stream.ops" ...but all of them fail to run with the following exception: java.lang.UnsupportedClassVersionError: Preview features are not enabled for org/openjdk/bench/java/util/stream/ops/value/generated/NoneMatchShort_seq_start_jmhTest (class file version 60.65535). Try running with '--enable-preview' What shall we do? Add similar annotation to all of them? Is there a way to specify that all micro benchmarks should be run with --enable-preview option passed to java? So this breaks running all non-preview using benchmarks? If so I say we need to backout the change for 8248135 while a proper solution is found. I guess it does break (at least the way I tried to run them). The problem is that this little change: --- a/make/test/BuildMicrobenchmark.gmk Wed Jun 24 01:02:19 2020 +0200 +++ b/make/test/BuildMicrobenchmark.gmk Wed Jun 24 11:05:09 2020 +0200 @@ -90,10 +90,11 @@ TARGET_RELEASE := $(TARGET_RELEASE_NEWJDK_UPGRADED), \ SMALL_JAVA := false, \ CLASSPATH := $(MICROBENCHMARK_CLASSPATH), \ - DISABLED_WARNINGS := processing rawtypes cast serial, \ + DISABLED_WARNINGS := processing rawtypes cast serial preview, \ SRC := $(MICROBENCHMARK_SRC), \ BIN := $(MICROBENCHMARK_CLASSES), \ JAVA_FLAGS := --add-modules jdk.unsupported --limit-modules java.management, \ + JAVAC_FLAGS := --enable-preview, \ )) ...was pushed as part of larger fix for 8247532 which has already been forward and backported. So I think backing out the whole patch (which is perfectly OK by itself) would cause more problems then fixing this particular problem in a followup, given that we can find a fix quickly. Its has been 14 days since the above was pushed and nobody noticed until now, so I guess this is not a big problem? Regards, Peter David - [1] https://bugs.openjdk.java.net/browse/JDK-8248135 Regards, Peter
Re: Change for 8248135: Build microbenchmarks with --enable-preview makes other non-preview JMH benchmarks to fail
On 7/7/20 10:13 AM, David Holmes wrote: Hi Peter, cc Claes On 7/07/2020 5:59 pm, Peter Levart wrote: Hi, Recently I proposed and pushed a change for [1] which adds --enable-preview option to javac compilation of JMH microbenchmarks in general to enable running a benchmark that uses preview feature (Records). This makes the class files produced marked with version 60.65535. The benchmark that uses preview feature executes without problems because it explicitly specifies the following in its code: @Fork(value = 1, warmups = 0, jvmArgsAppend = "--enable-preview") Recently I wanted to run JMH benchmarks for Stream ops with: make test TEST="micro:java.util.stream.ops" ...but all of them fail to run with the following exception: java.lang.UnsupportedClassVersionError: Preview features are not enabled for org/openjdk/bench/java/util/stream/ops/value/generated/NoneMatchShort_seq_start_jmhTest (class file version 60.65535). Try running with '--enable-preview' What shall we do? Add similar annotation to all of them? Is there a way to specify that all micro benchmarks should be run with --enable-preview option passed to java? So this breaks running all non-preview using benchmarks? If so I say we need to backout the change for 8248135 while a proper solution is found. I guess it does break (at least the way I tried to run them). The problem is that this little change: --- a/make/test/BuildMicrobenchmark.gmk Wed Jun 24 01:02:19 2020 +0200 +++ b/make/test/BuildMicrobenchmark.gmk Wed Jun 24 11:05:09 2020 +0200 @@ -90,10 +90,11 @@ TARGET_RELEASE := $(TARGET_RELEASE_NEWJDK_UPGRADED), \ SMALL_JAVA := false, \ CLASSPATH := $(MICROBENCHMARK_CLASSPATH), \ - DISABLED_WARNINGS := processing rawtypes cast serial, \ + DISABLED_WARNINGS := processing rawtypes cast serial preview, \ SRC := $(MICROBENCHMARK_SRC), \ BIN := $(MICROBENCHMARK_CLASSES), \ JAVA_FLAGS := --add-modules jdk.unsupported --limit-modules java.management, \ + JAVAC_FLAGS := --enable-preview, \ )) ...was pushed as part of larger fix for 8247532 which has already been forward and backported. So I think backing out the whole patch (which is perfectly OK by itself) would cause more problems then fixing this particular problem in a followup, given that we can find a fix quickly. Its has been 14 days since the above was pushed and nobody noticed until now, so I guess this is not a big problem? Regards, Peter David - [1] https://bugs.openjdk.java.net/browse/JDK-8248135 Regards, Peter
Re: Change for 8248135: Build microbenchmarks with --enable-preview makes other non-preview JMH benchmarks to fail
Hi Peter, cc Claes On 7/07/2020 5:59 pm, Peter Levart wrote: Hi, Recently I proposed and pushed a change for [1] which adds --enable-preview option to javac compilation of JMH microbenchmarks in general to enable running a benchmark that uses preview feature (Records). This makes the class files produced marked with version 60.65535. The benchmark that uses preview feature executes without problems because it explicitly specifies the following in its code: @Fork(value = 1, warmups = 0, jvmArgsAppend = "--enable-preview") Recently I wanted to run JMH benchmarks for Stream ops with: make test TEST="micro:java.util.stream.ops" ...but all of them fail to run with the following exception: java.lang.UnsupportedClassVersionError: Preview features are not enabled for org/openjdk/bench/java/util/stream/ops/value/generated/NoneMatchShort_seq_start_jmhTest (class file version 60.65535). Try running with '--enable-preview' What shall we do? Add similar annotation to all of them? Is there a way to specify that all micro benchmarks should be run with --enable-preview option passed to java? So this breaks running all non-preview using benchmarks? If so I say we need to backout the change for 8248135 while a proper solution is found. David - [1] https://bugs.openjdk.java.net/browse/JDK-8248135 Regards, Peter
Change for 8248135: Build microbenchmarks with --enable-preview makes other non-preview JMH benchmarks to fail
Hi, Recently I proposed and pushed a change for [1] which adds --enable-preview option to javac compilation of JMH microbenchmarks in general to enable running a benchmark that uses preview feature (Records). This makes the class files produced marked with version 60.65535. The benchmark that uses preview feature executes without problems because it explicitly specifies the following in its code: @Fork(value = 1, warmups = 0, jvmArgsAppend = "--enable-preview") Recently I wanted to run JMH benchmarks for Stream ops with: make test TEST="micro:java.util.stream.ops" ...but all of them fail to run with the following exception: java.lang.UnsupportedClassVersionError: Preview features are not enabled for org/openjdk/bench/java/util/stream/ops/value/generated/NoneMatchShort_seq_start_jmhTest (class file version 60.65535). Try running with '--enable-preview' What shall we do? Add similar annotation to all of them? Is there a way to specify that all micro benchmarks should be run with --enable-preview option passed to java? [1] https://bugs.openjdk.java.net/browse/JDK-8248135 Regards, Peter