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

[Kirill Zhaldybin]

Thomas,

Thank you for prompt reply!

Here are a new WebRev: 
http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8132715/webrev.04/

Changes:
1. Changed  * @requires vm.gc=="G1" | vm.gc=="null" to
@requires vm.gc.G1

2. Deleted FullGC from list of actions. We actually don't need since 
liveset size is calculated to prevent OOM. Without external Full GCs we 
get a lot of different GCs: young, mixed and full (caused by allocation 
failure).

3. I changed 80% heap scenario to 70% since I would like to avoid any 
OOMs and with 80% heap filled with Humongous objects we were pretty 
close to oom. I also decreased maximum used memory for allocators for 
the same reason.

4. Fixed typo "heep" to "heap".

Could you please let me know your opinion?

Regards, Kirill

On 27.06.2016 16:00, Thomas Schatzl wrote:
> Hi,
>
> On Fri, 2016-06-24 at 19:24 +0300, Kirill Zhaldybin wrote:
>> Thomas,
>>
>> Thank you for reviewing the fix.
>>
>> Could you please read comments inline?
>
> some more comments:
>
> -  38  * @requires vm.gc=="G1" | vm.gc=="null" could be replaced by the
> new @requires vm.gc.G1.
>
>> Regards, Kirill
>>
>> On 22.06.2016 15:16, Thomas Schatzl wrote:
>>>
>>> Hi Kirill,
>>>
>>> On Tue, 2016-06-21 at 18:14 -0400, Derek White wrote:
>>>>
>>>> On 6/21/16 1:45 PM, 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
> [...]
>>>
>>> My main concern is with the second part, the allocation of the
>>> "poison" objects by the Allocator thread.
>>> Since they are smaller than a region size, they will always be
>>> allocated in young gen first. Which means, that it is very unlikely
>>> (since they are fairly short living) that they may be allocated in
>>> old gen.
>>>
>>> So the amount of opportunities when such a bad allocation happens
>>> seems too small.
>> I rewrote test so now it keeps some objects alive and calls Full GC
>> often to promote them into old gen. I also set tenure threshold to 0
>> to speed up promotion.
>> I checked (I had some debug output now deleted) that we actually had
>> non-humongous objects in old gen during the testing.
>
> :)
>
> There seems to be just one problem: the full gc will make everything
> old gen, and compact the space, so most allocations in old gen may be
> due to the full gcs, and not due to evacuations.
>
> I think the test having full gcs may be useful (checking that
> compaction does not compact into the humongous regions), but it seems
> it is invoked too often, as it occurs after every allocation.
>
> Did you measure the number of "rounds" the allocators manage with this
> new version compared to the old one? It should be significantly
> smaller.
>
> Maybe decreasing the frequency of full gcs might help (or have another
> test run with full gcs enabled).
>
> Thanks,
>    Thomas
>