Hi all,
Please review this patch to change our usages of LingeredApp and
getVmOptions() to instead use getTestJavaOpts().
https://cr.openjdk.java.net/~stefank/8237111/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-8237111
This issue was encountered by both Coleen and I, independently.
We have two ways to pass JVM flags to jtreg. They come with different
names depending on the test layer (make, jtreg, test.lib etc):
1) Utils.getVmOptions(), -vmoptions, -Dtest.vm.options, VM_OPTIONS, ...
Is passed to all JVMs (not only the one under test)
2) -javaoptions, -Dtest.java.options, JAVA_OPTIONS, TEST_JAVA_OPTS, ...
Is passed to tested JVM
The problem is that mach5 uses the latter to propagate JVM flags, so
when tests explicitly uses Utils.getVmOptions() they won't run with the
specified flags.
The problem also arise if someone runs the following on the command line:
make -C ../build/fastdebug test
TEST=test/hotspot/jtreg/serviceability/sa/DeadlockDetectionTest.java
JTREG="JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions -XX:+UseZGC"
There's no Utils.getJavaOptions() function that fetches the (2) flags,
but there is a Utils.getTestJavaOpts() function that fetches flags from
both (1) and (2).
The proposal is to stop using (and remove) Utils.getVmOptions() and
instead use Utils.getTestJavaOpts(). This patch touches more than
LingeredApp, so we should probably rename it.
Some details about the patch:
- LingeredApp.startApp() now runs with getTestJavaOpts().
- getVmOptions() returned a List<String> and getTestJavaOpts() returns a
String[]. I've adapted the code to use String[] instead.
- Changed the parameter list of LingeredApp so that we could use
String..., and lower the amount of boiler plate code.
- Removed Utils.getVmOptions()
- Left Utils.getForwardVmOptions() for now, because replacing it
requires changes that needs to be reviewed on other lists.
- Added appendTestJavaOpts and prependTestJavaOpts since the order is
important to tests.
- Left addTestJavaOpts for now, because replacing it requires changes
that needs to be reviewed on other lists.
- Excluded some ZGC SA tests, because now we actually run with ZGC when
we ask for it.
- JMapHeapConfigTest.java is broken when (jlong)-1 is passed in a flag.
This prevented ZGC from running, because we set MaxNewSize to max
size_t. Apparently, someone had already noticed this problem with
MaxMetaspaceSize and added this cryptic line:
// ignoring MaxMetaspaceSize
I did the same for MaxNewSize and created a bug report.
- There are two instances of LingeredApp. I fixed both and created an
enhancement to combine the two classes.
- ClhsdbFlags.runAllTypesTest used to *append* getVmOptions(). This
started failing when I changed to getTestJavaOpts() because in some
configs we override some of the flags in the test. I fixed it by
*prepending* the getTestJavaOpts().
Tested with various tiers, but not on the absolute latest patch. Will
run this through more testing when the review is done.
Thanks,
StefanK
