<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi<br>
Thanks for comments.<br>
I have completely rework this tests according to comments.<br>
Also we have found that it will be easier to follow standard
work-flow of submitting patches to branch 9 and then back-porting it
to 8 and 7.<br>
So, here is webrev against
<a class="moz-txt-link-freetext" href="http://hg.openjdk.java.net/jdk9/hs-gc/hotspot">http://hg.openjdk.java.net/jdk9/hs-gc/hotspot</a> <br>
I hope, Jesper will help me to upload it right place.<br>
<br>
Thanks.<br>
<br>
<br>
<div class="moz-cite-prefix">On 05.03.2014 23:35, Lev wrote:<br>
</div>
<blockquote
cite="mid:bdmw6m8wbd1950op0lfe1f1i.1394047602842@email.android.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<div>Igor, you are right. You could check it in
proposal: <a class="moz-txt-link-freetext" href="http://openjdk.java.net/jeps/161">http://openjdk.java.net/jeps/161</a></div>
<div><br>
</div>
Best regards,
<div>Lev</div>
<br>
<br>
<div>-------- Original message --------</div>
<div>From: Igor Ignatyev <igor.ignatyev@oracle.com> </igor.ignatyev@oracle.com></div>
<div>Date:05/03/2014 13:12 (GMT+04:00) </div>
<div>To: Andrey Zakharov <andrey.x.zakharov@oracle.com>,Lev
Priima <lev.priima@oracle.com> </lev.priima@oracle.com></andrey.x.zakharov@oracle.com></div>
<div>Cc: hotspot-gc-dev@openjdk.java.net </div>
<div>Subject: Re: RFR: tests for JDK-8028720 Make the
Min/MaxHeapFreeRatio flags manageable </div>
<div><br>
</div>
Hi Andrey,<br>
<br>
0. In all your tests you have:<br>
> 27 * @key gc<br>
it's redundant, since the tests are already in gc directory.
please <br>
remove it.<br>
> 28 * @bug JDK-8028391<br>
according to convention, you have to use '8028391' instead of
'JDK-8028391'.<br>
$ grep -R "@bug" ./hotspot/test/ | wc -l<br>
434<br>
$ grep -R "@bug" ./hotspot/test/ | grep -c JDK<br>
0<br>
<br>
1. VMOptionValueChecker has checkInvalidValue and checkValid,
please <br>
normalize them:<br>
checkInvalidValue/checkValidValue<br>
or<br>
checkInvalid/checkValid<br>
<br>
2. I don't like that 'VMOptionValueChecker.getIntWritableValue'
method <br>
do two things, could you please replace it w/ two methods:<br>
VMOptionValueChecker.getIntValue(name)<br>
VMOptionValueChecker.checkIsWriteable(name)<br>
<br>
3. You use management packages, but not all JDK/JRE bundles have
it, so <br>
you have to add your tests in some group (needs_compact3, I
think). LevP <br>
should have more information on it.<br>
<br>
Lev, could you please help us here? com.sun.management.* and <br>
java.lang.management.* are available only in compact3, aren't
they?<br>
<br>
4. your tests look similar, can you create a super class for both
tests? <br>
we will be able to reuse it in other tests for writable flags<br>
<br>
> class TestDynIntVMFlags {<br>
> String name;<br>
> int originalValue;<br>
> TestDynIntVMFlags(name) {<br>
> name = name;<br>
> int originalValue = getIntValue(name)<br>
> println(name + " = " + originalValue);<br>
> }<br>
><br>
> void test() {<br>
> checkIsWriteable(name);<br>
> checkInvalidValue(name, "" + Integer.MIN_VALUE);<br>
> checkInvalidValue(name, "" + Integer.MAX_VALUE);<br>
> checkInvalidValue(name, "-10");<br>
> checkInvalidValue(name, "190");<br>
> checkValid(name, "0");<br>
> extraTest();<br>
> }<br>
> void extraTest() { }<br>
> }<br>
> //// TestDynMinHeapFreeRatio.java<br>
> TestDynMinHeapFreeRatio extends TestDynIntVMFlags {<br>
> private TestDynMinHeapFreeRatio() {
super("MinHeapFreeRatio"); }<br>
> String MaxFreeRatioFlagName = "MaxHeapFreeRatio";<br>
> void main() {<br>
> new TestDynMinHeapFreeRatio().test();<br>
> }<br>
> void extraTest() {<br>
> int maxHeapFreeValue = getIntValue(MaxFreeRatioFlagName);<br>
> checkInvalidValue(name, "" + (maxHeapFreeValue + 1));<br>
> checkValid(name, "" + (maxHeapFreeValue));<br>
> }<br>
> }//// TestDynMaxHeapFreeRatio.java<br>
> TestDynMaxHeapFreeRatio extends TestDynIntVMFlags {<br>
> TestDynMaxHeapFreeRatio() { super("MaxHeapFreeRatio"); }<br>
> String MaxFreeRatioFlagName = "MinHeapFreeRatio";<br>
> void main() {<br>
> new TestDynMaxHeapFreeRatio().test();<br>
> }<br>
> void extraTest() {<br>
> int minHeapFreeValue = getIntValue(MinFreeRatioFlagName);<br>
> checkInvalidValue(name, "" + (minHeapFreeValue - 1));<br>
> checkValid(name, "" + (minHeapFreeValue));<br>
> }<br>
> }<br>
<br>
Thanks,<br>
Igor<br>
<br>
On 02/20/2014 10:09 PM, Thomas Schatzl wrote:<br>
> Hi,<br>
><br>
> On Thu, 2014-02-20 at 15:15 +0400, Andrey Zakharov wrote:<br>
>> Any comments are welcome!<br>
>> Thanks.<br>
>><br>
>> On 14.02.2014 21:13, Jesper Wilhelmsson wrote:<br>
>>> Looks good.<br>
>>> /Jesper<br>
>>><br>
>>> Andrey Zakharov skrev 14/2/14 16:33:<br>
>>>> Simple tests to check that MaxHeapFreeRatio and
MinHeapFreeRatio<br>
>>>> flags are<br>
>>>> writable and managable in proper ways.<br>
>>>> webrev:
http://cr.openjdk.java.net/~jwilhelm/8028391/webrev.test.02/<br>
>>>> bug:
https://bugs.openjdk.java.net/browse/JDK-8028720<br>
>>>><br>
>>>> Thanks.<br>
>><br>
><br>
> some typos: "managable" -> "manageable"<br>
><br>
> Otherwise looks good.<br>
><br>
> Thomas<br>
><br>
><br>
</blockquote>
<br>
</body>
</html>