Re: RFR(S): 8132715: Add tests which check that no allocations allowed in any of humongous regions​

[Dmitry Fazunenko]

Hi Kirill,

new version looks good to me.
Thank you for addressing my concerns.

-- Dima

On 24.06.2016 19:35, Kirill Zhaldybin wrote:
> Dmitry,
>
> Thank you for reviewing the fix.
>
> Here are a new WebRev: 
> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132715/webrev.03/
>
> I rewrote the test to address Thomas comments.
>
> I also addressed your remarks - see below.
>
> On 22.06.2016 15:33, Dmitry Fazunenko wrote:
>> Hi Kirill,
>>
>> Overall the test looks good.
>> Some minor comments/suggestions
>>
>> 1)
>> @requires vm.gc == null | vm.gc == G1
>> -->
>> @requires vm.gc.G1
> fixed
>>
>> 2)
>> Calendar.getInstance().getTimeInMillis()
>> -->
>> System.currentTimeMillis
> fixed
>>
>> 3) Would you document parameters args[0] and args[1]
>> You have
>>
>>          long duration = Integer.parseInt(args[0]) * 1000L;
>>
>> Right after args.length check, it's good.
>> If you introduce the second parameters in the same way - it would be
>> enough.
>> Adding a short comment would also increase readability.
> fixed
>>
>> 4) Will it make sense to vary G1 region size as well?
> Well, I think that smallest region size could increase chance of wrong 
> allocation.
>>
>> 5) Would you consider renaming
>>
>> ALLOCATORS_COUNT
>> -->
>> ALLOC_THREAD_CONUT
>>
>> To include "thread" in the variable name.
> fixed
>>
>> Thanks,
>> Dima
>
> Thank you.
>
> Regards, Kirill
>>
>> On 21.06.2016 20:45, Kirill Zhaldybin wrote:
>>> Dear all,
>>>
>>> Could please review this test for 8132715?
>>>
>>> The test fills heap with humongous objects with a lot of wasted space,
>>> then starts to allocate non-humongous objects and checks that no
>>> allocations happened in humongous regions.
>>> There are 3 runs of the test: with filled 10%, 50% and 80% of heap.
>>>
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8132715
>>> WebRev:
>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132715/webrev.01/
>>>
>>> Thank you.
>>>
>>> Regards, Kirill
>>
>