RFR: 8026047: [TESTBUG] add regression test for DisableExplicitGC flag
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Wed Mar 11 17:14:29 UTC 2015
Hi Misha,
I guess you could save a couple of lines by merging
41 long collectionCountBefore = 0;
42 collectionCountBefore = getCollectionCount(list);
into
41 long collectionCountBefore = getCollectionCount(list);
And in lines 44, 45 as well.
Besides that it looks good. I don't need to see a new webrev if you decide to do
this change.
Thanks,
/Jesper
Michail Chernov skrev den 11/3/15 13:55:
> 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