RFR: 8214118: HeapRegions marked as archive even if CDS mapping fails
Thomas Schatzl
thomas.schatzl at oracle.com
Wed Nov 21 22:00:47 UTC 2018
Hi,
On Wed, 2018-11-21 at 13:05 -0800, Jiangli Zhou wrote:
> Hi Thomas,
>
> Just some comments for your comments. :)
>
> On 11/21/18 12:29 PM, Thomas Schatzl wrote:
> > Hi,
> >
> > On Wed, 2018-11-21 at 15:43 +0100, Stefan Johansson wrote:
> > > Hi,
> > >
> > > Please review this fix to avoid leaving heap metadata in an
> > > inconsistent state when CDS archive mapping fails.
> > >
> > > Issue: https://bugs.openjdk.java.net/browse/JDK-8214118
> > > Webrev: http://cr.openjdk.java.net/~sjohanss/8214118/00/
> >
> > the change seems good. I have two comments on this though:
> >
> > - would it be useful to clear both maps in
> > G1ArchiveAllocator::clear_range_archive()? I mean in the failure
> > case these regions can't be used for one purpose or the other as
> > soon as we fail the mapping, and saves passing the bool flag.
>
> We map the 'closed' archive regions before 'open' archive regions.
> The 'open' archive regions are only mapped if the 'closed' ones are
> successfully mapped.
>
> The 'open' archive regions have dependency on the 'closed' regions
> (objects in the open regions may have references to the closed
> objects).
> The 'closed' archive regions can be used independently however. If
> 'open' archive regions fail to map, we can still keep the 'closed'
> archives in use (no need to clear the closed map).
G1ArchiveAllocator::clear_range_archive() does not clear the whole heap
region range, just the given range which we failed to map the closed or
open archive into as far as I understand. There can be no overlap, so
clearing the range in both maps in that method should be fine.
That's also why I suggested having a single map containing an enum
value and not two maps with a bool each.
But maybe I'm too tired already today, so feel free to ignore my
suggestions :)
Thanks,
Thomas
More information about the hotspot-dev
mailing list