RFR (S): 8003237: G1: Reduce unnecessary (and failing) allocation attempts when handling an evacuation failure

[Thomas Schatzl]

Hi Jon,

  thanks for the review...

On Wed, 2015-08-05 at 09:36 -0700, Jon Masamitsu wrote:
> Thomas,
> 
> I think that this method gets called if a new region is allocated
> via allocate_inline_or_new_plab()
> 
> "src/share/vm/gc/g1/g1CollectedHeap.cpp"
> 
> 6436 HeapRegion* G1CollectedHeap::new_gc_alloc_region(size_t word_size,
> 6437                                                  uint count,
> 6438                                                  InCSetState dest) {
> 6439   assert(FreeList_lock->owned_by_self(), "pre-condition");
> 6440
> 6441   if (count < g1_policy()->max_regions(dest)) {
> 
> Might the g1_policy()->max_regions() conflict with the test
> in has_more_free_regions() which considers all free regions
> (even uncommitted).   That is, has_more_free_regions() will
> allow allocation attempts that will not success because of
> max_regions().

No, because the worst case is that we go into the take-the-Mutex case
too often (which is bad).

However, thinking about this again, I found another option (which is
much clearer to see after recent refactoring of g1allocator classes): if
G1AllocRegion::attempt_allocation_locked() fails, we know that there is
really no more space in the heap left.

In this case, just set an internal flag, instead of relying on the
has_more_free_regions() from G1CollectedHeap.

> 
> http://cr.openjdk.java.net/~tschatzl/8003237/webrev/src/share/vm/gc/g1/g1ParScanThreadState.hpp.frames.html
> 
> >    74   // Indicates whether in the last generation (old) there is no more space
> >    75   // available for allocation.
> >    76   bool _no_more_space_in_old;
> 
> Would a name like _old_is_full suit you here in place of 
> _no_more_space_in_old?  I like positives instead
> negatives.

Done.

New webrevs:
http://cr.openjdk.java.net/~tschatzl/8003237/webrev.1 (full)
http://cr.openjdk.java.net/~tschatzl/8003237/webrev.0_to_1 (partial)

I think this one looks better, performs the same (tested) and does not
introduce the additional dependency on G1CollectedHeap.

This also automatically takes into consideration restrictions like in
G1CollectedHeap::new_gc_alloc_region() without the need to repeat them.

The addition of the new methods as virtual has been intentional as other
Allocators may have different views on when the heap for the given
context is full.

Thanks,
  Thomas