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

[Kirill Zhaldybin]

Thomas,

Thank you for reviewing the fix.

Could you please read comments inline?

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
>
>    I would kind of prefer if the test took a more systematic approach,
> i.e. increase the "opportunity" that allocations at the end of
> humongous objects occur, and increase the kind of allocations that may
> occur.
>
> So the test first allocates one of three different kinds of humongous
> objects (half a region, "just above a region" and "just above two
> regions").
>
> Then it allocates random object arrays, trying to make G1 allocate at
> the end of an existing humongous object (apparently at least).
>
> So, while the setup seems fairly okay, I am not sure whether
> randomizing the initial humongous object sizes makes a lot of
> difference (it does not to the allocator). Maybe, just maybe having all
> of these in the test at the same time might make the test more
> reproducable. It's not a big deal though.
The test uses reproducible random from testlibrary, so initial setup 
could be reproduced if needed.


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


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

I also addressed some Dmitry's remarks.

>
> Thanks,
>    Thomas
>