RFR: 8237111: LingeredApp should be started with getTestJavaOpts

Stefan Karlsson stefan.karlsson at oracle.com
Tue Jan 21 14:58:53 UTC 2020


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






More information about the serviceability-dev mailing list