Hi Jorn,
On 2020-06-30 14:52, Jorn Vernee wrote:
Hi Claes,
I see what you mean.
I've created a patch that instead greps through all the benchmark source
files, and finds files with `--enable-preview` in them. Then, only those
files are compiled with --enable-preview, by using a separate call to
SetupJavaCompilation.
This relies on the fact that the benchmarks that use preview features
also use `@Fork(... jvmAppendArgs= "--enable-preview")`, but, maybe a
different marker can be used to mark benchmarks that need to be compiled
with --enable-preview as well. Alternatively, we could use 2 separate
directory structures to house preview and non-preview benchmarks. WDYT?
Webrev: http://cr.openjdk.java.net/~jvernee/8248429/webrev.00/
this looks great to me!
Testing: deleting build/<config>/support/test/micro and
build/<config>/images/test/micro and confirming that compiling and
running benchmarks with and without preview features works as expected.
I don't think there's any automated tests for benchmarks right?
Not really, no, but a tier1 build and test would at least build the
micros and detect any issues with your shell calls on our range of
platforms.
While we support running the per-build micro artifacts in our internal
benchmarking system on an adhoc basis, our promotion testing (where this
would have been discovered) is still a bit semi-automatic in regards to
when we roll over to a new benchmarks.jar.
/Claes
Jorn
On 27/06/2020 01:38, Claes Redestad wrote:
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)
