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

[John Coomes]

Bengt Rutisson (bengt.rutisson at oracle.com) wrote:
> 
> 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:
> >> ...
> > 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?

All the GCs used GrowableArray until Stack<> was added for 6423256,
and a number of GrowableArrays were converted as part of that fix.  I
checked some saved email and notes but didn't find any clues as to why
these weren't changed at the same time.

-John

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