RFR: 8026047: [TESTBUG] add regression test for DisableExplicitGC flag

Bengt Rutisson bengt.rutisson at oracle.com
Mon Mar 9 12:58:04 UTC 2015


Hi Michail,

On 2015-03-05 13:21, Michail Chernov wrote:
> Hi Bengt,
>
> I used your approach:
> http://cr.openjdk.java.net/~eistepan/~mchernov/8026047/webrev.03/

This looks fine to me. :)

>
> assertEQ was changed to assertLT, because different GC can produce 
> different count of collections.
>
> For example:
> -XX:+UseParallelGC
> can produce:
> PS MarkSweep 1
> PS Scavenge 1
>
> -XX:+UseConcMarkSweepGC -XX:+ExplicitGCInvokesConcurrent
> can produce:
> ParNew 1
> ConcurrentMarkSweep 1

Right. If you wanted to you could check the name of the 
GarbageCollectorMXBean to filter out the old GC that result from the 
System.GC() call. But I think the test as you have it now is better 
because it is simpler.

Thanks,
Bengt

>
> Thanks,
> Michail
>
> On 04.03.2015 11:42, Bengt Rutisson wrote:
>>
>> Hi Michail,
>>
>> On 2015-03-03 16:32, Michail Chernov wrote:
>>> Hi Bengt,
>>>
>>> I checked run time of test on raspberry-pi and on some solaris host 
>>> with -Xcomp option. On r-pi it works 2.8 seconds, on solaris host it 
>>> works 3.5 seconds. I can set NOTIFICATION_DELAY=5 (for example) to 
>>> speed up the test.
>>
>> This sounds like a pretty fragile approach. My opinion is that we 
>> should always avoid tests that wait a certain amount of time for 
>> things to happen. They almost always turn out to be unstable or take 
>> unnecessarily long time.
>>
>>> Your approach has some cons - it does not check that System.gc() was 
>>> real GC source.
>>
>> Agreed. But since we actually don't have any problems with the 
>> DisbableExplicitGC flag I don't think it is worth adding a test that 
>> is potentially unstable and complex to test it. If we feel that we 
>> really need to test this flag I think the test must be simple and fast.
>>
>>> In case if we don't check source of GC events, we can simplify test 
>>> more and avoid usage of JMX:
>>>
>>> import java.lang.ref.WeakReference;
>>>
>>> public class Test{
>>>     public static volatile WeakReference<Object> ref;
>>>     public static Object refObject;
>>>     public static void main(String[] args) {
>>>         ref=new WeakReference<>(new Object());
>>>         refObject=ref.get();
>>>         if ( refObject==null)
>>>             throw new RuntimeException("ERROR! Object was collected 
>>> before GC");
>>>         refObject=null;
>>>         System.gc();
>>>         refObject=ref.get();
>>>         if ( refObject!=null)
>>>             throw new RuntimeException("ERROR! Object was not 
>>> collected during GC");
>>>     }
>>> }
>>>
>>> However, this solution does not check the real cause of GC. If this 
>>> way is applicable here, I could use this code for testing 
>>> DisableExplicitGC flag.
>>
>> Sure, this will probably work, but I think it is kind of a strange 
>> way to check this. I prefer a more explicit way of testing.
>>
>> Thanks,
>> Bengt
>>
>>>
>>> Thanks,
>>> Michail
>>>
>>> On 03.03.2015 11:02, Bengt Rutisson wrote:
>>>>
>>>> Hi Michail,
>>>>
>>>> I like the idea of using the GarbageCollectionMXBean!
>>>>
>>>> However, I am not too happy about the test waiting for 20 seconds. 
>>>> Especially since you have the failing test which will actually wait 
>>>> for 20 seconds.
>>>>
>>>> Instead I would suggest to just use the collection count. Attaching 
>>>> a version of the test that use that instead. What do you think 
>>>> about this approach?
>>>>
>>>> Thanks,
>>>> Bengt
>>>>
>>>>
>>>>
>>>> On 2015-02-25 13:42, Michail Chernov wrote:
>>>>> Hi Bengt,
>>>>>
>>>>> I've rewritten test using JMX. I don't see here any reason to use 
>>>>> gc log for testing this flag.
>>>>>
>>>>> http://cr.openjdk.java.net/~eistepan/~mchernov/8026047/webrev.02/
>>>>>
>>>>> It seems better solution because it doesn't depend on used GC or 
>>>>> log message format.
>>>>>
>>>>> Tested locally with JDK 9 b51 using several GC. Tested using 
>>>>> Aurora on all platforms.
>>>>>
>>>>> Thanks,
>>>>> Michail
>>>>>
>>>>> On 12.02.2015 17:07, Bengt Rutisson wrote:
>>>>>>
>>>>>> Hi Michail,
>>>>>>
>>>>>> On 11/02/15 16:33, Michail Chernov wrote:
>>>>>>> Hi Bengt,
>>>>>>>
>>>>>>> Test works with all options passed to jtreg during testing ( see 
>>>>>>> line 97         vmOpts.addAll(0, Utils.getVmOptions());). 
>>>>>>> Doesn't need to check all GC's, it will be done during nightly.
>>>>>>
>>>>>> Ah. I see that now.
>>>>>>
>>>>>>>
>>>>>>> 44     public final static String[] PARALLEL_GC_OPTIONS = 
>>>>>>> {"UseParallelGC", "UseParallelOldGC"};
>>>>>>> Here is defined special options. If define one of these options 
>>>>>>> - will be used other pattern to match output.
>>>>>>
>>>>>> Right.
>>>>>>
>>>>>>>
>>>>>>> But it seems to me a little bit wrong. I've checked output of GC 
>>>>>>> log with different GCs and ExplicitGCInvokesConcurrent. Message 
>>>>>>> "Full GC (System.gc())" does not appear only in case of using G1 
>>>>>>> or CMS with ExplicitGCInvokesConcurrent=true. Will fix it.
>>>>>>
>>>>>> OK.
>>>>>>
>>>>>> My main point was that I think the whole structure of the test is 
>>>>>> different than how we usually write tests that verify the log 
>>>>>> output. I would prefer if the tests look similar. Would you mind 
>>>>>> re-writing the test to look like the other tests.
>>>>>>
>>>>>> I much prefer that you explicitly start with the GCs you want to 
>>>>>> test than that you use the WhiteBox API to find out which GC you 
>>>>>> are running.
>>>>>>
>>>>>> I'm having a similar discussion with Dima who recently invented 
>>>>>> yet another way to parse the GC log output from tests. I think it 
>>>>>> gets very hard to read the tests when they do the same thing in 
>>>>>> different ways.
>>>>>>
>>>>>> Thanks,
>>>>>> Bengt
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Michail
>>>>>>> On 11.02.2015 16:15, Bengt Rutisson wrote:
>>>>>>>>
>>>>>>>> Hi Michail,
>>>>>>>>
>>>>>>>> On 11/02/15 13:55, Michail Chernov wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Still hoping for review!
>>>>>>>>
>>>>>>>> Sorry for being so late in looking at this.
>>>>>>>>
>>>>>>>> A couple of questions:
>>>>>>>>
>>>>>>>> Why does the test only test the parallel GC? DisableExplicitGC 
>>>>>>>> is valid for all GCs.
>>>>>>>>
>>>>>>>> What do you think about writing this test similar to other 
>>>>>>>> tests that validate the output from the GC logging? Here's an 
>>>>>>>> example:
>>>>>>>>
>>>>>>>> http://hg.openjdk.java.net/jdk9/hs-gc/hotspot/file/566574421b40/test/gc/g1/TestGCLogMessages.java 
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Bengt
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Michail
>>>>>>>>>
>>>>>>>>> On 05.02.2015 21:05, Michail Chernov wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Still waiting for reviews!
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Michail
>>>>>>>>>>
>>>>>>>>>> On 03.02.2015 20:12, Michail Chernov wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Can someone take a look on these changes, please?
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Michail
>>>>>>>>>>>
>>>>>>>>>>> On 30.01.2015 18:33, Michail Chernov wrote:
>>>>>>>>>>>> Hi Leonid,
>>>>>>>>>>>>
>>>>>>>>>>>> Issues were fixed:
>>>>>>>>>>>> http://cr.openjdk.java.net/~eistepan/~mchernov/8026047/webrev.01/ 
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Now all testcases are executed  from the same VM.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Michail
>>>>>>>>>>>>
>>>>>>>>>>>> On 28.01.2015 18:28, Leonid Mesnik wrote:
>>>>>>>>>>>>> Hi
>>>>>>>>>>>>>
>>>>>>>>>>>>> Why is it needed to start VM twice for each test. It is 
>>>>>>>>>>>>> very expensive especially for low-end devices.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Is it possible to have driver which starts VM several 
>>>>>>>>>>>>> times with different combinations of options and check it 
>>>>>>>>>>>>> output/exit code etc? Also it would be much easier to read 
>>>>>>>>>>>>> such test.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Leonid
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 27.01.2015 18:35, Michail Chernov wrote:
>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Please review the fix with new test for DisableExplicitGC 
>>>>>>>>>>>>>> VM flag.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Webrev: 
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~eistepan/~mchernov/8026047/webrev.00/ 
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Enhancement: 
>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8026047
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> There is one scenario with 6 parameters combinations.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 1,2,3 scenarios test default value for DisableExplicitGC, 
>>>>>>>>>>>>>> DisableExplicitGC=true and DisableExplicitGC=false
>>>>>>>>>>>>>> 4,5,6 scenarios check how VM works when VM changes 
>>>>>>>>>>>>>> DisableExplicitGC flag using WhiteBox.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Test tries to call System.gc() and check that VM puts 
>>>>>>>>>>>>>> message to stdout. After (in case of 4,5,6 scenarios) 
>>>>>>>>>>>>>> test tries to change DisableExplicitGC value and calls 
>>>>>>>>>>>>>> System.gc() twice.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Test was executed locally on linux-i586 with all 
>>>>>>>>>>>>>> available GC and several GC-related flags. Also it was 
>>>>>>>>>>>>>> executed using Aurora on other platforms.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Michail
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>>
>>
>




More information about the hotspot-gc-dev mailing list