RFR 8037924: CMM Testing: Check Min/MaxHeapFreeRatio flags allows to shrink the heap when using ParallelGC
Andrey Zakharov
andrey.x.zakharov at oracle.com
Fri Apr 11 09:18:00 UTC 2014
Hi.
Here is updated webrevs
http://cr.openjdk.java.net/~iignatyev/anzakharov/8037924/webrev.14/
<http://cr.openjdk.java.net/%7Eiignatyev/anzakharov/8037924/webrev.14/>
http://cr.openjdk.java.net/~iignatyev/anzakharov/8037925/webrev.07/
<http://cr.openjdk.java.net/%7Eiignatyev/anzakharov/8037925/webrev.07/>
Thanks.
On 10.04.2014 16:13, Erik Helin wrote:
> Given that the issue for cleaning up these tests exist, is assigned
> and is already being worked on, I think this patch is ok.
>
> Erik
>
> On 2014-04-10 14:09, Erik Helin wrote:
>> Ok,
>>
>> here is a new patch made by Andrey:
>>
>> http://cr.openjdk.java.net/~ehelin/8037924/webrev.13/
>>
>> The new patch is now only for JDK-8037924, this patch does not fix
>> JDK-8037925 (a separate patch will be sent out for that).
>>
>> This patch makes no changes to the testlibrary, it only introduces the
>> new test. Please note that there already an issue filed for cleaning up
>> some of the duplication in these tests:
>> https://bugs.openjdk.java.net/browse/JDK-8039489
>>
>> Thanks,
>> Erik
>>
>> On 2014-04-03 16:01, Erik Helin wrote:
>>> Andrey, Igor,
>>>
>>> maybe we should have a chat session on IM where can discuss this?
>>>
>>> Thanks,
>>> Erik
>>>
>>> On 2014-04-03 15:57, Erik Helin wrote:
>>>> Andrey,
>>>>
>>>> is all the changes in this webrev part of the patch? The files in this
>>>> change looks very similar to the files in
>>>> http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.04/ ?
>>>>
>>>> Thanks,
>>>> Erik
>>>>
>>>> On 2014-04-02 19:46, Jesper Wilhelmsson wrote:
>>>>> New webrev is uploaded here:
>>>>>
>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.12/
>>>>>
>>>>> /Jesper
>>>>>
>>>>> Andrey Zakharov skrev 2/4/14 19:17:
>>>>>> Hi, here is not uploaded yet webrev with totally reverted
>>>>>> inheritance.
>>>>>> Jesper, can you please upload to your space.
>>>>>> Thanks.
>>>>>>
>>>>>>
>>>>>> On 02.04.2014 20:23, Igor Ignatyev wrote:
>>>>>>> Andrey,
>>>>>>>
>>>>>>> Why do test classes extend TestShrinkHeap?
>>>>>>>
>>>>>>> Igor
>>>>>>>
>>>>>>> On 04/02/2014 08:09 PM, Andrey Zakharov wrote:
>>>>>>>> Here is version without hidden testflow:
>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.11/
>>>>>>>> Thanks.
>>>>>>>>
>>>>>>>> On 01.04.2014 16:25, Jesper Wilhelmsson wrote:
>>>>>>>>> Uploaded:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.10/
>>>>>>>>>
>>>>>>>>> In general would agree regarding avoiding duplicated code, but
>>>>>>>>> when
>>>>>>>>> it comes to the control flow I agree with Erik. More hard to read
>>>>>>>>> code
>>>>>>>>> leads to harder maintenance than duplicating a few lines of code.
>>>>>>>>> /Jesper
>>>>>>>>>
>>>>>>>>> Andrey Zakharov skrev 1/4/14 11:05:
>>>>>>>>>> Hi, Erik. Thanks for comments.
>>>>>>>>>>
>>>>>>>>>> On 01.04.2014 10:49, Erik Helin wrote:
>>>>>>>>>>> Andrey,
>>>>>>>>>>>
>>>>>>>>>>> a couple of comments:
>>>>>>>>>>> - We do not use the @author tag in the tests
>>>>>>>>>>> (if you've seen other tests have the @author tag, then
>>>>>>>>>>> that is
>>>>>>>>>>> a bug)
>>>>>>>>>> removed
>>>>>>>>>>> - I'm not a big fan of using inheritance for sharing code
>>>>>>>>>>> between
>>>>>>>>>>> tests because it makes it very hard to open a test, e.g.
>>>>>>>>>>> TestHumongousShrinkHeap.java, and see what it does.
>>>>>>>>>> Agreed, I'm too. But doubled code leads to harder maintenance.
>>>>>>>>>>>
>>>>>>>>>>> The code you share in TestShrinkHeap is basically the
>>>>>>>>>>> check and
>>>>>>>>>>> the
>>>>>>>>>>> control flow of the tests. I would prefer if you could
>>>>>>>>>>> move the
>>>>>>>>>>> control flow (System.gc(), sample, eat, sample, free, check
>>>>>>>>>>> samples)
>>>>>>>>>>> into the tests and then write helper functions for retrieving
>>>>>>>>>>> the
>>>>>>>>>>> flag values.
>>>>>>>>>>>
>>>>>>>>>>> As for the check, please use the assertions in
>>>>>>>>>>> testlibrary/com/oracle/java/testlibrary/Asserts.java instead of
>>>>>>>>>>> throwing a RuntimeException.
>>>>>>>>>> Good point, changed.
>>>>>>>>>> Jesper, can we upload webrev.10 from attachment? Thanks.
>>>>>>>>>> I'm still need GC reviewers to approve push.
>>>>>>>>>>
>>>>>>>>>> Thanks.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Erik
>>>>>>>>>>>
>>>>>>>>>>> On 2014-03-31 13:46, Jesper Wilhelmsson wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> New webrev uploaded.
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.09/
>>>>>>>>>>>>
>>>>>>>>>>>> I'm not a Reviewer either so if you got Igor's blessing
>>>>>>>>>>>> already, my
>>>>>>>>>>>> review won't be enough to push unfortunately.
>>>>>>>>>>>> /Jesper
>>>>>>>>>>>>
>>>>>>>>>>>> Andrey Zakharov skrev 31/3/14 12:15:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>> Jepser, here is updated webrev.09
>>>>>>>>>>>>> Thomas, Jesper can you review it as well?
>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 31.03.2014 13:16, Igor Ignatyev wrote:
>>>>>>>>>>>>>> Andrey,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 1. TEST.groups:
>>>>>>>>>>>>>> please update copyright year:
>>>>>>>>>>>>>>> 2 # Copyright (c) 2013, Oracle and/or its affiliates. All
>>>>>>>>>>>>>>> rights
>>>>>>>>>>>>>>> reserved.
>>>>>>>>>>>>>> should be
>>>>>>>>>>>>>>> 2 # Copyright (c) 2013, 2014, Oracle and/or its
>>>>>>>>>>>>>>> affiliates.
>>>>>>>>>>>>>>> All
>>>>>>>>>>>>>>> rights
>>>>>>>>>>>>>>> reserved.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2. TestShrinkHeap.java
>>>>>>>>>>>>>>> 22 */
>>>>>>>>>>>>>>> 23 package com.oracle.java.testlibrary.gc;
>>>>>>>>>>>>>> add empty line between 22 and 23 as you do in all other
>>>>>>>>>>>>>> files,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> otherwise it looks good to me.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Keep in mind I'm not a Reviewer, but I can be mentioned in
>>>>>>>>>>>>>> 'Reviewed-by'
>>>>>>>>>>>>>> section )
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Igor
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 03/31/2014 10:33 AM, Andrey Zakharov wrote:
>>>>>>>>>>>>>>> Hi, team
>>>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.08/
>>>>>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8037924
>>>>>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8037925
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 30.03.2014 22:06, Jesper Wilhelmsson wrote:
>>>>>>>>>>>>>>>> Webrev uploaded:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.08/
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> /Jesper
>>>>>>>>>>>>>>> Thank you, Jesper.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Andrey Zakharov skrev 30/3/14 10:03:
>>>>>>>>>>>>>>>>> Hi! Here is webrev.08,
>>>>>>>>>>>>>>>>> Jesper, can you upload it? Thanks!
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Igor, thanks for well detailed review!
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 30.03.2014 02:33, Igor Ignatyev wrote:
>>>>>>>>>>>>>>>>>> Andrey,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> 1. both tests:
>>>>>>>>>>>>>>>>>>> 48 public void eat() {
>>>>>>>>>>>>>>>>>>> 61 public void free() {
>>>>>>>>>>>>>>>>>> change visibility of concrete methods to protected
>>>>>>>>>>>>>>>>>> 2. TestHumongousShrinkHeap
>>>>>>>>>>>>>>>>>>> 32 */
>>>>>>>>>>>>>>>>>>> 33 import
>>>>>>>>>>>>>>>>>>> com.oracle.java.testlibrary.gc.MemoryUsagePrinter;
>>>>>>>>>>>>>>>>>> add empty line between these lines or remove empty line
>>>>>>>>>>>>>>>>>> #32 in
>>>>>>>>>>>>>>>>>> TestDynShrinkHeap
>>>>>>>>>>>>>>>>>>> 62 //do not free last one list
>>>>>>>>>>>>>>>>>> space after '//'
>>>>>>>>>>>>>>>>>> 3. both tests:
>>>>>>>>>>>>>>>>>>> 26 * @bug 8037924
>>>>>>>>>>>>>>>>>>> 26 * @bug 8037925
>>>>>>>>>>>>>>>>>> after @bug you should specify product bug ids which
>>>>>>>>>>>>>>>>>> can be
>>>>>>>>>>>>>>>>>> tested by
>>>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>>>> test. I guess 8016479 instead of 8037924, and
>>>>>>>>>>>>>>>>>> 8036025/8037925.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> otherwise it looks good, thank you for your work and
>>>>>>>>>>>>>>>>>> patience )
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Igor
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On 03/30/2014 02:21 AM, Jesper Wilhelmsson wrote:
>>>>>>>>>>>>>>>>>>> Hi, New webrev in place:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.07/
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> /Jesper
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Andrey Zakharov skrev 29/3/14 20:57:
>>>>>>>>>>>>>>>>>>>> Hi, here updated webrev.
>>>>>>>>>>>>>>>>>>>> Jesper, can you, please, upload it to your webspace.
>>>>>>>>>>>>>>>>>>>> Igor, please, see comment below, everything
>>>>>>>>>>>>>>>>>>>> uncommented is
>>>>>>>>>>>>>>>>>>>> done.
>>>>>>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On 29.03.2014 20:08, Igor Ignatyev wrote:
>>>>>>>>>>>>>>>>>>>>> why isn't 'TestDynShrinkHeap.gc()'
>>>>>>>>>>>>>>>>>>>>> 'TestShrinkHeap.gc()'?
>>>>>>>>>>>>>>>>>>>> I cut this out.
>>>>>>>>>>>>>>>>>>>>> 5. if you need assured fullGC, we have WhiteBox
>>>>>>>>>>>>>>>>>>>>> API for
>>>>>>>>>>>>>>>>>>>>> it --
>>>>>>>>>>>>>>>>>>>>> s.h.WhiteBox.fullGC()
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Not sure is it applicable for me, because every
>>>>>>>>>>>>>>>>>>>> time in
>>>>>>>>>>>>>>>>>>>> feature
>>>>>>>>>>>>>>>>>>>> ticket
>>>>>>>>>>>>>>>>>>>> descriptions I see "System.gc()", so... let it be.
>>>>>>>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>>>>>>> Igor
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> On 03/29/2014 07:37 PM, Andrey Zakharov wrote:
>>>>>>>>>>>>>>>>>>>>>> Hi, team!
>>>>>>>>>>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.06/
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> bug:
>>>>>>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8037924
>>>>>>>>>>>>>>>>>>>>>> bug:
>>>>>>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8037925
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> On 29.03.2014 16:19, Andrey Zakharov wrote:
>>>>>>>>>>>>>>>>>>>>>>> Hi, Igor, here is just non-uploaded attachment to
>>>>>>>>>>>>>>>>>>>>>>> review. It
>>>>>>>>>>>>>>>>>>>>>>> will be
>>>>>>>>>>>>>>>>>>>>>>> upload later.
>>>>>>>>>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> On 29.03.2014 12:10, Igor Ignatyev wrote:
>>>>>>>>>>>>>>>>>>>>>>>> TestDynShrinkHeap:
>>>>>>>>>>>>>>>>>>>>>>>>> 45 public static byte[] temp;
>>>>>>>>>>>>>>>>>>>>>>>> unused field
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> it looks like 'g1/TestHumongousShrinkHeap.java'
>>>>>>>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>>>>>> 'parallelScavenge/TestDynShrinkHeap.java' should
>>>>>>>>>>>>>>>>>>>>>>>> have one
>>>>>>>>>>>>>>>>>>>>>>>> super
>>>>>>>>>>>>>>>>>>>>>>>> class,
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> abstract class TestDyn {
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> protected static final String
>>>>>>>>>>>>>>>>>>>>>>>> MIN_FREE_RATIO_FLAG_NAME =
>>>>>>>>>>>>>>>>>>>>>>>> "MinHeapFreeRatio";
>>>>>>>>>>>>>>>>>>>>>>>> protected static final String
>>>>>>>>>>>>>>>>>>>>>>>> MAX_FREE_RATIO_FLAG_NAME =
>>>>>>>>>>>>>>>>>>>>>>>> "MaxHeapFreeRatio";
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> protected abstract void eat();
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> protected abstract void free();
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> public final void test() {
>>>>>>>>>>>>>>>>>>>>>>>> gc();
>>>>>>>>>>>>>>>>>>>>>>>> MemoryUsagePrinter.printMemoryUsage("init");
>>>>>>>>>>>>>>>>>>>>>>>> eat();
>>>>>>>>>>>>>>>>>>>>>>>> MemoryUsagePrinter.printMemoryUsage("eaten");
>>>>>>>>>>>>>>>>>>>>>>>> MemoryUsage muFull =
>>>>>>>>>>>>>>>>>>>>>>>> ManagementFactory.getMemoryMXBean().getHeapMemoryUsage();
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> free();
>>>>>>>>>>>>>>>>>>>>>>>> MemoryUsage muFree =
>>>>>>>>>>>>>>>>>>>>>>>> ManagementFactory.getMemoryMXBean().getHeapMemoryUsage();
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> if (!(muFree.getCommitted() <
>>>>>>>>>>>>>>>>>>>>>>>> muFull.getCommitted())) {
>>>>>>>>>>>>>>>>>>>>>>>> throw new RuntimeException(
>>>>>>>>>>>>>>>>>>>>>>>> String.format("committed free heap size is not
>>>>>>>>>>>>>>>>>>>>>>>> less than
>>>>>>>>>>>>>>>>>>>>>>>> committed full heap size, heap hasn't been
>>>>>>>>>>>>>>>>>>>>>>>> shrunk?%n%s =
>>>>>>>>>>>>>>>>>>>>>>>> %s%n%s =
>>>>>>>>>>>>>>>>>>>>>>>> %s",
>>>>>>>>>>>>>>>>>>>>>>>> MIN_FREE_RATIO_FLAG_NAME,
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> ManagementFactoryHelper.getDiagnosticMXBean().getVMOption(MIN_FREE_RATIO_FLAG_NAME).getValue(),
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> MAX_FREE_RATIO_FLAG_NAME,
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> ManagementFactoryHelper.getDiagnosticMXBean().getVMOption(MAX_FREE_RATIO_FLAG_NAME).getValue()
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> )
>>>>>>>>>>>>>>>>>>>>>>>> );
>>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>> gc();
>>>>>>>>>>>>>>>>>>>>>>>> MemoryUsagePrinter.printMemoryUsage("done");
>>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> public static void gc() {
>>>>>>>>>>>>>>>>>>>>>>>> MemoryUsagePrinter.printMemoryUsage("before full
>>>>>>>>>>>>>>>>>>>>>>>> GC");
>>>>>>>>>>>>>>>>>>>>>>>> System.gc();
>>>>>>>>>>>>>>>>>>>>>>>> MemoryUsagePrinter.printMemoryUsage("after full
>>>>>>>>>>>>>>>>>>>>>>>> GC");
>>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Igor
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> On 03/29/2014 10:59 AM, Andrey Zakharov wrote:
>>>>>>>>>>>>>>>>>>>>>>>>> Hi, here is updated webrev:
>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.05/
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Any comments are welcome.
>>>>>>>>>>>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> On 28.03.2014 19:28, Igor Ignatyev wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>> On 03/28/2014 06:06 PM, Andrey Zakharov wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> On 28.03.2014 17:48, Igor Ignatyev wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Andrey,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> And as I said in 8037925:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> You don't need to use enum here and string
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> constant
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> (Labels.*) in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> TestDynShrinkHeap. I've mistakenly think
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> MemoryUsagePrinter
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> uses
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> String argument to change its own behavior.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Sorry for
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> useless
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> extra
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> work.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Please remove class 'Labels', it's unnecessary
>>>>>>>>>>>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>>>>>>>>>> just
>>>>>>>>>>>>>>>>>>>>>>>>>>>> decrease
>>>>>>>>>>>>>>>>>>>>>>>>>>>> readability.
>>>>>>>>>>>>>>>>>>>>>>>>>>> Ok
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 26 * @key gc
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> why do you need this?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hm, used as common practice.
>>>>>>>>>>>>>>>>>>>>>>>>>>>> if you don't understand meaning of '@key gc',
>>>>>>>>>>>>>>>>>>>>>>>>>>>> please don't
>>>>>>>>>>>>>>>>>>>>>>>>>>>> use
>>>>>>>>>>>>>>>>>>>>>>>>>>>> it.
>>>>>>>>>>>>>>>>>>>>>>>>>>> I think that this is used for test execution
>>>>>>>>>>>>>>>>>>>>>>>>>>> - to
>>>>>>>>>>>>>>>>>>>>>>>>>>> execute
>>>>>>>>>>>>>>>>>>>>>>>>>>> all test
>>>>>>>>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>>>>>>>> relates to "gc" for example.
>>>>>>>>>>>>>>>>>>>>>>>>>> All test related to "gc" are located in
>>>>>>>>>>>>>>>>>>>>>>>>>> test/gc, so
>>>>>>>>>>>>>>>>>>>>>>>>>> we don't
>>>>>>>>>>>>>>>>>>>>>>>>>> need it
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> otherwise it looks good.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Igor
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 03/28/2014 05:37 PM, Andrey Zakharov wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, here is updated webrev:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.04/
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 28.03.2014 02:29, Igor Ignatyev wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Andrey,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> you've placed the wrong link, the correct
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> one is
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.02/.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> And as I said in 8037925:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> You don't need to use enum here and string
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> constant
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> (Labels.*) in
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> TestDynShrinkHeap. I've mistakenly think
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> MemoryUsagePrinter
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> uses
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> String argument to change its own behavior.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Sorry for
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> useless
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> extra
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> work.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> MemoryUsagePrinter:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 44 float freeratio = 1f - (float)
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> memusage.getUsed() /
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> memusage.getCommitted();
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'd prefer '1.0f', but it's only my fad,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> it's
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> necessary to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> change.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> TestDynShrinkHeap:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 96 StringBuilder strb = new
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> StringBuilder("committed
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> heap size under pressure is not less than
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> committed
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> full heap
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> size,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> heap hasn't been shrunk?");
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 97
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> strb.append(System.getProperty("line.separator"));
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 98 strb.append(MinFreeRatioFlagName + " =
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> " +
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> minHeapFreeValue);
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 99
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> strb.append(System.getProperty("line.separator"));
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 100 strb.append(MaxFreeRatioFlagName + " =
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> " +
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> maxHeapFreeValue);
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 101 throw new
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> RuntimeException(strb.toString());
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> it can be replaced by:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> throw new
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> RuntimeException(String.format("committed
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> heap size
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> under
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> pressure is not less than committed full
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> heap
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> size, heap
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> hasn't
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> been
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> shrunk?%n%s=%d%n%s=%n",MinFreeRatioFlagName,minHeapFreeValue,MaxFreeRatioFlagName,maxHeapFreeValue));
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> but it's also not necessary.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Igor
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 03/27/2014 04:42 PM, Andrey Zakharov
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Here is updated webrev with string
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> constants:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.02/
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 26.03.2014 20:02, Andrey Zakharov wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Test to check that ParallelGC respect
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> dynamic
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> change of
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> MaxFreeRatio
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> and shrinks heap.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bug:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8037924
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20140411/97284361/attachment.htm>
More information about the hotspot-gc-dev
mailing list