On Thursday 28 November 2013 13.33.48 Staffan Larsen wrote: > 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.
Förutom om SQE injicerar commandline-flaggor i tester, typ -XX: +ExplicitGCInvokesConcurrent, eller rentav -XX:+DisableExpicitGC... tycker inte att ni ska ändra till WBapi-fullgc, men det är nog därifrån Leonid kommer. /m > > 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
