RFR (S): 8131734: Add free_archive_regions support to G1 for -Xshared:auto

[Thomas Schatzl]

Hi,

On Tue, 2015-08-11 at 20:02 -0400, Kim Barrett wrote:
> On Aug 11, 2015, at 5:40 PM, Tom Benson <tom.benson at oracle.com> wrote:
> > 
> > On 8/11/2015 5:24 PM, Kim Barrett wrote:
> >> On Aug 11, 2015, at 1:43 PM, Tom Benson <tom.benson at oracle.com> wrote:
> >>> Updated full and incremental webrevs of the GC code are at:
> >>> http://cr.openjdk.java.net/~tbenson/8131734/webrev.02/
> >>> http://cr.openjdk.java.net/~tbenson/8131734/webrev.02.vs.01/
> >> Can this introduce uncommitted "holes" in committed space?  It seems
> >> like it might.  Otherwise, why introduce shrink_at, rather than just
> >> using shrink_by.  I'm not sure such holes are presently possible, and
> >> I'm not sure they are handled properly everywhere.
> > shrink_by looks for free ranges it can uncommit.  Here, we want to
> > uncommit the specific region(s) allocated with alloc_archive_regions.
> > There is an expand_at that takes a specific index, which shrink_at is
> > intended to be analogous to.
> 
> I seem to have earlier convinced myself that shrink_by would be
> confused by a hole; after staring at it some more, I now think it's
> ok.  Sorry for the noise.
> 
> But I think there might be an unrelated pre-existing performance bug
> in shrink_by.  Line 431 is "cur -= num_last_found;".  If the recent
> scan had to skip over a block of !available || !empty regions to find
> the regions that were just removed, that skipped block isn't accounted
> for by that decrement.  I think a better iteration step would be "cur
> = idx_last_found;".  [This is part of what confused me about holes.]

That's true. I filed https://bugs.openjdk.java.net/browse/JDK-8133456.

> > G1 is designed to allow uncommitted holes.   Even without this
> >change, there is a big 'hole' in committed space:    All the regions
> >between the low end of the heap where mutator allocation is occurring
> >(initially sized by Xms)  and these archive regions which are at the
>>highest end of the (Xmx-sized) heap.
> > 
> > In a way,  as used by CDS, calling dealloc_archive_regions is
>>*removing* that hole. 8^)     The entire upper portion of the heap
>>(above Xms) will again be uncommitted, as if alloc_archive_regions had
>>never been called.
> 
> I do see code that seems to be intended to cope with such holes.
> 
> I wonder though, were there ever uncommitted holes in practice before
> the introduction of this new archive region feature?  Previously, the
> only time a region gets uncommitted is after a full gc.  I've not
> studied G1's full gc enough to even guess whether it could leave such
> holes. 

Yes, when there are humongous objects in the heap.

> If the answer is no, there could be lurking bugs waiting to be
> uncovered.  I guess we'll deal with those if/when we find them.

There are explicit tests in the jtreg tests (gc/g1/TestShrink*) that
test shrinking, and TestShrinkDefragmentedHeap in particular tests this.

Also there are a few stress tests (ArrayJuggle*) that repeatedly create
holes using large objects and do full collections that cause heap
shrinking.

Thanks,
  Thomas