RFR: 8047818: G1 HeapRegions can no longer be ContiguousSpaces
mikael.gerdin at oracle.com
Tue Jun 24 14:43:41 UTC 2014
On Tuesday 24 June 2014 07.26.52 Jon Masamitsu wrote:
> Did you consider creating a base class for ContiguousSpace and
> G1OffsetTableContigSpace that has a _top but does not assume
I did consider it but I thought that the added complexity of having even more
levels of inheritance were not worth the benefit of sharing the _top field.
Especially since G1 attempts to hack around the semantics around the _top
field with respect to concurrent access. See the disjunction I removed from
allocate_impl and when G1 calls cas_allocate and allocate.
> Could allocate_inner() have been called allocate_impl() as it is
> in ContiguousSpace? I don't know what the "inner" in the name
> is telling me.
"inner" tries to signal that this function is internal and wrapped by other
methods providing the external API. I don't have a particular naming
preference here, if the other reviewers are fine with "impl" or don't have a
preference I'm fine with changing it.
> I've just started on the review so more to come.
> On 06/23/2014 07:26 AM, Mikael Gerdin wrote:
> > Hi!
> > When G1 is modified to unload classes without doing full collections the
> > old HeapRegions can contain unparseable objects. This makes
> > ContiguousSpace unsuitable as a base class for HeapRegion since it
> > assumes that all objects below _top are parseable.
> > Modify G1OffsetTableContigSpace to implement allocation with a separate
> > _top and reimplement some Space pure virtuals to make object iteration
> > work as expected.
> > This change is the last part of a set of 4 changes: 8047818, 8047819,
> > 8047820, 8047821 which are needed to refactor the HeapRegion class and
> > its superclasses in order to simplify the G1 class unloading change which
> > is coming. This change depends on the 19, 20 and 21 changes.
> > Bug:
> > https://bugs.openjdk.java.net/browse/JDK-8047818
> > Webrev:
> > http://cr.openjdk.java.net/~mgerdin/8047818/webrev/
> > Notes:
> > The moving of set_offset_range is due to an introduced circular dependency
> > between g1BlockOffsetTable.inline.hpp and heapRegion.inline.hpp
> > Thanks
> > /Mikael
More information about the hotspot-gc-dev