RFR 8037925: CMM Testing: an allocated humongous object at the end of the heap should not prevents shrinking the heap

[Andrey Zakharov]

Hi, here is updated webrev: 
http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.04/
Thanks.


On 28.03.2014 18:03, Igor Ignatyev wrote:
>>>> how should I know about it? you didn't mention about it in your RFR
>>> Nop, I did. Please, check my first message.
> Oh, I'm sorry, I need more sleep or coffee.
> in future, you can use one patch for multiple fixes, it'll solve such 
> situation.
>
>>   47     private static final ArrayList< ArrayList< byte[] > > 
>> garbage = new ArrayList< ArrayList< byte[] > >();
>>   59             ArrayList< byte[] > stuff = new ArrayList< byte[] >();
> redundant spaces, + use diamonds
>
>>>>>>   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.
>
> 44-45,48-49:
> According to section 9 in java code conventions[1]:
>> The names of variables declared class constants and of ANSI constants 
>> should be all uppercase with words separated by underscores ("_"). 
>> (ANSI constants should be avoided, for ease of debugging.)
>
> [1] 
> http://www.oracle.com/technetwork/java/javase/documentation/codeconvtoc-136057.html
>
> Igor
>
> On 03/28/2014 05:35 PM, Andrey Zakharov wrote:
>> Hi,
>> here is updated webrev:
>> http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.03/
>>
>> Igor, please, see comments below.
>> Thanks.
>>
>> On 28.03.2014 02:24, Igor Ignatyev wrote:
>>> Andrey,
>>>
>>>>> Most probably I forgot TEST.groups again...
>>> yeap, you did.
>> Ok, note that due I have two unpushed changes in TEST.groups I have
>> change from previous webrev.
>>>
>>>>>> - I can't find MemoryUsagePrinter class in testlibrary[1]
>>>>> It is in webrev: http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/
>>> how should I know about it? you didn't mention about it in your RFR
>> Nop, I did. Please, check my first message.
>>>
>>>>>> - please replace string constant, which use in 
>>>>>> MemoryUsagePrinter, by
>>>>>> enum
>>> 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.
>> Yes, its just string labels and nothing more.
>>
>>>
>>>>>> - lines 79,82: why did you use
>>>>>> sun.management.ManagementFactoryHelper.getDiagnosticMXBean() instead
>>>>>> of
>>>>>> 'ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class)' 
>>>>>> ?
>>>>> Are we talking about this?
>>>>>              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",
>>>>>                              MinFreeRatioFlagName,
>>>>> ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class).
>>>>> getVMOption(MinFreeRatioFlagName).getValue(),
>>>>>                              MaxFreeRatioFlagName,
>>>>> ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class).
>>>>> getVMOption(MaxFreeRatioFlagName).getValue()
>>>>>                      )
>>> yes, we're.
>> Ok, changed.
>>>
>>> please, use generics in 54,65 lines:
>>>> 54     private static final List garbage = new ArrayList();
>>>> 65             List stuff = new ArrayList();
>>> and then, you won't have to use cast in line 95:
>>>>  95         ArrayList stuff = ((ArrayList) garbage.get(garbage.size()
>>>> - 1));
>>>
>> Done
>>>>   26  * @key gc
>>> why do you need this?
>> Hm, used as common practice.
>>>
>>>>   64         for (int i = 0; i < 5; i++) {
>>> magic number '5'
>> Good point, changed
>>>
>>>>  61         int humonSize = Math.round(1f * PageSize);
>>> I'd prefer '1.0f', but it's only my fad, it's not necessary to change.
>>>
>>> Thanks
>>> Igor
>>>
>>> On 03/27/2014 04:37 PM, Andrey Zakharov wrote:
>>>> Hi, here is updated webrev:
>>>> http://cr.openjdk.java.net/~jwilhelm/8037925/webrev.02/
>>>> Most probably I forgot TEST.groups again...
>>>>
>>>>
>>>> On 26.03.2014 23:12, Igor Ignatyev wrote:
>>>>> Hi Andrey,
>>>>>
>>>>> - I can't find MemoryUsagePrinter class in testlibrary[1]
>>>> It is in webrev: http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/
>>>>> - please replace string constant, which use in MemoryUsagePrinter, by
>>>>> enum
>>>>> - you forgot to remove commented line 53 Hi
>>>> Done
>>>>> - lines 79,82: why did you use
>>>>> sun.management.ManagementFactoryHelper.getDiagnosticMXBean() instead
>>>>> of
>>>>> 'ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class)' 
>>>>> ?
>>>> Are we talking about this?
>>>>              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",
>>>>                              MinFreeRatioFlagName,
>>>> ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class).
>>>> getVMOption(MinFreeRatioFlagName).getValue(),
>>>>                              MaxFreeRatioFlagName,
>>>> ManagementFactory.getPlatformMXBean(HotSpotDiagnosticMXBean.class).
>>>> getVMOption(MaxFreeRatioFlagName).getValue()
>>>>                      )
>>>>              );
>>>>>
>>>>> - just a nit: you again use extra spaces in some brackets:
>>>>> 55,68,61,74,..
>>>> Done
>>>> Thanks.
>>>>>
>>>>> [1]
>>>>> http://hg.openjdk.java.net/jdk9/hs-gc/hotspot/file/tip/test/testlibrary/com/oracle/java/testlibrary 
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Igor
>>>>>
>>>>> On 03/26/2014 11:02 PM, Andrey Zakharov wrote:
>>>>>> Test to check, that if we allocate h1, h2 and h3 humongous 
>>>>>> objects and
>>>>>> free h1 and h2, after GC G1 shrinks the heap.
>>>>>> webrev: http://cr.openjdk.java.net/~jwilhelm/8037925/webrev/
>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8037925
>>>>>>
>>>>>> This test used testlibrary MemoryUsagePrinter from
>>>>>> http://cr.openjdk.java.net/~jwilhelm/8037924/webrev/
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>
>>