Patch looks fine (although you might want to update the comment).
It's more concerning that I didn't catch this (seems all tests of
mine were with --enable-preview), and we'll still inconvenience users
who need to run the jar file directly. So it seems we should consider
fixing so that only those benchmarks that actually need --enable-preview
are built with that flag.
/Claes
On 2020-06-27 00:21, Jorn Vernee wrote:
Forgot to attach the JBS link:
https://bugs.openjdk.java.net/browse/JDK-8248429
Jorn
On 27/06/2020 00:14, Jorn Vernee wrote:
Hi,
https://bugs.openjdk.java.net/browse/JDK-8248135 added
--enable-preview to the javac options when building micro benchmarks.
We should also add it to the set of default VM arguments passed to the
microbenchmark jar so it doesn't need to be passed manually.
If --enable-preview is not passed, the microbenchmarks can not be run
(even the ones that don't use preview features). Since the class files
have a modified minor version due to building with --enable-preview,
the VM must also be started with --enable-preview in order to be able
to load the classes. In the absence of --enable-preview, for instance
the following error will occur:
java.lang.UnsupportedClassVersionError: Preview features are not
enabled for
org/openjdk/bench/jdk/incubator/foreign/generated/CallOverhead_panama_args10_jmhTest
(class file version 60.65535). Try running with '--enable-preview'
at java.base/java.lang.ClassLoader.defineClass1(Native Method)
at
java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1016)
at
java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:151)
at
java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:825)
at
java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:723)
at
java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:646)
at
java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:604)
at
java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:168)
at
java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
at java.base/java.lang.Class.forName0(Native Method)
at java.base/java.lang.Class.forName(Class.java:377)
at org.openjdk.jmh.util.ClassUtils.loadClass(ClassUtils.java:72)
at
org.openjdk.jmh.runner.BenchmarkHandler.<init>(BenchmarkHandler.java:68)
at
org.openjdk.jmh.runner.BaseRunner.runBenchmark(BaseRunner.java:233)
at
org.openjdk.jmh.runner.BaseRunner.doSingle(BaseRunner.java:139)
at
org.openjdk.jmh.runner.BaseRunner.runBenchmarksForked(BaseRunner.java:76)
at org.openjdk.jmh.runner.ForkedRunner.run(ForkedRunner.java:72)
at org.openjdk.jmh.runner.ForkedMain.main(ForkedMain.java:84)
Please review the patch attached inline at [1].
Testing: running a microbenchmark without passing '--enable-preview'
manually and confirming that it doesn't fail to load the classes.
Thanks,
Jorn
[1] :
diff --git a/make/RunTests.gmk b/make/RunTests.gmk
index 721bb827639..59911d89e9f 100644
--- a/make/RunTests.gmk
+++ b/make/RunTests.gmk
@@ -691,7 +691,7 @@ define SetupRunMicroTestBody
endif
# Set library path for native dependencies
- $1_JMH_JVM_ARGS := -Djava.library.path=$$(TEST_IMAGE_DIR)/micro/native
+ $1_JMH_JVM_ARGS :=
-Djava.library.path=$$(TEST_IMAGE_DIR)/micro/native --enable-preview
ifneq ($$(MICRO_VM_OPTIONS)$$(MICRO_JAVA_OPTIONS), )
$1_JMH_JVM_ARGS += $$(MICRO_VM_OPTIONS) $$(MICRO_JAVA_OPTIONS)
