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

[Kirill Zhaldybin]

Dmitry,

Thank you for review!

Regards, Kirill

On 24.06.2016 19:38, Dmitry Fazunenko wrote:
> 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
>>>
>>
>