RFR: 8026047: [TESTBUG] add regression test for DisableExplicitGC flag
Michail Chernov
michail.chernov at oracle.com
Wed Mar 11 12:55:14 UTC 2015
Hi,
Thanks, Bengt!
Could someone else have a look on this change please?
Thanks,
Michail
On 09.03.2015 15:58, Bengt Rutisson wrote:
>
> 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