RFR(S) 8211336 [TESTBUG] appcds tests with incorrect usage of -XX:+UseStringDeduplication (original) (raw)
David Holmes david.holmes at oracle.com
Tue Oct 30 03:16:55 UTC 2018
- Previous message: RFR(S) 8211336 [TESTBUG] appcds tests with incorrect usage of -XX:+UseStringDeduplication
- Next message: RFR(S) 8211336 [TESTBUG] appcds tests with incorrect usage of -XX:+UseStringDeduplication
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On 30/10/2018 12:23 PM, Ioi Lam wrote:
On 10/29/18 6:23 PM, David Holmes wrote:
Hi Ioi,
On 30/10/2018 7:32 AM, Ioi Lam wrote: http://cr.openjdk.java.net/~iklam/jdk12/8211336-cds-test-fix-for-string-dedup.v01/
https://bugs.openjdk.java.net/browse/JDK-8211336 Please review this small test fix: These are "shell" tests that need to pass certain options (such as -XX:+UseStringDeduplication) to child JVM processes that are launched via jdk.test.lib.process.ProcessTools. The tests were using this: * @run main/othervm -XX:+UseStringDeduplication ExerciseGC However, options passed this way are not passed by ProcessTools. The fix is to change to the following * @run driver ExerciseGC -XX:+UseStringDeduplication and let the main test program explicitly pass args[] (indirectly) to ProcessTools. That looks really weird to me. I would not expect to pass things this way. If the test is only intending to launch other VMs then the logic to add -XX:+UseStringDeduplication should be an explicit part of the test, directly in the main test logic, not incidentally passed through via the mechanism you suggest. The 11 tests each originally had only one @run flag, like * @run driver ExerciseGC Later, it was suggested (JDK-8185531) to add more flag combinations, such as -XX:+UseStringDeduplication and -XX:-CompactStrings, to increase test coverage. It's pretty common to do this by adding a new @run tag with different flags, as done in JDK-8185531. The only problem is JDK-8185531 doesn't work because jtreg doesn't pass the additional flags to the child processes. Hence the fix in this RFR.
Understood. But tests that launch other VMs to do the real testing generally manage the flag coverage themselves. The way you have done it here just seems really bizarre and non-obvious i.e. you have to realize that the intended flags get passed via the args to main and there can't be any real args for the test. And its tricky to run outside jtreg as you have to manually.
I see two AOT tests that employ a similar scheme, but I don't like it.
This way it's easy to add other VM flag combinations to the tests. You don't need to modify the test sources identically 11 times just to add a new flag combination to these 11 tests.
Umm how do you not have to modify the test source identically 11 times when you have to add the new arg to the "@run driver ..." command, or add a new @run driver line??
Thanks, David
Thanks - Ioi
David ----- (I also cleaned up some unnecessary @module tags) Test in progress with hs-tier1/2. Thanks - Ioi
- Previous message: RFR(S) 8211336 [TESTBUG] appcds tests with incorrect usage of -XX:+UseStringDeduplication
- Next message: RFR(S) 8211336 [TESTBUG] appcds tests with incorrect usage of -XX:+UseStringDeduplication
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]