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

Kirill Zhaldybin kirill.zhaldybin at oracle.com
Fri Jun 24 16:35:59 UTC 2016


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
>




More information about the hotspot-gc-dev mailing list