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

Evgeniya Stepanova evgeniya.stepanova at oracle.com
Wed Nov 12 16:07:23 UTC 2014


Hi Bengt,

Please see comments inline
On 12.11.2014 19:43, Bengt Rutisson wrote:
>
> On 2014-11-12 16:21, Evgeniya Stepanova wrote:
>> Hi everyone!
>>
>> Since the decision was made to change only tests which fail because 
>> of conflict for now (skip "selfish" tests), I post new webrev for jdk 
>> part of the JDK-8019361 
>> <https://bugs.openjdk.java.net/browse/JDK-8019361>:
>> http://cr.openjdk.java.net/~avstepan/eistepan/8062536/webrev.03/
>
> Thanks for updating the webrev!
>
> A couple of comments:
>
> MemoryTestAllGC.sh
>
> The test is run three times, once with no params, once with ParallelGC 
> and once with CMS. So, I think the @requires should just be vm.gc == 
> "null". Similarly to what was done for PendingAllGC.sh.
>
The third run (with CMS) is commented. Without this run UseParallelGC is 
valid option
#runOne -XX:+UseConcMarkSweepGC MemoryTest 3
(http://cr.openjdk.java.net/~avstepan/eistepan/8062536/webrev.03/test/java/lang/management/MemoryMXBean/MemoryTestAllGC.sh.frames.html)
>
> TestInputArgument.sh
>
> The changes here seem unrelated to @requires.
>
This test was changed after conversation with David Holmes  (see thread 
below)
>
> EnqueuePollRace.java
>
> Can you explain why it is safe to remove -XX:+UseSerialGC for this test?
>
>
This test was modified after conversation with Filipp Zhinkin and Mandy 
Chung (https://bugs.openjdk.java.net/browse/JDK-8051723)
> JpsHelper.java
>
> Can you explain why it is safe to remvoe -XX:+UseParallelGC for this test?
>
This test was changed after conversation with Katja Kantserova (see 
thread below), GC flag is just an arbitrary chosen test flag
>
> When I use Aurora to check what tests that currently are considered 
> known because of JDK-8019361 I get a pretty long list:
>
> http://aurora.ru.oracle.com/functional/faces/CRRules.xhtml?cr=JDK-8019361
>
> Are the tests in webrev.03 the only tests that still fail? Have the 
> others been fixed in other ways?
There would be 2 more changes in reviews in closed part :)

Thanks,
Evgeniya Stepanova
>
> Thanks,
> Bengt
>
>
>
>>
>> Thanks,
>> Evgeniya Stepanova
>> On 07.11.2014 15:34, 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/
>>
>> -- 
>> /Evgeniya Stepanova/
>

-- 
/Evgeniya Stepanova/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20141112/06a42a04/attachment-0001.html>


More information about the hotspot-gc-dev mailing list