RFR (S): 8131734: Add free_archive_regions support to G1 for -Xshared:auto
tom.benson at oracle.com
Thu Aug 20 14:12:24 UTC 2015
On 8/20/2015 10:06 AM, Thomas Schatzl wrote:
> Hi Tom,
> sorry for the delay...
> On Thu, 2015-08-13 at 15:32 -0400, Tom Benson wrote:
>> Hi Thomas,
>> On 8/12/2015 7:00 AM, Thomas Schatzl wrote:
>>> On Tue, 2015-08-11 at 13:43 -0400, Tom Benson wrote:
>>>> On 8/7/2015 10:56 AM, Tom Benson wrote:
>>>> After some discussion, I've changed the definition and name of
>>>> free_archive_regions. Now called dealloc_archive_regions, it uncommits
>>>> the specified regions, unmapping the memory, rather than adding them to
>>>> the free list. This means the CDS code will no longer do the unmapping
>>>> on verification failures.
>>>> Updated full and incremental webrevs of the GC code are at:
>>>> Tested with JPRT and running benchmarks with the dealloc_ performed
>>>> explicitly. Jiangli also tested the original failing cases, and will be
>>>> posting an updated webrev.
>>> - is it possible that shrink_by() uses shrink_at()? This would avoid two
>>> paths that uncommit regions like expand_by()/expand_at()?
>> OK, I made the change. I didn't do it originally because the asserts I
>> wanted to add for the call from g1CollectedHeap seemed superfluous for
>> the other call, and shrink_at was so small. Now shrink_at takes a
>> region count as well.
>> Updated full and incremental webrevs are at:
> Looks good.
>>> - I think the change should call at least HeapRegion::hr_clear() on the
>>> region to remove or reset any auxiliary data structures, if not
>>> G1CollectedHeap::free_region() (without adding the region to the free
>>> Since the HeapRegion* is not deallocated by the uncommit, this may cause
>>> strange behavior later when the region is reused.
>> I don't think calling hr_clear should be necessary... If it is, we
>> should be doing it in shrink_by as well, and I don't think we are. I
>> don't see how a HeapRegion can be 'reused' without having gone through
>> the constructor when expand_ asks (indirectly) for 'new HeapRegion', and
>> that does an hr_clear() as well as the rest of init. Or am I missing
>> something there?
> Leave it as is. I thought that a full gc (which is the only case where
> the heap shrinks at the moment) also clears the remset of these regions
> at least.
> It should, I filed JDK-8134048 for looking in this issue.
> Looks good.
More information about the hotspot-gc-dev