RFR 8037924: CMM Testing: Check Min/MaxHeapFreeRatio flags allows to shrink the heap when using ParallelGC
Andrey Zakharov
andrey.x.zakharov at oracle.com
Mon Mar 31 06:33:03 UTC 2014
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