RFR 8037924: CMM Testing: Check Min/MaxHeapFreeRatio flags allows to shrink the heap when using ParallelGC
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Sun Mar 30 18:06:08 UTC 2014
Webrev uploaded:
http://cr.openjdk.java.net/~jwilhelm/8037924/webrev.08/
/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