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

[Kim Barrett]

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

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