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

[Thomas Schatzl]

Hi,

On Wed, 2015-08-05 at 10:29 -0400, Tom Benson wrote:
> Hi Thomas,
> Thanks for reviewing.
> 
> On 8/5/2015 9:37 AM, Thomas Schatzl wrote:
> > Hi,
> >
> > On Tue, 2015-08-04 at 18:06 -0400, Kim Barrett wrote:
> >> But, is next_region_in_heap really the right stepper function?  It
> >> skips over regions that are not "is_available".  Are the regions we're
> >> dealing with in this function all guaranteed to be available?
> > Yes. We assume that in the earlier call in allocate_containing_regions()
> > G1 made them available (committed them).
> >
> > I assume that when mmap'ing it, the
> 
> missing end of sentence?   If you were going to say....  the mapping 
> fails, so we need to free the archive regions that were just 
> allocated..., then I agree.  8^)

  committed memory will be automatically released.

> 
> >
> >> Clearly, bad things happen if last_region is not available while using
> >> next_region_in_heap here.
> > One could do regular pointer arithmetic *within* the MemRegion (which is
> > always a guaranteed contiguous range) and then map the address back to
> > the HeapRegion*.
> 
> I don't think this was a suggestion...   was it?
> 

An option, maybe Kim likes that more.

> > Since last_region is the region containing the last address within the
> > memory range, wouldn't that mean given above preconditions, this could
> > not happen?
> >
> > - I think the method should add a assert_at_safepoint(true) at the top
> > (and possibly all other archival methods if they are not yet through the
> > call chain), or decrease_used() made safe against concurrent
> > modification using the ParGC_Rare_Event_lock.
> >
> > I would prefer just making sure the code is only run at a safepoint.
> 
> These restore-time routines (alloc_/free_ archive regions) are called at 
> the beginning of JVM init, not at a safepoint.  The dump-time archive 
> alloc routines (such as archive_mem_allocate) do check for safepoint 
> context.

Sigh. Is there a way to check this reliably?

As mentioned, at least the decrement_used() call is not guarded against
concurrent modification. It is probably easier for now to guard against
using these methods at the wrong time (in case somebody wants to try
them if some other requirement comes up) instead of trying to make them
MT safe.

Thanks,
  Thomas