Request for review (M): 8002144: G1: large number of evacuation failures may lead to large c heap memory usage

[Bengt Rutisson]

Hi John,

Thanks for looking at this!

On 2/4/13 10:18 PM, John Coomes wrote:
> Bengt Rutisson (bengt.rutisson at oracle.com) wrote:
>> Hi all,
>>
>> Could I have a couple of reviews for this change?
>> http://cr.openjdk.java.net/~brutisso/8002144/webrev.00/
>>
>> For the evacuation failure handling in G1 we need to store the mark word for
>> objects that we self-forward. We store these mark words in two GrowableArrays.
>> The problem is that if we have a lot of objects that need to be self-forwarded
>> we will add a lot of entries to the GrowableArrays. These arrays will double in
>> size when they get full. Worst case these arrays can require more consecutive
>> free memory than is available in the OS malloc heap.
>>
>> I have a test (ManyObjects) that runs out of native memory on Windows 32 bit
>> platforms because of this issue. We want to double from 10MB to 20MB when we
>> hit native out of memory.
>>
>> My fix for this will reduce the risk for native out of memory. Instead of
>> doubling the size I introduce a chunked data structure that will only malloc
>> one new chunk when it gets full. This requires less consecutive memory. It also
>> has the nice side effect that we don't have to copy the entries when we need
>> more memory.
> It doesn't appear that G1 needs to process these in FIFO order.  If
> not and LIFO order is ok, you can use the existing Stack<> template
> which is a chunked stack with chunks a page in size (by default).  The
> other collectors use this, e.g.,
>
>    Stack<markOop, mtGC>       PSScavenge::_preserved_mark_stack;
>    Stack<oop, mtGC>           PSScavenge::_preserved_oop_stack;
>
> It would be nice to avoid a new data structure if a Stack will do.
>
> If FIFO order really is needed, consider adding a more generic data
> structure (e.g., ChunkedQueue<T>) and making G1PreserveMarkQueue a
> typedef or simple wrapper.

I agree that FIFO order is probably not needed. The reason I preserved 
the FIFO order was mostly to reduce the risk of introducing regressions. 
So, I think using the Stack data structure should be fine. I'll change 
my patch to do that instead and try it out. Thanks for pointing me to 
the Stack!

I've been at JFokus all day today and I'll be there tomorrow as well. 
I'll send out an updated webrev as soon as I get a chance to try it out.

Any idea why G1 used the GrowableArray in the first place when the other 
GCs use Stack?

Thanks,
Bengt
>
>> Without this fix I get native out of memory about every three runs of the test.
>> With this fix the test has been running for several days and more than 5600
>> iterations.
>>
>> The chunk size is variable but has a max limit. I use 40 entries as initial
>> size since this is what the GrowableArrays used. I picked 10000 as the maximum
>> size. The value 10000 can probably be tuned further, but 100000 was too much (I
>> still got native OOME) and 10000 works fine.
>>
>> I have been comparing GC pause times with and without my fix for the
>> ManyObjects test. I don't see any large differences in the pause times. This
>> will only affect performance for runs that have a lot of evacuation failures.
>> These runs will benefit form the fact that we don't have to do as much copying
>> as before, but they will also do several more mallocs compared to before my
>> fix. The runs I've done indicate that this evens out. I can't see any large
>> differences.
> I hope we don't start caring about performance when there are many
> evacuation failures :-).
>
> -John