RFR 8037924: CMM Testing: Check Min/MaxHeapFreeRatio flags allows to shrink the heap when using ParallelGC
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Wed Apr 2 17:04:19 UTC 2014
Jon,
In this case I think finding the sources is as easy as we can wish for, since
these are jtreg-tests and the sources are located in the hotspot repository
(look in hotspot/test/).
However, I don't think the ease of finding the sources motivates splitting the
control flow of the test over two source files. Your argument of being able to
quickly see what the test is doing is more important imo.
/Jesper
Jon Masamitsu skrev 2/4/14 18:28:
>
> 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