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

David Holmes david.holmes at oracle.com
Tue Oct 30 07:54:43 UTC 2018

On 30/10/2018 2:54 PM, Ioi Lam wrote:
> On 10/29/18 8:16 PM, David Holmes wrote:
>> 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.
> There are plenty of tests that run the main test class multiple times 
> with different parameters using multiple @run tags. Just do a
>        grep -R '@run.*XX'
> And there are plenty of test classes whose main method accept extra 
> parameters from @run. What you described, including the difficulty of 
> running tests manually outside of jtreg, apply equally to these tests.
> What I am doing is not out of the ordinary.

Yes it is. Those other @run uses may add -XX args to othervm invocations 
but they don't pass -XX args to the Java test program. The difference is in:

-XX:Foo TheTest


TheTest -XX:Foo

> If only jtreg could make things easier ..... but that's outside of the 
> scope of this bug.

I don't see this as a jtreg issue. Your Java tests want to launch 
separate JVMs with specific sets of arguments. Maybe ProcessTools could 
help with that, but not jtreg.

>> there can't be any real args for the test.
> but these test cases don't need any "real" args.

That's fortunate isn't it. ;-) If they had needed them and you then had 
to implement arg parsing to split out the -XX args from the real ones 
would you still have used this mechanism?

>>> 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??
> What I mean is, I'd rather add new @run lines, than edit the Java code.
> If you have a better suggestion, I'd love to hear it. But the last thing 
> I want to do, is to cut-and-paste the test body multiple times, or add 
> in some sort of for loop so I can run the test body multiple times with 
> different VM parameters.

A loop is exactly what I'd suggest. Though it could perhaps be hidden in 
a ProcessTools utility function. I don't see what the issue is with a loop.

public static void main(String[] args) throws Exception {
   String[][] arglists = {
     { },
     { "-XX:+UseStringDeduplication" },
     { "-XX:-CompactStrings" }
   for (String[] vmargs : arglists) {

You're currently using jtreg plus multiple @run driver lines to get the 
effect of a loop.

Anyway just my 2c. If others don't mind this approach I won't insist on 
not using it.


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

More information about the hotspot-runtime-dev mailing list