Fwd: Re: RFR: 8062536: [TESTBUG] Conflicting GC combinations in jdk tests

Filipp Zhinkin filipp.zhinkin at oracle.com
Fri Nov 7 10:35:17 UTC 2014


Thank you for taking care of it,
the fix looks good to me (not a R-reviewer).

Thanks,
Filipp.

On 11/07/2014 03:34 PM, Evgeniya Stepanova wrote:
> David, Filipp, Katja
>
> Diff have been updated one more time:
> java/lang/management/RuntimeMXBean/TestInputArgument.sh and 
> test/java/lang/ref/EnqueuePollRace.java have been changed
> http://cr.openjdk.java.net/~eistepan/8062536/webrev.02/
>
> Thanks
>
> On 07.11.2014 9:37, David Holmes wrote:
>> On 7/11/2014 12:36 AM, Evgeniya Stepanova wrote:
>>> New webrev:
>>> http://cr.openjdk.java.net/~eistepan/8062536/webrev.01/
>>
>> In:
>>
>> test/java/lang/management/RuntimeMXBean/TestInputArgument.sh
>>
>> the use of the gc options seems incidental - it's just picking two innocuous 
>> options to use - similar to the JpsHelper case. You could replace 
>> +UseParallelGC with something like +UseFastJNIAccessors (platform independent 
>> and normally true).
>>
>> David
>> -----
>>
>>> Thanks,
>>> Evgeniya Stepanova
>>> On 06.11.2014 17:35, Yekaterina Kantserova wrote:
>>>> Thanks a lot!
>>>>
>>>> On 11/06/2014 02:05 PM, Evgeniya Stepanova wrote:
>>>>> Hi Katja,
>>>>>
>>>>> Ok, this seems to be a perfect solution. Thank you. I'll change the
>>>>> diff accordingly.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Evgeniya Stepanova
>>>>> On 06.11.2014 16:56, Yekaterina Kantserova wrote:
>>>>>> Hi Dima,
>>>>>>
>>>>>> On 11/06/2014 11:22 AM, Dmitry Fazunenko wrote:
>>>>>>> Hi Katja,
>>>>>>>
>>>>>>> You are right, there will be no conflict, because test ignores any
>>>>>>> external VM flags.
>>>>>>> So, adding @requires seems unnecessary here, but...
>>>>>>>
>>>>>>> Ignoring external options is bad thing, such "selfish" tests are
>>>>>>> not applicable for other areas, like GC, compiler, RT.
>>>>>>
>>>>>> This particular case is to test the defined flags are shown up as
>>>>>> expected.
>>>>>>
>>>>>> Evgeniya,
>>>>>>
>>>>>> would you mind to change JpsHelper.java instead?
>>>>>>
>>>>>> +++ b/test/sun/tools/jps/JpsHelper.java
>>>>>> @@ -93,7 +93,7 @@
>>>>>>      /**
>>>>>>       * VM arguments to start test application with
>>>>>>       */
>>>>>> -    public static final String[] VM_ARGS = {"-Xmx512m",
>>>>>> "-XX:+UseParallelGC"};
>>>>>> +    public static final String[] VM_ARGS = {"-Xmx512m",
>>>>>> "-XX:+PrintGCDetails"};
>>>>>>      /**
>>>>>>       * VM flag to start test application with
>>>>>>       */
>>>>>>
>>>>>> Best regards,
>>>>>> Katja
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> @requires  will allow to modify tests to include external vm
>>>>>>> options without any risk of bumping into conflict and extend area
>>>>>>> of test applicability.
>>>>>>>
>>>>>>> But if you still believe, that @requires is not necessary - it's
>>>>>>> not a problem, tests could be kept as is.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Dima
>>>>>>>
>>>>>>>
>>>>>>> On 06.11.2014 16:27, Yekaterina Kantserova wrote:
>>>>>>>>
>>>>>>>> Hi Evgeniya,
>>>>>>>>
>>>>>>>> As David has pointed out these jps tests are not testing gc. The
>>>>>>>> -XX:+UseParallelGC is just an arbitrary chosen test flag. There
>>>>>>>> should not be any conflicts either since these tests are running
>>>>>>>> in driver mode:
>>>>>>>>
>>>>>>>> ...
>>>>>>>>  * @run driver TestJpsJar
>>>>>>>> ...
>>>>>>>>
>>>>>>>> which means no flags from above are accepted.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Katja
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/06/2014 11:05 AM, Evgeniya Stepanova wrote:
>>>>>>>>> Hi David,
>>>>>>>>>
>>>>>>>>> tag added because tests contain string
>>>>>>>>>  cmd.addAll(JpsHelper.getVmArgs());
>>>>>>>>>
>>>>>>>>> and JpsHelper defines
>>>>>>>>> ...
>>>>>>>>> public static final String[] VM_ARGS = {"-Xmx512m",
>>>>>>>>> "-XX:+UseParallelGC"};
>>>>>>>>> ...
>>>>>>>>> public static List<String> getVmArgs() throws IOException {
>>>>>>>>>         if (testVmArgs == null) {
>>>>>>>>>             testVmArgs = new ArrayList<>();
>>>>>>>>> testVmArgs.addAll(Arrays.asList(VM_ARGS));
>>>>>>>>>             testVmArgs.add("-XX:Flags=" +
>>>>>>>>> getVmFlagsFile().getAbsolutePath());
>>>>>>>>>         }
>>>>>>>>>         return testVmArgs;
>>>>>>>>>     }
>>>>>>>>>
>>>>>>>>> Tests itself wouldn't fail if we use another GC, tag added for
>>>>>>>>> cleanup-if we use for example SerialGC we must be sure that tests
>>>>>>>>> passed with this GC, not with another one. Now you will assume
>>>>>>>>> that concrete test passed with Serial GC, but it run only with
>>>>>>>>> Parallel GC. Plus there is no any sense to run test twice in TC
>>>>>>>>> (with different GC, since it use only Parallel)
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Evgeniya Stepanova
>>>>>>>>> On 06.11.2014 6:20, David Holmes wrote:
>>>>>>>>>> Hi Evgeniya,
>>>>>>>>>>
>>>>>>>>>> On 6/11/2014 1:33 AM, Evgeniya Stepanova wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Please review changes for 8062536, the OpenJDK/jdk part of the
>>>>>>>>>>> JDK-8019361
>>>>>>>>>>>
>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8062536
>>>>>>>>>>> fix: http://cr.openjdk.java.net/~eistepan/8062536/webrev.00/
>>>>>>>>>>>
>>>>>>>>>>> Problem: Some tests explicitly set GC and fail when another GC
>>>>>>>>>>> is set
>>>>>>>>>>> outside
>>>>>>>>>>
>>>>>>>>>> I don't see why you have done this for the
>>>>>>>>>>
>>>>>>>>>> test/sun/tools/jps/TestJps*.java
>>>>>>>>>>
>>>>>>>>>> tests. They don't set any GC flags.
>>>>>>>>>>
>>>>>>>>>>> Solution: Such tests marked with the jtreg tag "requires" to
>>>>>>>>>>> skip test
>>>>>>>>>>> if there is a conflict
>>>>>>>>>>
>>>>>>>>>> Just wondering: Does a skipped test get a .jtr file showing it
>>>>>>>>>> was skipped; or does it only appear in the higher-level jtreg log?
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> David
>>>>>>>>>>
>>>>>>>>>>> Tested locally with different GC flags (-XX:+UseG1GC,
>>>>>>>>>>> -XX:+UseParallelGC, -XX:+UseSerialGC, -XX:+UseConcMarkSweep and
>>>>>>>>>>> without
>>>>>>>>>>> any GC flag). Tests are being excluded as expected. No tests
>>>>>>>>>>> failed
>>>>>>>>>>> because of the conflict.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Evgeniya Stepanova
>>>>>>>>>>>
>>>>>>>>>>> //
>>>>>>>>>
>>>>>>>>> -- 
>>>>>>>>> /Evgeniya Stepanova/
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>> -- 
>>>>> /Evgeniya Stepanova/
>>>>
>>>
>>> -- 
>>> /Evgeniya Stepanova/
>
> -- 
> /Evgeniya Stepanova/

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20141107/27bd0cd1/attachment-0001.html>


More information about the hotspot-gc-dev mailing list