RFR(S) 8211336 [TESTBUG] appcds tests with incorrect usage of -XX:+UseStringDeduplication

[Ioi Lam]

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.

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.

Thanks
- Ioi





> David
> -----
>
>> (I also cleaned up some unnecessary @module tags)
>>
>> Test in progress with hs-tier1/2.
>>
>> Thanks
>> - Ioi