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

[Igor Ignatyev]

>>> 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.
>>>>>
>>>
>