RFR (XS): 8078673: Update TEST.groups for recent GC tests

Derek White derek.white at oracle.com
Tue Feb 23 16:41:38 UTC 2016


[RFR back from the dead...]

I think last webrev for this was updated to follow the consensus 
opinion, but it never received thumbs up/down from reviewers.

I've created a new webrev for critical copyright notice updates.

Summary:
1) Updated the "needs_g1gc" list in TEST.groups to prevent G1 tests 
running in embedded.
2) Added appropriate "@requires vm.gc" annotations to a few GC tests 
that use the process builder and pass in a collector explicitly. The new 
@requires annotations prevent unexpected GC-specific tests running when 
a conflicting collector argument is specified by the outer test harness.

CR:
https://bugs.openjdk.java.net/browse/JDK-8078673

Webrev:
http://cr.openjdk.java.net/~drwhite/8078673/webrev.04/

Testing:
   JPRT

Thanks!
  - Derek

On 7/13/15 5:37 PM, Derek White wrote:
> On 7/12/15 10:38 PM, David Holmes wrote:
>> Hi Derek,
>>
>> On 11/07/2015 1:28 AM, Derek White wrote:
>>> Please review this partial fix for GC tests that require certain
>>> collectors (e.g. shouldn't run in embedded).
>>>
>>> This is an updated webrev to account for Leonid's fix for "8079134:
>>> [TESTBUG] Remove applicable_*gc and needs_*gc groups from TEST.groups",
>>> which removed a pile of TEST.groups lists including needs_gc,
>>> needs_serialgc, needs_parallelgc, and needs_cmsgc.
>>>
>>> Now the fix simply updates the needs_g1gc list in TEST.groups and adds
>>> appropriate "@requires vm.gc" annotations to a few GC tests.
>>>
>>> Note that the "@requires vm.gc" changes are /almost/ purely 
>>> documentary.
>>> These are ProcessBuilder-based tests, so any "-XX:+UsexxxGC" flags
>>> passed in by jtreg are ignored. It's very confusing, as well as
>>> unnecessary, for a jtreg run specifying -XX:+UseG1GC to be running a
>>> test that then replaces the flag with "-XX:+UseParallelGC" (etc).
>>
>> So even though we would never pass through the jtreg specific GC 
>> option we will skip these tests if that option doesn't match with the 
>> GC's the test will be testing. As long as that doesn't lead to these 
>> tests being untested that seems okay.
> In all of the tests I modified, it always runs the test if jtreg did 
> not specify any GC. Most of the tests will also run if jtreg specifies 
> the same GC as the test. So we should have test coverage.
>>
>> But really this highlights a basic problem we have with our approach 
>> to testing with different VM options. Unless the GC option is the 
>> only option that changes across the runs you would want the other 
>> options to be passed through to the actual tests in many cases. :(
>
> Some test do pass along the outer (jtreg) arguments, and some don't. 
> So the fix with "@requires vm.gc" removes the possibility of 
> conflicting GC's being specified in that case. There could still be 
> conflicts between other arguments specified by jtreg vs. the 
> ProcessBuilder. If the ProcessBuilder arguments were constant, they 
> could also be specified by |"@requires vm.opt." annotations, but this 
> leaves out many cases.
>
> I'm also not satisfied by the current state of affairs, but I think 
> this fix gets us slightly closer to what we want.
>
>  - Derek
>
> |
>>
>>> *CR:*
>>> https://bugs.openjdk.java.net/browse/JDK-8078673
>>>
>>> *Webrev:*
>>> http://cr.openjdk.java.net/~drwhite/8078673/webrev.03/
>>>
>>> *Testing:*
>>>   JPRT
>>>
>>> *Open review comments:*
>>> David H: Your last comments on this subject requested changes to the
>>> "needs_gc" list, which has been removed by 8079134.
>>>
>>> Kim and Jesper: I agree with your comments about wanting some better 
>>> for
>>> both testing multiple GCs and dealing with SE Embedded properly in the
>>> testing infrastructure. This webrev is simply a good fix within the
>>> existing infrastructure.
>>>
>>> Thanks,
>>>   - Derek
>>>
>>> On 4/29/15 5:03 PM, Kim Barrett wrote:
>>>> On Apr 29, 2015, at 4:38 PM, Jesper 
>>>> Wilhelmsson<jesper.wilhelmsson at oracle.com> wrote:
>>>>> Hi,
>>>>>
>>>>> There are tests like 
>>>>> hotspot/test/gc/g1/TestShrinkAuxiliaryData**.java where there is a 
>>>>> base class that provides the test and a bunch of test classes that 
>>>>> only starts the base test with different arguments. This case 
>>>>> would be similar but slightly more ugly since the actual code 
>>>>> would be the same and trigger the same base test, but with 
>>>>> different @requires in the comment above the test.
>>>>>
>>>>> I'm not sure it would help though. What we really would need here 
>>>>> is a @requires that could check the host name or the hardware 
>>>>> platform or OS.
>>>> @requires has os.{name, family, arch, simpleArch} properties that 
>>>> can be tested.  But I’m not sure any of those are really right for 
>>>> testing for “embedded system” (whatever that actually means).
>>>>
>>>>> Kim Barrett skrev den 29/4/15 20:35:
>>>>>> On Apr 29, 2015, at 2:06 PM, Derek White<derek.white at oracle.com> 
>>>>>> wrote:
>>>>>>> So most of these tests use ProcessBuilder to specify a command 
>>>>>>> line, and explicitly mention a GC to use. A single test might 
>>>>>>> actually run each collector (gc/logging/TestGCId, for example). 
>>>>>>> Does @requires matter in this case?
>>>>>>>
>>>>>>> Oh, maybe they should all have @requires vm.gc=="null", becuase 
>>>>>>> the actual test is ignoring GC passed in by the test harness GC 
>>>>>>> and running as a separate process anyway. It's misleading if the 
>>>>>>> harness said UseG1GC and the test was actually running 
>>>>>>> UseParallelGC, for example.
>>>>>> That’s one solution.  A different solution would be to clone into 
>>>>>> multiple tests, one for each relevant collector, where the vm.gc 
>>>>>> can be “null” or the corresponding collector.  That cloning is 
>>>>>> kind of ugly though; it’s too bad there can only be one @requires 
>>>>>> constraint per test, rather than per @run line or the like.  But 
>>>>>> we didn’t get any traction with such a 
>>>>>> suggestion:https://bugs.openjdk.java.net/browse/CODETOOLS-7901090.
>>>>>>
>>>>>>> FYI, it sounds like my original problem does require the exclude 
>>>>>>> lists to keep the embedded JVM from running GC tests that it 
>>>>>>> shouldn't.
>>>>>> I’m not sure how to address this problem.  For example, we don’t 
>>>>>> want to exclude TestSmallHeap.java on embedded JVMs, we just want 
>>>>>> to exclude its sub-cases that would attempt to use a gc that 
>>>>>> isn’t supported.
>>>>
>>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160223/e7b7207e/attachment.htm>


More information about the hotspot-gc-dev mailing list