RFR (S): 8131734: Add free_archive_regions support to G1 for -Xshared:auto
jiangli.zhou at oracle.com
Thu Aug 20 19:55:21 UTC 2015
Here is the updated runtime webrev that reflects Tom’s latest GC changes.
I renamed the FileMapInfo::unmap_string_regions() to FileMapInfo::dealloc_string_regions(), which only deallocates the archived string region from the java heap without unmapping. The unmapping is handled by the GC system as the archived string region is part of the java heap. I also added dealloc_string_regions() call to the case where the string region verification fails.
On Aug 20, 2015, at 7:12 AM, Tom Benson <tom.benson at oracle.com> wrote:
> Hi Thomas,
> OK, thanks!
> 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