RFR 8037924: CMM Testing: Check Min/MaxHeapFreeRatio flags allows to shrink the heap when using ParallelGC
Jon Masamitsu
jon.masamitsu at oracle.com
Wed Apr 2 16:28:07 UTC 2014
On 4/2/14 2:07 AM, Igor Ignatyev wrote:
> Jon,
>
>> Given the current implementation of these tests, what will I need
>> to do to understand what TestDynShrinkHeap is doing?
>
> You'll have to open one more file -- TestShrinkHeap, and it's easy to
> understand since you see that TestDynShrinkHeap is a subclass of
> TestShrinkHeap. I offered to use inheritance, since my understanding
> is that we are going to write other tests w/ the same control flow,
> and for me in future it'll cost too much to maintain N tests w/
> copy-pasted control flow.
My problem in the past was that I could not find other
source files. Will that be fixed? I don't need to look
at the sources often so tend to forget where they are.
Actually, I don't think I've ever found the sources if
they were not in the nightly testing "dir" test directory.
>
> But I understand your point and don't insist on using inheritance, as
> I said before:
>> Since these tests are gc tests, it's up to you to decide how to write
>> better.
> I just want to avoid having tens tests w/ the almost same code, since
> changing something in them will be very hard.
Avoiding the duplication would be nice.
I usually need to look at what the test is doing because
I'm having trouble reproducing the failure and want
to understand if tweaking the jvm flags would help
reproduce it. This pushes work back to SQE but
if I only had to look at failures that had a reliable
reproducer, I would not need to look at the test sources.
That might just be the way I work though.
Maybe some way for me to search for source files.
Or some tool to assemble the source files into a
directory.
Jon
>
> Thanks,
> Igor
>
> On 04/02/2014 12:44 AM, Jon Masamitsu wrote:
>> Igor,
>>
>> If I see a test failure of TestDynShrinkHeap, I need to understand
>> what that test is doing. My recent experience (nightly testing)
>> tells me that I will easily be able to look at
>> TestDynShrinkHeap.java but maybe not much
>> more.
>>
>> Given the current implementation of these tests, what will I need
>> to do to understand what TestDynShrinkHeap is doing?
>>
>> Jon
>>
>>
>> On 4/1/14 6:24 AM, Igor Ignatyev wrote:
>>> Erik/Jesper,
>>>
>>> g1/TestHumongousShrinkHeap, parallelScavenge/TestDynShrinkHeap are
>>> test cases of TestShrinkHeap, so they suppose to share control flow,
>>> or more precisely they shouldn't contain control-flow.
>>>
>>> let's say they just specify function in TestShrinkHeap:
>>>> TestShrinkHeap {
>>>> Runnable eat;
>>>> Runnable free;
>>>> TestShrinkHeap(Runnable eat, Runnable free) { ... }
>>>> void test() {
>>>> System.gc();
>>>> MemoryUsagePrinter.printMemoryUsage("init");
>>>>
>>>> eat.run();
>>>> MemoryUsagePrinter.printMemoryUsage("eaten");
>>>> MemoryUsage muFull =
>>>> ManagementFactory.getMemoryMXBean().getHeapMemoryUsage();
>>>>
>>>> free.run();
>>>> MemoryUsagePrinter.printMemoryUsage("free");
>>>> MemoryUsage muFree =
>>>> ManagementFactory.getMemoryMXBean().getHeapMemoryUsage();
>>>>
>>>> assertLessThan(muFree.getCommitted(), muFull.getCommitted(),
>>>> ...));
>>>>
>>>> }
>>>> }
>>>
>>> then g1/TestHumongousShrinkHeap is
>>> new TestShrinkHeap(
>>> ()->{
>>>> int HumongousObjectSize = Math.round(.9f * PAGE_SIZE);
>>>> 51 System.out.println("Will allocate objects of size=" +
>>>> 52 MemoryUsagePrinter.humanReadableByteCount(HumongousObjectSize,
>>>> true));
>>>> 53
>>>> 54 for (int i = 0; i < PAGES_NUM; i++) {
>>>> 55 ArrayList<byte[]> stuff = new ArrayList<>();
>>>> 56 eatList(stuff, 100, HumongousObjectSize);
>>>> 57 MemoryUsagePrinter.printMemoryUsage("eat #" + i);
>>>> 58 garbage.add(stuff);
>>>> 59 }
>>> },
>>> ()->{
>>>> 63 // do not free last one list
>>>> 64 garbage.subList(0, garbage.size() - 1).clear();
>>>> 65
>>>> 66 // do not free last one element from last list
>>>> 67 ArrayList stuff = garbage.get(garbage.size() - 1);
>>>> 68 stuff.subList(0, stuff.size() - 1).clear();
>>>> 69 System.gc();
>>> }
>>>
>>> from my point of view, it's quite simple to understand and maintain
>>> (of course if we replace lambdas by method references), and it's much
>>> easier to write add new test-cases and extend test's logic.
>>>
>>> however, it's just a question of code-style and preferences. Since
>>> these tests are gc tests, it's up to you to decide how to write better.
>>>
>>> Igor
>>>
>>> On 04/01/2014 04:45 PM, Erik Helin wrote:
>>>> On 2014-04-01 11:05, Andrey Zakharov wrote:
>>>>> 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.
>>>>
>>>> Duplicated code often leads to harder maintenance, but it is not the
>>>> only thing making code hard to maintain. By using inheritance, you
>>>> have
>>>> moved the control flow from the test to another file. When I want to
>>>> look at test test because it failed, I want to immediately see what it
>>>> did. I do *not* want to traverse an inheritance hierarchy and build a
>>>> mental model of how the control flows up and down in the inheritance
>>>> hierarchy. It is very important that tests are explicit and clearly
>>>> describe what they are testing.
>>>>
>>>> Looking at the code in TestShrinkHeap::test:
>>>>
>>>> public final void test() {
>>>> System.gc();
>>>> MemoryUsagePrinter.printMemoryUsage("init");
>>>>
>>>> eat();
>>>> MemoryUsagePrinter.printMemoryUsage("eaten");
>>>> MemoryUsage muFull =
>>>> ManagementFactory.getMemoryMXBean().getHeapMemoryUsage();
>>>>
>>>> free();
>>>> MemoryUsagePrinter.printMemoryUsage("free");
>>>> MemoryUsage muFree =
>>>> ManagementFactory.getMemoryMXBean().getHeapMemoryUsage();
>>>>
>>>> assertLessThan(muFree.getCommitted(), muFull.getCommitted(),
>>>> ...));
>>>> }
>>>>
>>>> You can easily move the control flow back into tests and share code
>>>> with
>>>> utility classes, for example:
>>>>
>>>> public class TestHumongousShrinkHeap {
>>>> ....
>>>>
>>>> private void eat() {
>>>> // old eat code
>>>> ShrinkHeapUtils.log("eat");
>>>> }
>>>>
>>>> private void free() {
>>>> // old free code
>>>> ShrinkHeapUtils.log("free");
>>>> }
>>>>
>>>> public final void test() {
>>>> System.gc();
>>>> ShrinkHeapUtils.log("init");
>>>>
>>>> eat();
>>>> MemoryUsage full = ShrinkHeapUtils.getMemoryUsage();
>>>>
>>>> free();
>>>> MemoryUsage free = ShrinkHeapUtils.getMemoryUsage();
>>>>
>>>> ShrinkHeapUtils.assertLessThan(free, full);
>>>> }
>>>> }
>>>>
>>>> Now you can move the code from TestShrinkHeap to ShrinkHeapUtils. You
>>>> are now duplicating the call to System.gc(), the calls to eat and
>>>> free,
>>>> the log calls, the getMemoryUsage calls and the assert. That is nine
>>>> lines of code.
>>>>
>>>> Duplicating nine lines of code and being able to see the flow in the
>>>> test itself is better than sharing the nine lines of code and not
>>>> being
>>>> able to see the what test is doing.
>>>>
>>>> Thanks,
>>>> Erik
>>>>
>>>>>>
>>>>>> 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.
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>
>>
More information about the hotspot-gc-dev
mailing list