On 28 nov 2013, at 11:16, Leonid Mesnik <leonid.mes...@oracle.com> wrote:
> Eric, Mandy > > Sorry that I looping on very late step. It is not a review just suggestion. > We have whitebox API in Hotspot which includes fullGC() method. It could be > used to reliably provoke full GC. > See example in > http://hg.openjdk.java.net/jdk8/tl/hotspot/file/c9f439732b18/test/runtime/interned/SanityTest.java > > Should such approach works for you? It’s not necessary. System.gc is implemented as a full GC for all of our GC implementations. I don’t think there is a problem in relying on that fact. > > Also please note that your new variant of test fails if any of GC is set > explicitly. It is incompatible with GC setting. > We set GC's and GC-related options during Promotion/Nightly/PIT in > Hotspot/SVC. For us is better if test just works > with any GC set externally. This is broken (as has been discussed many times). Tests *need* to be able to provide their own flags without someone overriding them and still expecting the test to work. /Staffan > > Do you need to run it with all GC each time? > > Leonid > On 11/28/2013 09:21 AM, Eric Wang wrote: >> Hi Mandy, >> >> Yes, I have tested and all settings are passed, as you mentioned the test >> hangs with -XX:+UseG1GC -XX:+ExplicitGCInvokesConcurrent and default heap >> size as no GC happens on Old Gen. That is why to add -Xmx2m and big object >> to make sure GC happens. >> >> I didn't realized the -Xconcgc is same as -XX:+UseConcMarkSweepGC, i have >> updated the webrev: >> http://cr.openjdk.java.net/~ewang/JDK-7067973/webrev.00/ >> >> Thanks, >> Eric >> On 2013/11/27 10:17, Mandy Chung wrote: >>> Hi Eric, >>> >>> I'll defer this to the serviceability team to sponsor it and also get one >>> more review. >>> >>> I don't think you need all 7 @runs. -Xconcgc is equivalent to setting >>> -XX:+UseConcMarkSweepGC. For G1 and CMS, you should use >>> -XX:+ExplicitGCInvokesConcurrent so that System.gc will force a GC in >>> foreground that you can count the GC reliably. The test wants to get >>> notified for each System.gc and if there is any GC caused by allocation >>> failure, the test would fail due to the unexpected GC count. It seems that >>> you may run into this issue setting -Xmx2m. >>> >>> Have you got the test passed in all settings? I'm seeing that the test >>> hangs with -XX:+UseG1GC -XX:+ExplicitGCInvokesConcurrent without -Xmx2m. >>> Looks like there is no GC in the old gen - I'm not familiar with G1 if it >>> allocates the big object in the old gen. Jarolsav - can you help Eric >>> diagnose this issue? I recalled you ran into something like that before - >>> maybe Staffan? >>> >>> thanks >>> Mandy >>> >>> On 11/25/2013 8:53 PM, Eric Wang wrote: >>>> Hi Mandy, >>>> >>>> 1. for L34-40, executing tests with 7 settings is trying to cover more >>>> cases (normal cases and special cases), especially last 3 settings, as >>>> found that the test hung if using vm option >>>> "-XX:+ExplicitGCInvokesConcurrent" with one of 3 options -XX:+UseG1GC, >>>> -XX:+UseConcMarkSweepGC or -Xconcgc >>>> >>>> 2. for L61, that is right, the test has been updated. please review. >>>> http://cr.openjdk.java.net/~ewang/JDK-7067973/webrev.00/ >>>> >>>> Thanks, >>>> Eric >>>> On 2013/11/26 8:37, Mandy Chung wrote: >>>>> Hi Eric, >>>>> >>>>> On 11/24/2013 7:41 PM, Eric Wang wrote: >>>>>> Hi Mandy & All, >>>>>> >>>>>> Sorry for late! >>>>>> The webrev below is just finished based on the comments from peers, >>>>>> please help to review. >>>>>> http://cr.openjdk.java.net/~ewang/JDK-7067973/webrev.00/ >>>>>> >>>>> >>>>> Thanks for the patch that looks okay. Some comments: >>>>> >>>>> L34-40: can you explain why you want to run all 7 settings? I would >>>>> expect one for each collector. >>>>> L61: I think the static checker variable is meant to be a local variable >>>>> (and looks like "pools" and "managers" don't need to be static variable). >>>>> >>>>> Mandy >>>>> >>>>>> Thanks, >>>>>> Eric >>>>>> On 2013/11/15 10:55, Mandy Chung wrote: >>>>>>> Hi Eric, >>>>>>> >>>>>>> On 11/14/2013 6:16 PM, Eric Wang wrote: >>>>>>>> Hi Everyone, >>>>>>>> >>>>>>>> I'm working on the bug >>>>>>>> https://bugs.openjdk.java.net/browse/JDK-7067973. >>>>>>>> >>>>>>>> It is a test bug as the test doesn't guarantee memory allocated from >>>>>>>> the Old Gen, if the used memory is zero and doesn't cross the >>>>>>>> threshold, no notification is sent, so both the main thread and >>>>>>>> Checker thread are blocked to wait for the GC notification. >>>>>>>> >>>>>>>> so the suggested fix is similar as the fix ResetPeakMemoryUsage.java >>>>>>>> to create big object to make sure the old gen usage crosses the >>>>>>>> threshold and run test with different GC vmoptions. >>>>>>> >>>>>>> What are you looking for specifically? I have provided the above >>>>>>> information. I need to see the webrev to provide further feedback. >>>>>>> >>>>>>> Mandy >>>>>> >>>>> >>>> >>> >> > > > -- > Leonid Mesnik > JVM SQE
