[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: Review quest for JDK-7067973: test/java/lang/management/MemoryMXBean/CollectionUsageThreshold.ja
From: Leonid Mesnik <leonid.mesnik () oracle ! com>
Date: 2013-11-28 14:17:24
Message-ID: 52975074.5070208 () oracle ! com
[Download RAW message or body]
On 11/28/2013 04:33 PM, Staffan Larsen wrote:
>
> On 28 nov 2013, at 11:16, Leonid Mesnik <leonid.mesnik@oracle.com
> <mailto:leonid.mesnik@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.
>
I agree that this is not good however it is how all works now. There are
several proposals about jtreg improvement but they are not implemented yet.
Leonid
> /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/
>>> <http://cr.openjdk.java.net/%7Eewang/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/
>>>>> <http://cr.openjdk.java.net/%7Eewang/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/
>>>>>>> <http://cr.openjdk.java.net/%7Eewang/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
>>>>>>>>> <http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a0896634ab46> 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
>
--
Leonid Mesnik
JVM SQE
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 11/28/2013 04:33 PM, Staffan Larsen
wrote:<br>
</div>
<blockquote
cite="mid:577D3085-FB34-4623-8F2F-6C001799A002@oracle.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<br>
<div>
<div>On 28 nov 2013, at 11:16, Leonid Mesnik <<a
moz-do-not-send="true"
href="mailto:leonid.mesnik@oracle.com">leonid.mesnik@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Eric, Mandy<br>
<br>
Sorry that I looping on very late step. It is not a review
just suggestion.<br>
We have whitebox API in Hotspot which includes fullGC()
method. It could be used to reliably provoke full GC.<br>
See example in <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk8/tl/hotspot/file/c9f439732b18/test/runtime/intern \
ed/SanityTest.java">http://hg.openjdk.java.net/jdk8/tl/hotspot/file/c9f439732b18/test/runtime/interned/SanityTest.java</a><br>
<br>
Should such approach works for you?<br>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>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.</div>
<br>
<blockquote type="cite">
<div bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"> <br>
Also please note that your new variant of test fails if
any of GC is set explicitly. It is incompatible with GC
setting.<br>
We set GC's and GC-related options during
Promotion/Nightly/PIT in Hotspot/SVC. For us is better if
test just works<br>
with any GC set externally. <br>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
</div>
</blockquote>
I agree that this is not good however it is how all works now. There
are several proposals about jtreg improvement but they are not
implemented yet.<br>
<br>
Leonid<br>
<blockquote
cite="mid:577D3085-FB34-4623-8F2F-6C001799A002@oracle.com"
type="cite">
<div>
<div>/Staffan</div>
<br>
<blockquote type="cite">
<div bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"> <br>
Do you need to run it with all GC each time? <br>
<br>
Leonid<br>
On 11/28/2013 09:21 AM, Eric Wang wrote:<br>
</div>
<blockquote cite="mid:5296D2C1.3080209@oracle.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Mandy,<br>
<br>
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.<br>
<br>
I didn't realized the -Xconcgc is same as <span
class="changed">-XX:+UseConcMarkSweepGC, i have
updated the webrev:</span><br>
<span class="changed"><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Eewang/JDK-7067973/webrev.00/">http://cr.openjdk.java.net/~ewang/JDK-7067973/webrev.00/</a><br>
<br>
</span>Thanks,<br>
Eric <br>
On 2013/11/27 10:17, Mandy Chung wrote:<br>
</div>
<blockquote cite="mid:52955625.4050907@oracle.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
Hi Eric,<br>
<br>
I'll defer this to the serviceability team to sponsor it
and also get one more review. <br>
<br>
I don't think you need all 7 @runs. -Xconcgc is
equivalent to setting <span \
class="changed">-XX:+UseConcMarkSweepGC</span>.
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. <br>
<br>
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?<br>
<br>
thanks<br>
Mandy<br>
<br>
<div class="moz-cite-prefix">On 11/25/2013 8:53 PM, Eric
Wang wrote:<br>
</div>
<blockquote cite="mid:52942965.40102@oracle.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Mandy,<br>
<br>
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<br>
<br>
2. for L61, that is right, the test has been
updated. please review.<br>
<a moz-do-not-send="true"
\
href="http://cr.openjdk.java.net/%7Eewang/JDK-7067973/webrev.00/">http://cr.openjdk.java.net/~ewang/JDK-7067973/webrev.00/</a><br>
<br>
Thanks,<br>
Eric<br>
On 2013/11/26 8:37, Mandy Chung wrote:<br>
</div>
<blockquote cite="mid:5293ED67.9010406@oracle.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
Hi Eric,<br>
<br>
<div class="moz-cite-prefix">On 11/24/2013 7:41 PM,
Eric Wang wrote:<br>
</div>
<blockquote cite="mid:5292C6E3.4050101@oracle.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Mandy & All,<br>
<br>
Sorry for late! <br>
The webrev below is just finished based on the
comments from peers, please help to review.<br>
<a moz-do-not-send="true"
\
href="http://cr.openjdk.java.net/%7Eewang/JDK-7067973/webrev.00/">http://cr.openjdk.java.net/~ewang/JDK-7067973/webrev.00/</a><br>
<br>
</div>
</blockquote>
<br>
Thanks for the patch that looks okay. Some
comments:<br>
<br>
L34-40: can you explain why you want to run all 7
settings? I would expect one for each collector.<br>
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).<br>
<br>
Mandy<br>
<br>
<blockquote cite="mid:5292C6E3.4050101@oracle.com"
type="cite">
<div class="moz-cite-prefix"> Thanks,<br>
Eric<br>
On 2013/11/15 10:55, Mandy Chung wrote:<br>
</div>
<blockquote cite="mid:52858D04.3080206@oracle.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
Hi Eric,<br>
<br>
<div class="moz-cite-prefix">On 11/14/2013 6:16
PM, Eric Wang wrote:<br>
</div>
<blockquote
cite="mid:52858405.2090809@oracle.com"
type="cite">
<meta http-equiv="content-type"
content="text/html; charset=windows-1252">
Hi Everyone,<br>
<br>
I'm working on the bug <a
moz-do-not-send="true"
\
href="https://bugs.openjdk.java.net/browse/JDK-7067973">https://bugs.openjdk.java.net/browse/JDK-7067973</a>.<br>
<br>
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.<br>
<br>
so the suggested fix is similar as the fix <a
moz-do-not-send="true"
\
href="http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a0896634ab46">ResetPeakMemoryUsage.java</a>
to create big object to make sure the old gen
usage crosses the threshold and run test with
different GC vmoptions.<br>
</blockquote>
<br>
What are you looking for specifically? I have
provided the above information. I need to see
the webrev to provide further feedback.<br>
<br>
Mandy<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Leonid Mesnik
JVM SQE</pre>
</div>
</blockquote>
</div>
<br>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Leonid Mesnik
JVM SQE</pre>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic